diff --git a/cmd/mcpmanage.go b/cmd/mcpmanage.go index a0fa56a..515e7a2 100644 --- a/cmd/mcpmanage.go +++ b/cmd/mcpmanage.go @@ -18,14 +18,14 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) -var mcpManageSubcommands = map[string]bool{"add": true, "list": true, "remove": true, "test": true} +var mcpManageSubcommands = map[string]bool{"add": true, "list": true, "remove": true, "test": true, "enable": true, "disable": true} // IsMCPManageSubcommand reports whether `nib mcp ` is a management command // (as opposed to the server-serving forms: bare, --http, --stdio, --addr). func IsMCPManageSubcommand(s string) bool { return mcpManageSubcommands[s] } func mcpUsage() { - fmt.Fprintln(os.Stderr, "usage: nib mcp ...") + fmt.Fprintln(os.Stderr, "usage: nib mcp ...") } // RunMCPCommand dispatches `nib mcp ...` and returns an exit code. @@ -44,6 +44,10 @@ func RunMCPCommand(args []string) int { return mcpRemove(cfgr, args[1:]) case "test": return mcpTest(cfgr, args[1:]) + case "enable": + return mcpSetEnabled(cfgr, args[1:], true) + case "disable": + return mcpSetEnabled(cfgr, args[1:], false) default: fmt.Fprintf(os.Stderr, "unknown mcp command: %s\n", args[0]) mcpUsage() @@ -150,6 +154,10 @@ func mcpList(cfgr *manage.Configurator) int { return 0 } for _, s := range servers { + dis := "" + if !s.Enabled { + dis = " (disabled)" + } if s.URL != "" { tr := s.Transport if tr == "" { @@ -159,14 +167,31 @@ func mcpList(cfgr *manage.Configurator) int { if s.Authenticated { suffix = " (authenticated)" } - fmt.Printf("%-20s %s %s%s\n", s.Name, tr, s.URL, suffix) + fmt.Printf("%-20s %s %s%s%s\n", s.Name, tr, s.URL, suffix, dis) } else { - fmt.Printf("%-20s %s\n", s.Name, strings.TrimSpace(s.Command+" "+strings.Join(s.Args, " "))) + fmt.Printf("%-20s %s%s\n", s.Name, strings.TrimSpace(s.Command+" "+strings.Join(s.Args, " ")), dis) } } return 0 } +func mcpSetEnabled(cfgr *manage.Configurator, args []string, enabled bool) int { + if len(args) < 1 { + fmt.Fprintln(os.Stderr, "usage: nib mcp enable|disable ") + return 1 + } + if err := cfgr.SetMCPServerEnabled(args[0], enabled); err != nil { + fmt.Fprintf(os.Stderr, "failed: %v\n", err) + return 1 + } + state := "disabled" + if enabled { + state = "enabled" + } + fmt.Printf("MCP server %q %s.\n", args[0], state) + return 0 +} + func mcpRemove(cfgr *manage.Configurator, args []string) int { if len(args) < 1 { fmt.Fprintln(os.Stderr, "usage: nib mcp remove ") diff --git a/cmd/mcpmanage_test.go b/cmd/mcpmanage_test.go index d0b9b4a..91ffd3c 100644 --- a/cmd/mcpmanage_test.go +++ b/cmd/mcpmanage_test.go @@ -1,7 +1,10 @@ package cmd import ( + "io" + "os" "reflect" + "strings" "testing" "github.com/mudler/nib/manage" @@ -179,6 +182,77 @@ func TestMCPListShowsAuthenticatedMarker(t *testing.T) { // consistent with the rest of this file. } +func TestMCPSetEnabledTogglesAndErrors(t *testing.T) { + dir := t.TempDir() + cfgr := manage.New(dir, dir+"/config.yaml") + if err := cfgr.AddMCPServer("s", types.MCPServer{Command: "cmd"}); err != nil { + t.Fatalf("AddMCPServer: %v", err) + } + if code := mcpSetEnabled(cfgr, []string{"s"}, false); code != 0 { + t.Fatalf("disable exit=%d", code) + } + if got, _ := cfgr.GetMCPServer("s"); !got.Disabled { + t.Fatal("expected server disabled after disable") + } + if code := mcpSetEnabled(cfgr, []string{"s"}, true); code != 0 { + t.Fatalf("enable exit=%d", code) + } + if got, _ := cfgr.GetMCPServer("s"); got.Disabled { + t.Fatal("expected server enabled after enable") + } + if code := mcpSetEnabled(cfgr, nil, true); code == 0 { + t.Fatal("expected nonzero exit for missing name") + } + if code := mcpSetEnabled(cfgr, []string{"nope"}, false); code == 0 { + t.Fatal("expected nonzero exit for unknown server") + } +} + +func TestMCPListShowsDisabledMarker(t *testing.T) { + dir := t.TempDir() + cfgr := manage.New(dir, dir+"/config.yaml") + if err := cfgr.AddMCPServer("on", types.MCPServer{URL: "https://a"}); err != nil { + t.Fatalf("AddMCPServer on: %v", err) + } + if err := cfgr.AddMCPServer("off", types.MCPServer{URL: "https://b"}); err != nil { + t.Fatalf("AddMCPServer off: %v", err) + } + if err := cfgr.SetMCPServerEnabled("off", false); err != nil { + t.Fatalf("SetMCPServerEnabled: %v", err) + } + + // Capture mcpList's stdout so we can assert the per-line marker. + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + os.Stdout = w + code := mcpList(cfgr) + w.Close() + os.Stdout = old + if code != 0 { + t.Fatalf("mcpList exit=%d", code) + } + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("ReadAll: %v", err) + } + + for _, line := range strings.Split(strings.TrimRight(string(out), "\n"), "\n") { + switch { + case strings.HasPrefix(line, "off"): + if !strings.Contains(line, "(disabled)") { + t.Errorf("disabled server line missing marker: %q", line) + } + case strings.HasPrefix(line, "on"): + if strings.Contains(line, "(disabled)") { + t.Errorf("enabled server line should not have marker: %q", line) + } + } + } +} + func TestIsMCPManageSubcommand(t *testing.T) { for _, s := range []string{"add", "list", "remove", "test"} { if !IsMCPManageSubcommand(s) { diff --git a/manage/configurator.go b/manage/configurator.go index fc3129e..8a90010 100644 --- a/manage/configurator.go +++ b/manage/configurator.go @@ -102,7 +102,26 @@ func (c *Configurator) ListSkills() ([]SkillInfo, error) { } // EffectiveConfig recomputes the merged config (same as startup) so callers can -// re-wire a live session after a change. +// re-wire a live session after a change. Disabled MCP servers are dropped here +// so they never start a transport; ListMCPServers still reports them for the UI. func (c *Configurator) EffectiveConfig() (types.Config, error) { - return config.Load(), nil + cfg := config.Load() + cfg.MCPServers = enabledMCPServers(cfg.MCPServers) + return cfg, nil +} + +// enabledMCPServers returns a copy of servers without entries whose Disabled +// flag is set. nil/empty input is returned unchanged; the input is never mutated. +func enabledMCPServers(servers map[string]types.MCPServer) map[string]types.MCPServer { + if len(servers) == 0 { + return servers + } + out := make(map[string]types.MCPServer, len(servers)) + for name, s := range servers { + if s.Disabled { + continue + } + out[name] = s + } + return out } diff --git a/manage/configurator_test.go b/manage/configurator_test.go index 02547ef..5d6ae25 100644 --- a/manage/configurator_test.go +++ b/manage/configurator_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/mudler/nib/plugin" + "github.com/mudler/nib/types" ) func newTestConfigurator(t *testing.T) (*Configurator, string) { @@ -43,3 +44,25 @@ func TestListPluginsReflectsRegistry(t *testing.T) { t.Fatalf("unexpected: %+v", plugins) } } + +func TestEnabledMCPServersDropsDisabled(t *testing.T) { + in := map[string]types.MCPServer{ + "on": {Command: "a"}, + "off": {Command: "b", Disabled: true}, + } + out := enabledMCPServers(in) + if _, ok := out["off"]; ok { + t.Fatalf("disabled server should be dropped: %+v", out) + } + if _, ok := out["on"]; !ok { + t.Fatalf("enabled server should be kept: %+v", out) + } + // Input map is not mutated (in still has both). + if len(in) != 2 { + t.Fatalf("input map must not be mutated: %+v", in) + } + // nil in, nil out. + if enabledMCPServers(nil) != nil { + t.Fatalf("nil input should return nil") + } +} diff --git a/manage/mcpconfig.go b/manage/mcpconfig.go index b2b1b18..2ef42e8 100644 --- a/manage/mcpconfig.go +++ b/manage/mcpconfig.go @@ -24,6 +24,8 @@ type MCPServerInfo struct { // a real auth token from an ordinary custom header — its purpose is "don't // silently print secrets", not "classify which headers are authentication". Authenticated bool + // Enabled is false when the server's Disabled flag is set. Absent == enabled. + Enabled bool } // userConfigServers reads only the user config file's mcp_servers map (not the @@ -92,6 +94,7 @@ func (c *Configurator) ListMCPServers() ([]MCPServerInfo, error) { URL: s.URL, Transport: s.Transport, Authenticated: s.BearerToken != "" || len(s.Headers) > 0, + Enabled: !s.Disabled, }) } sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) @@ -158,3 +161,20 @@ func (c *Configurator) RemoveMCPServer(name string) error { delete(servers, name) return writeUserConfigServers(c.configPath, servers) } + +// SetMCPServerEnabled flips a configured server's enabled state by toggling its +// Disabled flag in the user config file. Unknown names error, matching +// GetMCPServer/RemoveMCPServer. +func (c *Configurator) SetMCPServerEnabled(name string, enabled bool) error { + servers, err := userConfigServers(c.configPath) + if err != nil { + return err + } + srv, ok := servers[name] + if !ok { + return fmt.Errorf("mcp server %q not configured in %s", name, c.configPath) + } + srv.Disabled = !enabled + servers[name] = srv + return writeUserConfigServers(c.configPath, servers) +} diff --git a/manage/mcpconfig_test.go b/manage/mcpconfig_test.go index 516a8c4..bccb138 100644 --- a/manage/mcpconfig_test.go +++ b/manage/mcpconfig_test.go @@ -149,3 +149,39 @@ func TestListMCPServersRedactsAuth(t *testing.T) { t.Fatalf("headers-only server should be marked authenticated") } } + +func TestSetMCPServerEnabledTogglesDisabledFlag(t *testing.T) { + c, _ := newTestConfigurator(t) + if err := c.AddMCPServer("s", types.MCPServer{Command: "cmd"}); err != nil { + t.Fatalf("AddMCPServer: %v", err) + } + // A freshly added server is enabled (Disabled absent == false). + servers, _ := c.ListMCPServers() + if len(servers) != 1 || !servers[0].Enabled { + t.Fatalf("new server should be Enabled: %+v", servers) + } + // Disable it. + if err := c.SetMCPServerEnabled("s", false); err != nil { + t.Fatalf("SetMCPServerEnabled(false): %v", err) + } + got, _ := c.GetMCPServer("s") + if !got.Disabled { + t.Fatalf("expected Disabled=true, got %+v", got) + } + servers, _ = c.ListMCPServers() + if servers[0].Enabled { + t.Fatalf("listing should report Enabled=false after disable") + } + // Re-enable it. + if err := c.SetMCPServerEnabled("s", true); err != nil { + t.Fatalf("SetMCPServerEnabled(true): %v", err) + } + got, _ = c.GetMCPServer("s") + if got.Disabled { + t.Fatalf("expected Disabled=false after re-enable, got %+v", got) + } + // Unknown server errors. + if err := c.SetMCPServerEnabled("missing", false); err == nil { + t.Fatalf("expected error for unknown server") + } +} diff --git a/types/config.go b/types/config.go index 40afe48..8702665 100644 --- a/types/config.go +++ b/types/config.go @@ -254,4 +254,9 @@ type MCPServer struct { BearerToken string `yaml:"token,omitempty"` // remote only: sent as "Authorization: Bearer " Headers map[string]string `yaml:"headers,omitempty"` // remote only: custom HTTP headers + + // Disabled, when true, keeps the server in config (so the UI can list and + // re-enable it) but excludes it from EffectiveConfig, so it starts no + // transport. Absent (omitempty) reads as enabled — no migration needed. + Disabled bool `yaml:"disabled,omitempty"` }