fix(mcp): rewrite mcpcmd for new core/cli Command API + correct bridge test

The mcpcmd package was using the removed Cobra-style cli.Command API
(Use/Short/Long/RunE/StringFlag/AddCommand). Rewrites it to the current
core.Command{Description, Action, Flags} path-routed pattern so the
core-mcp binary compiles again. Registers both "mcp" and "mcp/serve"
for parity with the existing OnStartup service-mode flow.

Fixes the bridge DescribableGroup test that expected len == svc.Tools()
but ToolBridge.Describe prepends the GET tool-listing entry, so the
correct expectation is len + 1.

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-04-14 18:01:17 +01:00
parent e5caa8d32e
commit 62c1949458
3 changed files with 275 additions and 76 deletions

View file

@ -1,7 +1,14 @@
// Package mcpcmd provides the MCP server command.
// SPDX-License-Identifier: EUPL-1.2
// Package mcpcmd registers the `mcp` and `mcp serve` CLI commands.
//
// Wiring example:
//
// cli.Main(cli.WithCommands("mcp", mcpcmd.AddMCPCommands))
//
// Commands:
// - mcp serve: Start the MCP server for AI tool integration
// - mcp Start the MCP server on stdio (default transport).
// - mcp serve Start the MCP server with auto-selected transport.
package mcpcmd
import (
@ -10,75 +17,89 @@ import (
"os/signal"
"syscall"
core "dappco.re/go/core"
"dappco.re/go/mcp/pkg/mcp"
"dappco.re/go/mcp/pkg/mcp/agentic"
"dappco.re/go/mcp/pkg/mcp/brain"
"dappco.re/go/core/cli/pkg/cli"
)
var workspaceFlag string
var unrestrictedFlag bool
// newMCPService is the service constructor, indirected for tests.
var newMCPService = mcp.New
// runMCPService starts the MCP server, indirected for tests.
var runMCPService = func(svc *mcp.Service, ctx context.Context) error {
return svc.Run(ctx)
}
// shutdownMCPService performs graceful shutdown, indirected for tests.
var shutdownMCPService = func(svc *mcp.Service, ctx context.Context) error {
return svc.Shutdown(ctx)
}
var mcpCmd = &cli.Command{
Use: "mcp",
Short: "MCP server for AI tool integration",
Long: "Model Context Protocol (MCP) server providing file operations, RAG, and metrics tools.",
// workspaceFlag mirrors the --workspace CLI flag value.
var workspaceFlag string
// unrestrictedFlag mirrors the --unrestricted CLI flag value.
var unrestrictedFlag bool
// AddMCPCommands registers the `mcp` command tree on the Core instance.
//
// cli.Main(cli.WithCommands("mcp", mcpcmd.AddMCPCommands))
func AddMCPCommands(c *core.Core) {
c.Command("mcp", core.Command{
Description: "Model Context Protocol server (stdio, TCP, Unix socket, HTTP).",
Action: runServeAction,
Flags: core.NewOptions(
core.Option{Key: "workspace", Value: ""},
core.Option{Key: "w", Value: ""},
core.Option{Key: "unrestricted", Value: false},
),
})
c.Command("mcp/serve", core.Command{
Description: "Start the MCP server with auto-selected transport (stdio, TCP, Unix, or HTTP).",
Action: runServeAction,
Flags: core.NewOptions(
core.Option{Key: "workspace", Value: ""},
core.Option{Key: "w", Value: ""},
core.Option{Key: "unrestricted", Value: false},
),
})
}
var serveCmd = &cli.Command{
Use: "serve",
Short: "Start the MCP server",
Long: `Start the MCP server on stdio (default), TCP, Unix socket, or HTTP.
// runServeAction is the CLI entrypoint for `mcp` and `mcp serve`.
//
// opts := core.NewOptions(core.Option{Key: "workspace", Value: "."})
// result := runServeAction(opts)
func runServeAction(opts core.Options) core.Result {
workspaceFlag = core.Trim(firstNonEmpty(opts.String("workspace"), opts.String("w")))
unrestrictedFlag = opts.Bool("unrestricted")
The server provides file operations plus the brain and agentic subsystems
registered by this command.
Environment variables:
MCP_ADDR TCP address to listen on (e.g., "localhost:9999")
MCP_UNIX_SOCKET
Unix socket path to listen on (e.g., "/tmp/core-mcp.sock")
Selected after MCP_ADDR and before stdio.
MCP_HTTP_ADDR
HTTP address to listen on (e.g., "127.0.0.1:9101")
Selected before MCP_ADDR and stdio.
Examples:
# Start with stdio transport (for Claude Code integration)
core mcp serve
# Start with workspace restriction
core mcp serve --workspace /path/to/project
# Start unrestricted (explicit opt-in)
core mcp serve --unrestricted
# Start TCP server
MCP_ADDR=localhost:9999 core mcp serve`,
RunE: func(cmd *cli.Command, args []string) error {
return runServe()
},
if err := runServe(); err != nil {
return core.Result{Value: err, OK: false}
}
return core.Result{OK: true}
}
func initFlags() {
cli.StringFlag(serveCmd, &workspaceFlag, "workspace", "w", "", "Restrict file operations to this directory")
cli.BoolFlag(serveCmd, &unrestrictedFlag, "unrestricted", "", false, "Disable filesystem sandboxing entirely")
}
// AddMCPCommands registers the 'mcp' command and all subcommands.
func AddMCPCommands(root *cli.Command) {
initFlags()
mcpCmd.AddCommand(serveCmd)
root.AddCommand(mcpCmd)
// firstNonEmpty returns the first non-empty string argument.
//
// firstNonEmpty("", "foo") == "foo"
// firstNonEmpty("bar", "baz") == "bar"
func firstNonEmpty(values ...string) string {
for _, v := range values {
if v != "" {
return v
}
}
return ""
}
// runServe wires the MCP service together and blocks until the context is
// cancelled by SIGINT/SIGTERM or a transport error.
//
// if err := runServe(); err != nil {
// core.Error("mcp serve failed", "err", err)
// }
func runServe() error {
opts := mcp.Options{}
@ -88,22 +109,20 @@ func runServe() error {
opts.WorkspaceRoot = workspaceFlag
}
// Register OpenBrain and agentic subsystems
// Register OpenBrain and agentic subsystems.
opts.Subsystems = []mcp.Subsystem{
brain.NewDirect(),
agentic.NewPrep(),
}
// Create the MCP service
svc, err := newMCPService(opts)
if err != nil {
return cli.Wrap(err, "create MCP service")
return core.E("mcpcmd.runServe", "create MCP service", err)
}
defer func() {
_ = shutdownMCPService(svc, context.Background())
}()
// Set up signal handling for clean shutdown
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
@ -111,10 +130,12 @@ func runServe() error {
signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
go func() {
<-sigCh
cancel()
select {
case <-sigCh:
cancel()
case <-ctx.Done():
}
}()
// Run the server (blocks until context cancelled or error)
return runMCPService(svc, ctx)
}

View file

@ -1,26 +1,18 @@
// SPDX-License-Identifier: EUPL-1.2
package mcpcmd
import (
"context"
"testing"
core "dappco.re/go/core"
"dappco.re/go/mcp/pkg/mcp"
)
func TestRunServe_Good_ShutsDownService(t *testing.T) {
oldNew := newMCPService
oldRun := runMCPService
oldShutdown := shutdownMCPService
oldWorkspace := workspaceFlag
oldUnrestricted := unrestrictedFlag
t.Cleanup(func() {
newMCPService = oldNew
runMCPService = oldRun
shutdownMCPService = oldShutdown
workspaceFlag = oldWorkspace
unrestrictedFlag = oldUnrestricted
})
func TestCmdMCP_RunServe_Good_ShutsDownService(t *testing.T) {
restore := stubMCPService(t)
defer restore()
workspaceFlag = ""
unrestrictedFlag = false
@ -50,3 +42,186 @@ func TestRunServe_Good_ShutsDownService(t *testing.T) {
t.Fatal("expected shutdownMCPService to be called")
}
}
func TestCmdMCP_RunServeAction_Good_PropagatesFlags(t *testing.T) {
restore := stubMCPService(t)
defer restore()
workspaceFlag = ""
unrestrictedFlag = false
var gotOpts mcp.Options
newMCPService = func(opts mcp.Options) (*mcp.Service, error) {
gotOpts = opts
return mcp.New(mcp.Options{WorkspaceRoot: t.TempDir()})
}
runMCPService = func(svc *mcp.Service, ctx context.Context) error {
return nil
}
shutdownMCPService = func(svc *mcp.Service, ctx context.Context) error {
return nil
}
tmp := t.TempDir()
opts := core.NewOptions(core.Option{Key: "workspace", Value: tmp})
result := runServeAction(opts)
if !result.OK {
t.Fatalf("expected OK, got %+v", result)
}
if gotOpts.WorkspaceRoot != tmp {
t.Fatalf("expected workspace root %q, got %q", tmp, gotOpts.WorkspaceRoot)
}
if gotOpts.Unrestricted {
t.Fatal("expected Unrestricted=false when --workspace is set")
}
}
func TestCmdMCP_RunServeAction_Good_UnrestrictedFlag(t *testing.T) {
restore := stubMCPService(t)
defer restore()
workspaceFlag = ""
unrestrictedFlag = false
var gotOpts mcp.Options
newMCPService = func(opts mcp.Options) (*mcp.Service, error) {
gotOpts = opts
return mcp.New(mcp.Options{Unrestricted: true})
}
runMCPService = func(svc *mcp.Service, ctx context.Context) error {
return nil
}
shutdownMCPService = func(svc *mcp.Service, ctx context.Context) error {
return nil
}
opts := core.NewOptions(core.Option{Key: "unrestricted", Value: true})
result := runServeAction(opts)
if !result.OK {
t.Fatalf("expected OK, got %+v", result)
}
if !gotOpts.Unrestricted {
t.Fatal("expected Unrestricted=true when --unrestricted is set")
}
}
func TestCmdMCP_RunServe_Bad_CreateServiceFails(t *testing.T) {
restore := stubMCPService(t)
defer restore()
workspaceFlag = ""
unrestrictedFlag = false
sentinel := core.E("mcpcmd.test", "boom", nil)
newMCPService = func(opts mcp.Options) (*mcp.Service, error) {
return nil, sentinel
}
runMCPService = func(svc *mcp.Service, ctx context.Context) error {
t.Fatal("runMCPService should not be called when New fails")
return nil
}
shutdownMCPService = func(svc *mcp.Service, ctx context.Context) error {
t.Fatal("shutdownMCPService should not be called when New fails")
return nil
}
err := runServe()
if err == nil {
t.Fatal("expected error when newMCPService fails")
}
}
func TestCmdMCP_RunServeAction_Bad_PropagatesFailure(t *testing.T) {
restore := stubMCPService(t)
defer restore()
workspaceFlag = ""
unrestrictedFlag = false
newMCPService = func(opts mcp.Options) (*mcp.Service, error) {
return nil, core.E("mcpcmd.test", "construction failed", nil)
}
runMCPService = func(svc *mcp.Service, ctx context.Context) error {
return nil
}
shutdownMCPService = func(svc *mcp.Service, ctx context.Context) error {
return nil
}
result := runServeAction(core.NewOptions())
if result.OK {
t.Fatal("expected runServeAction to fail when service creation fails")
}
if result.Value == nil {
t.Fatal("expected error value on failure")
}
}
func TestCmdMCP_FirstNonEmpty_Ugly_HandlesAllVariants(t *testing.T) {
tests := []struct {
name string
values []string
want string
}{
{"no args", nil, ""},
{"empty string", []string{""}, ""},
{"all empty", []string{"", "", ""}, ""},
{"first non-empty", []string{"foo", "bar"}, "foo"},
{"skip empty", []string{"", "baz"}, "baz"},
{"mixed", []string{"", "", "last"}, "last"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := firstNonEmpty(tc.values...)
if got != tc.want {
t.Fatalf("firstNonEmpty(%v) = %q, want %q", tc.values, got, tc.want)
}
})
}
}
func TestCmdMCP_AddMCPCommands_Good_RegistersMcpTree(t *testing.T) {
c := core.New()
AddMCPCommands(c)
commands := c.Commands()
if len(commands) == 0 {
t.Fatal("expected at least one registered command")
}
mustHave := map[string]bool{
"mcp": false,
"mcp/serve": false,
}
for _, path := range commands {
if _, ok := mustHave[path]; ok {
mustHave[path] = true
}
}
for path, present := range mustHave {
if !present {
t.Fatalf("expected command %q to be registered", path)
}
}
}
// stubMCPService captures the package-level function pointers and returns a
// restore hook so each test can mutate them without leaking into siblings.
func stubMCPService(t *testing.T) func() {
t.Helper()
oldNew := newMCPService
oldRun := runMCPService
oldShutdown := shutdownMCPService
oldWorkspace := workspaceFlag
oldUnrestricted := unrestrictedFlag
return func() {
newMCPService = oldNew
runMCPService = oldRun
shutdownMCPService = oldShutdown
workspaceFlag = oldWorkspace
unrestrictedFlag = oldUnrestricted
}
}

View file

@ -81,13 +81,16 @@ func TestBridgeToAPI_Good_DescribableGroup(t *testing.T) {
var dg api.DescribableGroup = bridge
descs := dg.Describe()
if len(descs) != len(svc.Tools()) {
t.Fatalf("expected %d descriptions, got %d", len(svc.Tools()), len(descs))
// ToolBridge.Describe prepends a GET entry describing the tool listing
// endpoint, so the expected count is svc.Tools() + 1.
wantDescs := len(svc.Tools()) + 1
if len(descs) != wantDescs {
t.Fatalf("expected %d descriptions, got %d", wantDescs, len(descs))
}
for _, d := range descs {
if d.Method != "POST" {
t.Errorf("expected Method=POST for %s, got %q", d.Path, d.Method)
if d.Method != "POST" && d.Method != "GET" {
t.Errorf("expected Method=POST or GET for %s, got %q", d.Path, d.Method)
}
if d.Summary == "" {
t.Errorf("expected non-empty Summary for %s", d.Path)