From ada56b8592554b4611a295f89fcac7f6960a3bdf Mon Sep 17 00:00:00 2001 From: George Lydakis Date: Fri, 28 Aug 2026 14:03:07 -0400 Subject: [PATCH 1/2] fix: stop CWD config discovery, listing probes, and fallback command shadowing Rewrite the still-valid security/correctness fixes from stale PRs #12, #11, and #10 onto current main. Bare mcpx stays config-only, project-local .mcp.json is no longer auto-imported, and fallback merge is skipped when managed servers already exist. --- docs/usage.md | 3 +- internal/config/fallback.go | 38 ++++----------- internal/config/fallback_test.go | 65 ++++---------------------- internal/daemon/daemon_tools_test.go | 45 ++++++++---------- internal/daemon/runtime_config_test.go | 5 +- internal/servercatalog/catalog.go | 20 +------- internal/servercatalog/catalog_test.go | 20 ++++---- 7 files changed, 52 insertions(+), 144 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 200890d..7d5074e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -235,7 +235,6 @@ man mcpx - Cline MCP settings - Claude Code user/local config (`~/.claude.json`) - Codex config (`~/.codex/config.toml`, `mcp_servers.*`; when `[features].apps = true`, `mcpx` adds virtual per-app servers like `linear`/`zillow`, backed by Codex connector auth from `CODEX_CONNECTORS_TOKEN` or `~/.codex/auth.json`) - - Claude Code project config (`.mcp.json`, nearest parent) - Kiro user config (`~/.kiro/settings/mcp.json`) - - Kiro project config (`.kiro/settings/mcp.json`, nearest parent) + - Project-local `.mcp.json` and `.kiro/settings/mcp.json` are not auto-discovered from the working directory. - Check fallback files exist and expose either `mcpServers` (JSON sources) or `mcp_servers` (Codex TOML). Claude Code local scope uses `projects[].mcpServers`. diff --git a/internal/config/fallback.go b/internal/config/fallback.go index 7ad06d6..f123445 100644 --- a/internal/config/fallback.go +++ b/internal/config/fallback.go @@ -151,18 +151,24 @@ func FailedFallbackSourcePaths(err error) []string { } // MergeFallbackServers fills cfg.Servers from external MCP fallback sources. -// Managed entries already present in cfg.Servers always win over discovered ones. +// If cfg already has managed servers, fallback sources are not merged. +// When merging, managed entries already present in cfg.Servers always win +// over discovered ones of the same name. func MergeFallbackServers(cfg *Config) error { return MergeFallbackServersForCWD(cfg, "") } // MergeFallbackServersForCWD is like MergeFallbackServers but resolves -// project-scoped fallback files against the provided working directory. +// Claude Code project entries in home-level configs against cwd. // When cwd is empty, it falls back to the process working directory. +// Fallback discovery is skipped entirely when managed servers already exist. func MergeFallbackServersForCWD(cfg *Config, cwd string) error { if cfg == nil { return nil } + if len(cfg.Servers) > 0 { + return nil + } fallback, err := loadFallbackServersWithSourcesForCWD(fallbackSourcePathsForCWD(cfg, cwd), cwd) if len(fallback) > 0 { @@ -588,28 +594,6 @@ func isWithinPath(path, root string) bool { return strings.HasPrefix(path, root+string(os.PathSeparator)) } -func nearestUpwardPath(relPath, cwd string) string { - base := resolveWorkingDirectory(cwd) - if base == "" { - return "" - } - - dir := base - for { - candidate := filepath.Join(dir, relPath) - info, err := os.Stat(candidate) - if err == nil && !info.IsDir() { - return candidate - } - - parent := filepath.Dir(dir) - if parent == dir { - return "" - } - dir = parent - } -} - func fallbackSourcePaths(cfg *Config) []string { return fallbackSourcePathsForCWD(cfg, "") } @@ -649,7 +633,7 @@ func defaultFallbackSourcePaths() []string { return defaultFallbackSourcePathsForCWD("") } -func defaultFallbackSourcePathsForCWD(cwd string) []string { +func defaultFallbackSourcePathsForCWD(_ string) []string { home, _ := os.UserHomeDir() if home == "" { return nil @@ -663,9 +647,7 @@ func defaultFallbackSourcePathsForCWD(cwd string) []string { filepath.Join(home, "Library", "Application Support", "Code", "User", "globalStorage", "saoudrizwan.claude-dev", "settings", "cline_mcp_settings.json"), filepath.Join(home, ".claude.json"), filepath.Join(home, ".codex", "config.toml"), - nearestUpwardPath(".mcp.json", cwd), filepath.Join(home, ".kiro", "settings", "mcp.json"), - nearestUpwardPath(filepath.Join(".kiro", "settings", "mcp.json"), cwd), } case "linux": return []string{ @@ -674,9 +656,7 @@ func defaultFallbackSourcePathsForCWD(cwd string) []string { filepath.Join(home, ".config", "Code", "User", "globalStorage", "saoudrizwan.claude-dev", "settings", "cline_mcp_settings.json"), filepath.Join(home, ".claude.json"), filepath.Join(home, ".codex", "config.toml"), - nearestUpwardPath(".mcp.json", cwd), filepath.Join(home, ".kiro", "settings", "mcp.json"), - nearestUpwardPath(filepath.Join(".kiro", "settings", "mcp.json"), cwd), } default: return nil diff --git a/internal/config/fallback_test.go b/internal/config/fallback_test.go index 10f3f12..26f3448 100644 --- a/internal/config/fallback_test.go +++ b/internal/config/fallback_test.go @@ -85,7 +85,7 @@ func TestMergeFallbackServersUsesFallbackWhenConfigEmpty(t *testing.T) { } } -func TestMergeFallbackServersKeepsManagedAndAddsDiscovered(t *testing.T) { +func TestMergeFallbackServersKeepsManagedOnlyWhenConfigured(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -127,11 +127,8 @@ func TestMergeFallbackServersKeepsManagedAndAddsDiscovered(t *testing.T) { t.Fatalf("managed origin kind = %q, want %q", origin.Kind, ServerOriginKindMCPXConfig) } - if _, ok := cfg.Servers["filesystem"]; !ok { - t.Fatalf("cfg.Servers = %#v, want discovered filesystem server", cfg.Servers) - } - if origin := cfg.ServerOrigins["filesystem"]; origin.Kind == ServerOriginKindMCPXConfig { - t.Fatalf("filesystem origin kind = %q, want discovered source kind", origin.Kind) + if _, ok := cfg.Servers["filesystem"]; ok { + t.Fatalf("cfg.Servers = %#v, want no discovered servers when managed config exists", cfg.Servers) } } @@ -449,6 +446,7 @@ bearer_token_env_var = "REMOTE_TOKEN" } func TestLoadCodexConfigFileAddsCodexAppsServerFromAuthFile(t *testing.T) { + t.Setenv("CODEX_HOME", "") home := t.TempDir() t.Setenv("HOME", home) @@ -492,6 +490,7 @@ apps = true } func TestLoadCodexConfigFileCodexAppsUsesConnectorsTokenEnv(t *testing.T) { + t.Setenv("CODEX_HOME", "") home := t.TempDir() t.Setenv("HOME", home) t.Setenv(codexConnectorsTokenEnvVar, "connectors-999") @@ -730,43 +729,6 @@ func TestLoadMCPServersFileReadsClaudeCodeProjectServers(t *testing.T) { } } -func TestNearestUpwardPathFindsNearestParent(t *testing.T) { - root := t.TempDir() - parent := filepath.Join(root, "parent") - child := filepath.Join(parent, "child") - grandChild := filepath.Join(child, "grandchild") - if err := os.MkdirAll(grandChild, 0700); err != nil { - t.Fatalf("mkdir grandchild: %v", err) - } - - nearest := filepath.Join(child, ".mcp.json") - farther := filepath.Join(parent, ".mcp.json") - for _, path := range []string{nearest, farther} { - if err := os.WriteFile(path, []byte(`{"mcpServers":{}}`), 0600); err != nil { - t.Fatalf("write %s: %v", path, err) - } - } - - prevWD, err := os.Getwd() - if err != nil { - t.Fatalf("getwd: %v", err) - } - t.Cleanup(func() { - _ = os.Chdir(prevWD) - }) - if err := os.Chdir(grandChild); err != nil { - t.Fatalf("chdir grandchild: %v", err) - } - - if got := nearestUpwardPath(".mcp.json", ""); got != nearest { - gotResolved, gotErr := filepath.EvalSymlinks(got) - wantResolved, wantErr := filepath.EvalSymlinks(nearest) - if gotErr != nil || wantErr != nil || gotResolved != wantResolved { - t.Fatalf("nearestUpwardPath(.mcp.json) = %q, want %q", got, nearest) - } - } -} - func TestMergeFallbackServersForCWDUsesProvidedWorkingDirectory(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -791,17 +753,6 @@ func TestMergeFallbackServersForCWDUsesProvidedWorkingDirectory(t *testing.T) { t.Fatalf("write project-b config: %v", err) } - prevWD, err := os.Getwd() - if err != nil { - t.Fatalf("getwd: %v", err) - } - t.Cleanup(func() { - _ = os.Chdir(prevWD) - }) - if err := os.Chdir(projectASubdir); err != nil { - t.Fatalf("chdir project-a subdir: %v", err) - } - paths := fallbackSourcePathsForCWD(nil, projectBSubdir) if len(paths) == 0 { t.Skip("no fallback source paths for this platform") @@ -811,11 +762,11 @@ func TestMergeFallbackServersForCWDUsesProvidedWorkingDirectory(t *testing.T) { if err := MergeFallbackServersForCWD(cfg, projectBSubdir); err != nil { t.Fatalf("MergeFallbackServersForCWD() error = %v", err) } - if _, ok := cfg.Servers["server-b"]; !ok { - t.Fatalf("cfg.Servers = %#v, want server-b from provided cwd", cfg.Servers) + if _, ok := cfg.Servers["server-b"]; ok { + t.Fatalf("cfg.Servers = %#v, want project-local server excluded", cfg.Servers) } if _, ok := cfg.Servers["server-a"]; ok { - t.Fatalf("cfg.Servers = %#v, want server-a excluded", cfg.Servers) + t.Fatalf("cfg.Servers = %#v, want project-local server excluded", cfg.Servers) } } diff --git a/internal/daemon/daemon_tools_test.go b/internal/daemon/daemon_tools_test.go index 359fd7d..71c7ce7 100644 --- a/internal/daemon/daemon_tools_test.go +++ b/internal/daemon/daemon_tools_test.go @@ -3,7 +3,6 @@ package daemon import ( "context" "encoding/json" - "errors" "reflect" "strings" "testing" @@ -171,7 +170,7 @@ func TestToolSchemaPayloadUsesNativeToolName(t *testing.T) { } } -func TestListServersHidesCodexAppsAndShowsVirtualServers(t *testing.T) { +func TestListServersHidesCodexAppsWithoutDiscovery(t *testing.T) { cfg := &config.Config{ Servers: map[string]config.ServerConfig{ "github": {}, @@ -186,16 +185,11 @@ func TestListServersHidesCodexAppsAndShowsVirtualServers(t *testing.T) { ka := NewKeepalive(nil) defer ka.Stop() + calls := 0 deps := runtimeDefaultDeps() - deps.poolListTools = func(_ context.Context, _ *mcppool.Pool, server string) ([]mcppool.ToolInfo, error) { - if server != codexAppsServerName { - t.Fatalf("poolListTools server = %q, want %q", server, codexAppsServerName) - } - return []mcppool.ToolInfo{ - {Name: "linear_get_profile"}, - {Name: "zillow_get_zestimate"}, - {Name: "google calendar_search"}, - }, nil + deps.poolListTools = func(_ context.Context, _ *mcppool.Pool, _ string) ([]mcppool.ToolInfo, error) { + calls++ + return nil, nil } resp := listServersWithDeps(context.Background(), cfg, nil, ka, false, deps) @@ -204,23 +198,19 @@ func TestListServersHidesCodexAppsAndShowsVirtualServers(t *testing.T) { } got := decodeServerLines(resp.Content) - want := []string{"github", "google_calendar", "linear", "supermemory", "zillow"} + want := []string{"github", "supermemory"} if !reflect.DeepEqual(got, want) { t.Fatalf("server list = %#v, want %#v", got, want) } entries := decodeServerEntries(resp.Content) for _, entry := range entries { - switch entry.Name { - case "github", "supermemory": - if entry.Origin.Kind != config.ServerOriginKindMCPXConfig { - t.Fatalf("server %q origin kind = %q, want %q", entry.Name, entry.Origin.Kind, config.ServerOriginKindMCPXConfig) - } - case "google_calendar", "linear", "zillow": - if entry.Origin.Kind != config.ServerOriginKindCodexApps { - t.Fatalf("server %q origin kind = %q, want %q", entry.Name, entry.Origin.Kind, config.ServerOriginKindCodexApps) - } + if entry.Origin.Kind != config.ServerOriginKindMCPXConfig { + t.Fatalf("server %q origin kind = %q, want %q", entry.Name, entry.Origin.Kind, config.ServerOriginKindMCPXConfig) } } + if calls != 0 { + t.Fatalf("codex list-tools calls = %d, want 0", calls) + } for _, name := range got { if name == codexAppsServerName { t.Fatalf("server list = %#v, want %q omitted", got, codexAppsServerName) @@ -228,7 +218,7 @@ func TestListServersHidesCodexAppsAndShowsVirtualServers(t *testing.T) { } } -func TestListServersKeepsConfiguredServersWhenCodexAppsDiscoveryFails(t *testing.T) { +func TestListServersKeepsConfiguredServersWhenCodexAppsConfigured(t *testing.T) { cfg := &config.Config{ Servers: map[string]config.ServerConfig{ "github": {}, @@ -239,9 +229,11 @@ func TestListServersKeepsConfiguredServersWhenCodexAppsDiscoveryFails(t *testing ka := NewKeepalive(nil) defer ka.Stop() + calls := 0 deps := runtimeDefaultDeps() deps.poolListTools = func(_ context.Context, _ *mcppool.Pool, _ string) ([]mcppool.ToolInfo, error) { - return nil, errors.New("token expired") + calls++ + return nil, nil } resp := listServersWithDeps(context.Background(), cfg, nil, ka, false, deps) @@ -260,8 +252,11 @@ func TestListServersKeepsConfiguredServersWhenCodexAppsDiscoveryFails(t *testing t.Fatalf("server %q origin kind = %q, want %q", entry.Name, entry.Origin.Kind, config.ServerOriginKindMCPXConfig) } } - if !strings.Contains(resp.Stderr, "failed to enumerate codex apps") { - t.Fatalf("listServers() stderr = %q, want codex-apps warning", resp.Stderr) + if resp.Stderr != "" { + t.Fatalf("listServers() stderr = %q, want empty", resp.Stderr) + } + if calls != 0 { + t.Fatalf("codex list-tools calls = %d, want 0", calls) } } diff --git a/internal/daemon/runtime_config_test.go b/internal/daemon/runtime_config_test.go index ddf50e6..7c398a7 100644 --- a/internal/daemon/runtime_config_test.go +++ b/internal/daemon/runtime_config_test.go @@ -1220,7 +1220,10 @@ func TestRuntimeRequestHandlerReloadsSameCWDWhenFallbackSourceChanges(t *testing t.Fatalf("WriteFile(config): %v", err) } - fallbackPath := filepath.Join(projectDir, ".mcp.json") + fallbackPath := filepath.Join(tmp, ".cursor", "mcp.json") + if err := os.MkdirAll(filepath.Dir(fallbackPath), 0o755); err != nil { + t.Fatalf("MkdirAll(fallback dir): %v", err) + } writeFallback := func(serverName string) { t.Helper() content := fmt.Sprintf(`{"mcpServers":{"%s":{"command":"echo"}}}`, serverName) diff --git a/internal/servercatalog/catalog.go b/internal/servercatalog/catalog.go index fa665dd..ddbde82 100644 --- a/internal/servercatalog/catalog.go +++ b/internal/servercatalog/catalog.go @@ -38,6 +38,7 @@ func New(cfg *config.Config, listTools ListToolsFunc) *Catalog { } func (c *Catalog) ServerNames(ctx context.Context) ([]string, error) { + _ = ctx if c == nil || c.cfg == nil { return nil, nil } @@ -50,25 +51,6 @@ func (c *Catalog) ServerNames(ctx context.Context) ([]string, error) { names[name] = struct{}{} } - if c.hasCodexApps() { - if c.listTools == nil { - return nil, fmt.Errorf("codex apps discovery requires list tools callback") - } - tools, err := c.listTools(ctx, CodexAppsServerName) - if err != nil { - return nil, err - } - for name := range codexVirtualServerMap(tools) { - if strings.TrimSpace(name) == "" { - continue - } - if _, exists := c.cfg.Servers[name]; exists { - continue - } - names[name] = struct{}{} - } - } - out := make([]string, 0, len(names)) for name := range names { out = append(out, name) diff --git a/internal/servercatalog/catalog_test.go b/internal/servercatalog/catalog_test.go index 917f2f6..3fa79a8 100644 --- a/internal/servercatalog/catalog_test.go +++ b/internal/servercatalog/catalog_test.go @@ -9,7 +9,7 @@ import ( "github.com/lydakis/mcpx/internal/mcppool" ) -func TestServerNamesHidesCodexAppsAndAddsVirtualApps(t *testing.T) { +func TestServerNamesHidesCodexAppsWithoutDiscovery(t *testing.T) { cfg := &config.Config{ Servers: map[string]config.ServerConfig{ "playwright": {}, @@ -18,15 +18,10 @@ func TestServerNamesHidesCodexAppsAndAddsVirtualApps(t *testing.T) { }, } - catalog := New(cfg, func(_ context.Context, server string) ([]mcppool.ToolInfo, error) { - if server != CodexAppsServerName { - t.Fatalf("listTools server = %q, want %q", server, CodexAppsServerName) - } - return []mcppool.ToolInfo{ - {Name: "linear_get_profile"}, - {Name: "zillow_get_zestimate"}, - {Name: "google calendar_search"}, - }, nil + calls := 0 + catalog := New(cfg, func(_ context.Context, _ string) ([]mcppool.ToolInfo, error) { + calls++ + return nil, nil }) names, err := catalog.ServerNames(context.Background()) @@ -34,10 +29,13 @@ func TestServerNamesHidesCodexAppsAndAddsVirtualApps(t *testing.T) { t.Fatalf("ServerNames() error = %v", err) } - want := []string{"google_calendar", "linear", "playwright", "supermemory", "zillow"} + want := []string{"playwright", "supermemory"} if !reflect.DeepEqual(names, want) { t.Fatalf("ServerNames() = %#v, want %#v", names, want) } + if calls != 0 { + t.Fatalf("codex list-tools calls = %d, want 0", calls) + } } func TestResolveReturnsConfiguredRouteWithoutCodexAppsProbe(t *testing.T) { From e0fd8f758b4289c23e37dbcbd3b4434066f05f9a Mon Sep 17 00:00:00 2001 From: George Lydakis Date: Fri, 28 Aug 2026 14:39:22 -0400 Subject: [PATCH 2/2] fix: align fallback watch with managed config and keep named virtual servers Skip merge and source watching only when config.toml has MCPXConfig origins, not whenever cfg.Servers is non-empty. Filter reserved CLI names from fallback discovery, and resolve explicit --help / shim install targets via list_tools so Codex virtual servers still work. --- internal/cli/add.go | 6 +- internal/cli/add_test.go | 69 ++++++++++ internal/cli/root.go | 74 +++++++--- internal/cli/root_test.go | 184 ++++++++++++++++++++----- internal/cli/shim.go | 45 +++++- internal/cli/shim_test.go | 89 +++++++++++- internal/cli/skill.go | 6 +- internal/config/fallback.go | 51 ++++++- internal/config/fallback_test.go | 129 +++++++++++++++++ internal/daemon/runtime_config_test.go | 42 ++++++ 10 files changed, 620 insertions(+), 75 deletions(-) diff --git a/internal/cli/add.go b/internal/cli/add.go index 710d292..eb9acaa 100644 --- a/internal/cli/add.go +++ b/internal/cli/add.go @@ -31,10 +31,8 @@ func maybeHandleAddCommand(args []string, cfg *config.Config, stdout, stderr io. return false, 0 } - if cfg != nil { - if _, ok := cfg.Servers["add"]; ok { - return false, 0 - } + if utilityCommandDeferredToServer(cfg, "add") { + return false, 0 } return true, runAddCommand(args[1:], stdout, stderr) diff --git a/internal/cli/add_test.go b/internal/cli/add_test.go index 6b220b1..7d72122 100644 --- a/internal/cli/add_test.go +++ b/internal/cli/add_test.go @@ -488,3 +488,72 @@ func TestPrintAddHelpIncludesHeaderAndOverwriteGuidance(t *testing.T) { t.Fatalf("help output missing overwrite guidance: %q", help) } } + +func TestRunAddIgnoresFallbackServerNamedAdd(t *testing.T) { + tmp := t.TempDir() + home := tmp + xdgConfigHome := filepath.Join(tmp, "xdg-config") + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", xdgConfigHome) + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + + cursorDir := filepath.Join(home, ".cursor") + if err := os.MkdirAll(cursorDir, 0o700); err != nil { + t.Fatalf("MkdirAll(cursor): %v", err) + } + if err := os.WriteFile(filepath.Join(cursorDir, "mcp.json"), []byte(`{"mcpServers":{"add":{"command":"false"}}}`), 0o600); err != nil { + t.Fatalf("WriteFile(cursor mcp.json): %v", err) + } + + oldSpawn := spawnOrConnectFn + defer func() { spawnOrConnectFn = oldSpawn }() + spawnOrConnectFn = func() (string, error) { + t.Fatal("spawnOrConnectFn should not run for mcpx add") + return "", errors.New("spawn should not run") + } + + oldOut := rootStdout + oldErr := rootStderr + defer func() { + rootStdout = oldOut + rootStderr = oldErr + }() + var out bytes.Buffer + var errOut bytes.Buffer + rootStdout = &out + rootStderr = &errOut + + code := Run([]string{"add", "--help"}) + if code != ipc.ExitOK { + t.Fatalf("Run([add --help]) = %d, want %d (stderr=%q)", code, ipc.ExitOK, errOut.String()) + } + if !bytes.Contains(out.Bytes(), []byte("mcpx add ")) { + t.Fatalf("stdout = %q, want add command help", out.String()) + } + if errOut.Len() != 0 { + t.Fatalf("stderr = %q, want empty", errOut.String()) + } +} + +func TestMaybeHandleAddCommandDoesNotDeferToFallbackOrigin(t *testing.T) { + cfg := &config.Config{ + Servers: map[string]config.ServerConfig{ + "add": {Command: "false"}, + }, + ServerOrigins: map[string]config.ServerOrigin{ + "add": config.NewServerOrigin(config.ServerOriginKindCursor, "/tmp/.cursor/mcp.json"), + }, + } + var out bytes.Buffer + var errOut bytes.Buffer + handled, code := maybeHandleAddCommand([]string{"add", "--help"}, cfg, &out, &errOut) + if !handled { + t.Fatal("handled = false, want true (fallback origin must not shadow add)") + } + if code != ipc.ExitOK { + t.Fatalf("code = %d, want %d", code, ipc.ExitOK) + } + if !bytes.Contains(out.Bytes(), []byte("mcpx add ")) { + t.Fatalf("stdout = %q, want add help", out.String()) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 9e1b644..eca1ff8 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -119,23 +119,47 @@ func Run(args []string) int { knownServerEntries := decodeServerListEntries(resp.Content) knownServers := serverNamesFromEntries(knownServerEntries) - if !containsServerName(knownServers, requestServer) { - ephemeral, resolveResp := resolveEphemeralSource(requestServer) - if resolveResp != nil { + if containsServerName(knownServers, requestServer) { + printToolListHelp(rootStdout, server) + return ipc.ExitOK + } + + // list_servers is config-only and omits Codex virtual servers. + // Explicit named operations resolve via list_tools instead of + // ServerNames membership. + resolveResp, err := client.Send(&ipc.Request{ + Type: "list_tools", + Server: requestServer, + CWD: cwd, + }) + if err != nil { + fmt.Fprintf(rootStderr, "mcpx: %v\n", err) + return ipc.ExitInternal + } + if !isUnknownServerResponse(resolveResp, requestServer) { + if resolveResp.ExitCode != ipc.ExitOK { if resolveResp.Stderr != "" { fmt.Fprintln(rootStderr, resolveResp.Stderr) } return resolveResp.ExitCode } - if ephemeral != nil { - printToolListHelp(rootStdout, server) - return ipc.ExitOK + printToolListHelp(rootStdout, server) + return ipc.ExitOK + } + + ephemeral, ephemeralResp := resolveEphemeralSource(requestServer) + if ephemeralResp != nil { + if ephemeralResp.Stderr != "" { + fmt.Fprintln(rootStderr, ephemeralResp.Stderr) } - printUnknownServer(server, visibleServerNamesFromEntries(knownServerEntries)) - return ipc.ExitUsageErr + return ephemeralResp.ExitCode } - printToolListHelp(rootStdout, server) - return ipc.ExitOK + if ephemeral != nil { + printToolListHelp(rootStdout, server) + return ipc.ExitOK + } + printUnknownServer(server, visibleServerNamesFromEntries(knownServerEntries)) + return ipc.ExitUsageErr } // Connect to daemon @@ -161,17 +185,13 @@ func maybeHandleCompletionCommand(args []string, cfg *config.Config, stdout, std switch args[0] { case "completion": - if cfg != nil { - if _, ok := cfg.Servers["completion"]; ok { - return false, 0 - } + if utilityCommandDeferredToServer(cfg, "completion") { + return false, 0 } return true, runCompletionCommand(args[1:], stdout, stderr) case "__complete": - if cfg != nil { - if _, ok := cfg.Servers["__complete"]; ok { - return false, 0 - } + if utilityCommandDeferredToServer(cfg, "__complete") { + return false, 0 } return true, runInternalCompletion(args[1:], stdout, stderr) default: @@ -179,6 +199,24 @@ func maybeHandleCompletionCommand(args []string, cfg *config.Config, stdout, std } } +// utilityCommandDeferredToServer reports whether a reserved CLI command name +// should yield to a configured MCP server of the same name. Only servers +// loaded from the mcpx config file (or fixtures without origin metadata) +// shadow utility commands; fallback-discovered names do not. +func utilityCommandDeferredToServer(cfg *config.Config, name string) bool { + if cfg == nil { + return false + } + if _, ok := cfg.Servers[name]; !ok { + return false + } + origin, ok := cfg.ServerOrigins[name] + if !ok { + return true + } + return config.NormalizeServerOrigin(origin).Kind == config.ServerOriginKindMCPXConfig +} + type rootServerListArgs struct { output outputMode verbose bool diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 4f5b73e..0a32d70 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -36,6 +36,14 @@ func (w errWriter) Write(p []byte) (int, error) { return 0, w.err } +func unknownServerIPCResponse(server string) *ipc.Response { + return &ipc.Response{ + ExitCode: ipc.ExitUsageErr, + Stderr: fmt.Sprintf("unknown server: %s", server), + ErrorCode: ipc.ErrorCodeUnknownServer, + } +} + func TestHandleRootFlagsVersion(t *testing.T) { oldVersion := buildVersion oldOut := rootStdout @@ -985,13 +993,18 @@ args = ["ok"] return stubDaemonClient{ sendFn: func(req *ipc.Request) (*ipc.Response, error) { calls++ - if req.Type != "list_servers" { + switch req.Type { + case "list_servers": + if !req.IncludeHidden { + return nil, errors.New("expected include_hidden list_servers request") + } + return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil + + case "list_tools": + return unknownServerIPCResponse(req.Server), nil + default: return nil, errors.New("unexpected request type") } - if !req.IncludeHidden { - return nil, errors.New("expected include_hidden list_servers request") - } - return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil }, } } @@ -1022,12 +1035,12 @@ args = ["ok"] if !bytes.Contains(errOut.Bytes(), []byte(" github")) { t.Fatalf("stderr = %q, want configured server listing", errOut.String()) } - if calls != 1 { - t.Fatalf("daemon requests = %d, want 1", calls) + if calls != 2 { + t.Fatalf("daemon requests = %d, want 2", calls) } } -func TestRunUnknownServerHelpDoesNotProbeDaemonListTools(t *testing.T) { +func TestRunUnknownServerHelpResolvesUnknownViaListTools(t *testing.T) { tmp := t.TempDir() xdgConfigHome := filepath.Join(tmp, "xdg-config") configDir := filepath.Join(xdgConfigHome, "mcpx") @@ -1070,7 +1083,7 @@ args = ["ok"] } return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil case "list_tools": - return &ipc.Response{ExitCode: ipc.ExitInternal, Stderr: "runtime bootstrap failed"}, nil + return unknownServerIPCResponse(req.Server), nil default: return nil, errors.New("unexpected request type") } @@ -1109,8 +1122,8 @@ args = ["ok"] if out.Len() != 0 { t.Fatalf("stdout = %q, want empty", out.String()) } - if calls != 1 { - t.Fatalf("daemon requests = %d, want 1", calls) + if calls != 2 { + t.Fatalf("daemon requests = %d, want 2", calls) } } @@ -1164,13 +1177,17 @@ args = ["ok"] return stubDaemonClient{ sendFn: func(req *ipc.Request) (*ipc.Response, error) { calls++ - if req.Type != "list_servers" { + switch req.Type { + case "list_servers": + if !req.IncludeHidden { + return nil, errors.New("expected include_hidden list_servers request") + } + return &ipc.Response{ExitCode: ipc.ExitOK, Content: listPayload}, nil + case "list_tools": + return unknownServerIPCResponse(req.Server), nil + default: return nil, errors.New("unexpected request type") } - if !req.IncludeHidden { - return nil, errors.New("expected include_hidden list_servers request") - } - return &ipc.Response{ExitCode: ipc.ExitOK, Content: listPayload}, nil }, } } @@ -1212,8 +1229,8 @@ args = ["ok"] if out.Len() != 0 { t.Fatalf("stdout = %q, want empty", out.String()) } - if calls != 1 { - t.Fatalf("daemon requests = %d, want 1", calls) + if calls != 2 { + t.Fatalf("daemon requests = %d, want 2", calls) } } @@ -1252,13 +1269,18 @@ func TestRunUnknownServerHelpSurfacesResolveErrorForExplicitSource(t *testing.T) return stubDaemonClient{ sendFn: func(req *ipc.Request) (*ipc.Response, error) { calls++ - if req.Type != "list_servers" { + switch req.Type { + case "list_servers": + if !req.IncludeHidden { + return nil, errors.New("expected include_hidden list_servers request") + } + return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil + + case "list_tools": + return unknownServerIPCResponse(req.Server), nil + default: return nil, errors.New("unexpected request type") } - if !req.IncludeHidden { - return nil, errors.New("expected include_hidden list_servers request") - } - return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil }, } } @@ -1296,8 +1318,8 @@ func TestRunUnknownServerHelpSurfacesResolveErrorForExplicitSource(t *testing.T) if bytes.Contains(errOut.Bytes(), []byte("unknown server:")) { t.Fatalf("stderr = %q, did not want unknown-server error", errOut.String()) } - if calls != 1 { - t.Fatalf("daemon requests = %d, want 1", calls) + if calls != 2 { + t.Fatalf("daemon requests = %d, want 2", calls) } } @@ -1339,13 +1361,18 @@ args = ["ok"] return stubDaemonClient{ sendFn: func(req *ipc.Request) (*ipc.Response, error) { calls++ - if req.Type != "list_servers" { + switch req.Type { + case "list_servers": + if !req.IncludeHidden { + return nil, errors.New("expected include_hidden list_servers request") + } + return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil + + case "list_tools": + return unknownServerIPCResponse(req.Server), nil + default: return nil, errors.New("unexpected request type") } - if !req.IncludeHidden { - return nil, errors.New("expected include_hidden list_servers request") - } - return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil }, } } @@ -1386,8 +1413,8 @@ args = ["ok"] if out.Len() != 0 { t.Fatalf("stdout = %q, want empty", out.String()) } - if calls != 1 { - t.Fatalf("daemon requests = %d, want 1", calls) + if calls != 2 { + t.Fatalf("daemon requests = %d, want 2", calls) } } @@ -1844,14 +1871,18 @@ func TestRunUnknownServerHelpReResolvesWhenDaemonHasRelativeRuntimeEntry(t *test newDaemonClient = func(_, _ string) daemonRequester { return stubDaemonClient{ sendFn: func(req *ipc.Request) (*ipc.Response, error) { - if req.Type != "list_servers" { + switch req.Type { + case "list_servers": + if !req.IncludeHidden { + return nil, errors.New("expected include_hidden list_servers request") + } + // Simulate stale relative runtime-ephemeral entry in daemon memory. + return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte(source + "\n")}, nil + case "list_tools": + return unknownServerIPCResponse(req.Server), nil + default: return nil, errors.New("unexpected request type") } - if !req.IncludeHidden { - return nil, errors.New("expected include_hidden list_servers request") - } - // Simulate stale relative runtime-ephemeral entry in daemon memory. - return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte(source + "\n")}, nil }, } } @@ -2579,3 +2610,80 @@ func TestWriteCallResponseUsageErrorPrintsStderrWithoutVerbose(t *testing.T) { t.Fatalf("stderr = %q, want invalid params diagnostics", errOut.String()) } } + +func TestRunVirtualServerHelpResolvesViaListToolsWithoutListMembership(t *testing.T) { + tmp := t.TempDir() + xdgConfigHome := filepath.Join(tmp, "xdg-config") + configDir := filepath.Join(xdgConfigHome, "mcpx") + if err := os.MkdirAll(configDir, 0o755); err != nil { + t.Fatalf("MkdirAll(configDir): %v", err) + } + configToml := []byte(`[servers.github] +command = "echo" +args = ["ok"] +`) + if err := os.WriteFile(filepath.Join(configDir, "config.toml"), configToml, 0o600); err != nil { + t.Fatalf("WriteFile(config.toml): %v", err) + } + + t.Setenv("XDG_CONFIG_HOME", xdgConfigHome) + t.Setenv("HOME", tmp) + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + + oldSpawn := spawnOrConnectFn + oldClient := newDaemonClient + defer func() { + spawnOrConnectFn = oldSpawn + newDaemonClient = oldClient + }() + + spawnOrConnectFn = func() (string, error) { return "nonce", nil } + var listServersCalls int + var listToolsCalls int + newDaemonClient = func(_, _ string) daemonRequester { + return stubDaemonClient{ + sendFn: func(req *ipc.Request) (*ipc.Response, error) { + switch req.Type { + case "list_servers": + listServersCalls++ + if !req.IncludeHidden { + return nil, errors.New("expected include_hidden list_servers request") + } + return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("github\n")}, nil + case "list_tools": + listToolsCalls++ + if req.Server != "linear" { + return nil, fmt.Errorf("unexpected list_tools server %q", req.Server) + } + return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte(`[{"name":"linear_get_profile"}]`)}, nil + default: + return nil, errors.New("unexpected request type") + } + }, + } + } + + oldOut := rootStdout + oldErr := rootStderr + defer func() { + rootStdout = oldOut + rootStderr = oldErr + }() + var out bytes.Buffer + var errOut bytes.Buffer + rootStdout = &out + rootStderr = &errOut + + if code := Run([]string{"linear", "--help"}); code != ipc.ExitOK { + t.Fatalf("Run([linear --help]) = %d, want %d (stderr=%q)", code, ipc.ExitOK, errOut.String()) + } + if !bytes.Contains(out.Bytes(), []byte("Usage: mcpx linear [FLAGS]")) { + t.Fatalf("stdout = %q, want linear help usage", out.String()) + } + if errOut.Len() != 0 { + t.Fatalf("stderr = %q, want empty", errOut.String()) + } + if listServersCalls != 1 || listToolsCalls != 1 { + t.Fatalf("daemon calls list_servers=%d list_tools=%d, want 1/1", listServersCalls, listToolsCalls) + } +} diff --git a/internal/cli/shim.go b/internal/cli/shim.go index f195934..11c1a5e 100644 --- a/internal/cli/shim.go +++ b/internal/cli/shim.go @@ -40,6 +40,7 @@ type shimListArgs struct { } var shimKnownServersFn = listShimKnownServers +var shimResolveServerFn = resolveShimServerViaListTools var installServerSkillFn = installServerSkill func maybeHandleShimCommand(args []string, cfg *config.Config, stdout, stderr io.Writer) (bool, int) { @@ -47,10 +48,8 @@ func maybeHandleShimCommand(args []string, cfg *config.Config, stdout, stderr io return false, 0 } - if cfg != nil { - if _, ok := cfg.Servers["shim"]; ok { - return false, 0 - } + if utilityCommandDeferredToServer(cfg, "shim") { + return false, 0 } return true, runShimCommandWithConfig(args[1:], cfg, stdout, stderr) @@ -468,7 +467,43 @@ func shimServerKnown(server string, cfg *config.Config) (bool, error) { // as a pure pass-through wrapper. return true, nil } - return containsServerName(known, server), nil + if containsServerName(known, server) { + return true, nil + } + + // list_servers is config-only and omits Codex virtual servers. Explicit + // install targets still resolve via list_tools rather than ServerNames. + resolved, err := shimResolveServerFn(server) + if err != nil { + return true, nil + } + return resolved, nil +} + +func resolveShimServerViaListTools(server string) (bool, error) { + nonce, err := spawnOrConnectFn() + if err != nil { + return false, err + } + client := newDaemonClient(ipc.SocketPath(), nonce) + resp, err := client.Send(&ipc.Request{ + Type: "list_tools", + Server: server, + CWD: callerWorkingDirectory(), + }) + if err != nil { + return false, err + } + if isUnknownServerResponse(resp, server) { + return false, nil + } + if resp.ExitCode != ipc.ExitOK { + if resp.Stderr != "" { + return false, errors.New(resp.Stderr) + } + return false, fmt.Errorf("resolving server %q failed (exit %d)", server, resp.ExitCode) + } + return true, nil } func listShimKnownServers() ([]string, error) { diff --git a/internal/cli/shim_test.go b/internal/cli/shim_test.go index 6a8d642..22afb3f 100644 --- a/internal/cli/shim_test.go +++ b/internal/cli/shim_test.go @@ -54,10 +54,17 @@ func TestMaybeHandleShimCommandInstallRejectsUnknownServer(t *testing.T) { t.Setenv("PATH", tmp) oldKnownServersFn := shimKnownServersFn - defer func() { shimKnownServersFn = oldKnownServersFn }() + oldResolveFn := shimResolveServerFn + defer func() { + shimKnownServersFn = oldKnownServersFn + shimResolveServerFn = oldResolveFn + }() shimKnownServersFn = func() ([]string, error) { return []string{"known-server"}, nil } + shimResolveServerFn = func(string) (bool, error) { + return false, nil + } cfg := &config.Config{Servers: map[string]config.ServerConfig{}} var out bytes.Buffer @@ -350,7 +357,14 @@ func TestPrintShimRemoveAndListHelpIncludeHelpFlag(t *testing.T) { func TestShimServerKnownHandlesConfiguredDiscoveredAndDiscoveryError(t *testing.T) { oldKnownServersFn := shimKnownServersFn - defer func() { shimKnownServersFn = oldKnownServersFn }() + oldResolveFn := shimResolveServerFn + defer func() { + shimKnownServersFn = oldKnownServersFn + shimResolveServerFn = oldResolveFn + }() + shimResolveServerFn = func(string) (bool, error) { + return false, nil + } known, err := shimServerKnown("github", nil) if err != nil { @@ -763,3 +777,74 @@ func TestRunShimListCommandReportsNoEntries(t *testing.T) { t.Fatalf("stderr = %q, want empty", errOut.String()) } } + +func TestShimServerKnownResolvesVirtualServerWhenListOmitsIt(t *testing.T) { + oldKnownServersFn := shimKnownServersFn + oldResolveFn := shimResolveServerFn + defer func() { + shimKnownServersFn = oldKnownServersFn + shimResolveServerFn = oldResolveFn + }() + + shimKnownServersFn = func() ([]string, error) { + return []string{"github"}, nil + } + var resolved []string + shimResolveServerFn = func(server string) (bool, error) { + resolved = append(resolved, server) + return server == "linear", nil + } + + cfg := &config.Config{Servers: map[string]config.ServerConfig{"codex_apps": {}}} + known, err := shimServerKnown("linear", cfg) + if err != nil { + t.Fatalf("shimServerKnown(linear) error = %v", err) + } + if !known { + t.Fatal("shimServerKnown(linear) = false, want true via Resolve") + } + if len(resolved) != 1 || resolved[0] != "linear" { + t.Fatalf("resolve calls = %#v, want [linear]", resolved) + } + + known, err = shimServerKnown("ghost", cfg) + if err != nil { + t.Fatalf("shimServerKnown(ghost) error = %v", err) + } + if known { + t.Fatal("shimServerKnown(ghost) = true, want false") + } +} + +func TestMaybeHandleShimCommandInstallResolvesVirtualServerWhenListOmitsIt(t *testing.T) { + tmp := t.TempDir() + t.Setenv("PATH", tmp) + + oldKnownServersFn := shimKnownServersFn + oldResolveFn := shimResolveServerFn + defer func() { + shimKnownServersFn = oldKnownServersFn + shimResolveServerFn = oldResolveFn + }() + shimKnownServersFn = func() ([]string, error) { + return []string{"github"}, nil + } + shimResolveServerFn = func(server string) (bool, error) { + return server == "linear", nil + } + + cfg := &config.Config{Servers: map[string]config.ServerConfig{"codex_apps": {}}} + var out bytes.Buffer + var errOut bytes.Buffer + + handled, code := maybeHandleShimCommand([]string{"shim", "install", "linear", "--dir", tmp}, cfg, &out, &errOut) + if !handled { + t.Fatal("handled = false, want true") + } + if code != ipc.ExitOK { + t.Fatalf("code = %d, want %d (stderr=%q)", code, ipc.ExitOK, errOut.String()) + } + if !strings.Contains(out.String(), `Installed shim "linear"`) { + t.Fatalf("stdout = %q, want install confirmation", out.String()) + } +} diff --git a/internal/cli/skill.go b/internal/cli/skill.go index f84b7c4..a3f6459 100644 --- a/internal/cli/skill.go +++ b/internal/cli/skill.go @@ -36,10 +36,8 @@ func maybeHandleSkillCommand(args []string, cfg *config.Config, stdout, stderr i return false, 0 } - if cfg != nil { - if _, ok := cfg.Servers["skill"]; ok { - return false, 0 - } + if utilityCommandDeferredToServer(cfg, "skill") { + return false, 0 } return true, runSkillCommand(args[1:], stdout, stderr) diff --git a/internal/config/fallback.go b/internal/config/fallback.go index f123445..5d876fc 100644 --- a/internal/config/fallback.go +++ b/internal/config/fallback.go @@ -151,9 +151,11 @@ func FailedFallbackSourcePaths(err error) []string { } // MergeFallbackServers fills cfg.Servers from external MCP fallback sources. -// If cfg already has managed servers, fallback sources are not merged. +// If cfg already has managed servers loaded from the mcpx config file +// (origin kind MCPXConfig), fallback sources are not merged. // When merging, managed entries already present in cfg.Servers always win -// over discovered ones of the same name. +// over discovered ones of the same name. Reserved utility command names +// are omitted from discovered servers so they cannot shadow CLI commands. func MergeFallbackServers(cfg *Config) error { return MergeFallbackServersForCWD(cfg, "") } @@ -161,12 +163,14 @@ func MergeFallbackServers(cfg *Config) error { // MergeFallbackServersForCWD is like MergeFallbackServers but resolves // Claude Code project entries in home-level configs against cwd. // When cwd is empty, it falls back to the process working directory. -// Fallback discovery is skipped entirely when managed servers already exist. +// Fallback discovery is skipped entirely when managed mcpx-config servers +// already exist. Non-empty cfg.Servers from a previous fallback merge is +// not enough to skip; those discovered servers must still be refreshable. func MergeFallbackServersForCWD(cfg *Config, cwd string) error { if cfg == nil { return nil } - if len(cfg.Servers) > 0 { + if hasManagedConfigServers(cfg) { return nil } @@ -179,6 +183,9 @@ func MergeFallbackServersForCWD(cfg *Config, cwd string) error { cfg.ServerOrigins = make(map[string]ServerOrigin) } for name, resolved := range fallback { + if isReservedFallbackServerName(name) { + continue + } if _, exists := cfg.Servers[name]; exists { continue } @@ -219,6 +226,9 @@ func loadFallbackServersWithSourcesForCWD(paths []string, cwd string) (map[strin } for name, srv := range found { + if isReservedFallbackServerName(name) { + continue + } if _, exists := servers[name]; exists { continue } @@ -594,14 +604,47 @@ func isWithinPath(path, root string) bool { return strings.HasPrefix(path, root+string(os.PathSeparator)) } +// hasManagedConfigServers reports whether cfg already contains servers loaded +// from the mcpx config file. After a fallback merge, cfg.Servers is non-empty +// from discovered servers; those must still be watched and are not "managed". +func hasManagedConfigServers(cfg *Config) bool { + if cfg == nil { + return false + } + for name := range cfg.Servers { + origin, ok := cfg.ServerOrigins[name] + if !ok { + continue + } + if NormalizeServerOrigin(origin).Kind == ServerOriginKindMCPXConfig { + return true + } + } + return false +} + +func isReservedFallbackServerName(name string) bool { + switch name { + case "add", "shim", "skill", "completion", "__complete": + return true + default: + return false + } +} + func fallbackSourcePaths(cfg *Config) []string { return fallbackSourcePathsForCWD(cfg, "") } // RuntimeConfigSourcePathsForCWD returns the ordered file paths that can affect // the runtime config for the given working directory. +// Fallback, auth, and OAuth files are omitted when managed mcpx-config servers +// already exist, matching MergeFallbackServersForCWD's skip-merge predicate. func RuntimeConfigSourcePathsForCWD(cfg *Config, cwd string) []string { sourcePaths := []string{paths.ConfigFile()} + if hasManagedConfigServers(cfg) { + return compactPaths(sourcePaths) + } for _, sourcePath := range fallbackSourcePathsForCWD(cfg, cwd) { sourcePath = strings.TrimSpace(sourcePath) if sourcePath == "" { diff --git a/internal/config/fallback_test.go b/internal/config/fallback_test.go index 26f3448..86b5815 100644 --- a/internal/config/fallback_test.go +++ b/internal/config/fallback_test.go @@ -843,3 +843,132 @@ func TestReadCodexAuthTokensParsesAndValidatesDocument(t *testing.T) { t.Fatalf("readCodexAuthTokens(valid) accountID = %q, want %q", accountID, "acct-9") } } + +func TestHasManagedConfigServersRequiresMCPXConfigOrigin(t *testing.T) { + if hasManagedConfigServers(nil) { + t.Fatal("hasManagedConfigServers(nil) = true, want false") + } + if hasManagedConfigServers(&Config{Servers: map[string]ServerConfig{"github": {}}}) { + t.Fatal("hasManagedConfigServers(servers without origins) = true, want false") + } + + fallbackOnly := &Config{ + Servers: map[string]ServerConfig{ + "github": {Command: "npx"}, + }, + ServerOrigins: map[string]ServerOrigin{ + "github": NewServerOrigin(ServerOriginKindCursor, "/tmp/.cursor/mcp.json"), + }, + } + if hasManagedConfigServers(fallbackOnly) { + t.Fatal("hasManagedConfigServers(cursor origin) = true, want false") + } + + managed := &Config{ + Servers: map[string]ServerConfig{ + "github": {Command: "echo"}, + }, + ServerOrigins: map[string]ServerOrigin{ + "github": NewServerOrigin(ServerOriginKindMCPXConfig, "/tmp/config.toml"), + }, + } + if !hasManagedConfigServers(managed) { + t.Fatal("hasManagedConfigServers(mcpx_config origin) = false, want true") + } +} + +func TestRuntimeConfigSourcePathsOmitFallbacksWhenManagedServersExist(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + xdg := filepath.Join(home, "xdg-config") + t.Setenv("XDG_CONFIG_HOME", xdg) + + fallbackPath := filepath.Join(home, ".cursor", "mcp.json") + if err := os.MkdirAll(filepath.Dir(fallbackPath), 0700); err != nil { + t.Fatalf("mkdir fallback dir: %v", err) + } + if err := os.WriteFile(fallbackPath, []byte(`{"mcpServers":{"cursor":{"command":"npx"}}}`), 0600); err != nil { + t.Fatalf("write fallback file: %v", err) + } + + managed := &Config{ + Servers: map[string]ServerConfig{ + "github": {Command: "echo"}, + }, + ServerOrigins: map[string]ServerOrigin{ + "github": NewServerOrigin(ServerOriginKindMCPXConfig, "/tmp/config.toml"), + }, + FallbackSources: []string{fallbackPath}, + } + got := RuntimeConfigSourcePathsForCWD(managed, home) + for _, path := range got { + if path == fallbackPath { + t.Fatalf("RuntimeConfigSourcePathsForCWD(managed) = %#v, did not want fallback path", got) + } + } + + empty := &Config{Servers: map[string]ServerConfig{}, FallbackSources: []string{fallbackPath}} + got = RuntimeConfigSourcePathsForCWD(empty, home) + found := false + for _, path := range got { + if path == fallbackPath { + found = true + break + } + } + if !found { + t.Fatalf("RuntimeConfigSourcePathsForCWD(empty) = %#v, want fallback path %q", got, fallbackPath) + } + + afterMerge := &Config{ + Servers: map[string]ServerConfig{ + "cursor": {Command: "npx"}, + }, + ServerOrigins: map[string]ServerOrigin{ + "cursor": NewServerOrigin(ServerOriginKindCursor, fallbackPath), + }, + FallbackSources: []string{fallbackPath}, + } + got = RuntimeConfigSourcePathsForCWD(afterMerge, home) + found = false + for _, path := range got { + if path == fallbackPath { + found = true + break + } + } + if !found { + t.Fatalf("RuntimeConfigSourcePathsForCWD(fallback-only) = %#v, want fallback path still watched", got) + } +} + +func TestMergeFallbackServersOmitsReservedUtilityNames(t *testing.T) { + customPath := filepath.Join(t.TempDir(), "custom-mcp.json") + raw := []byte(`{"mcpServers":{ + "add":{"command":"false"}, + "shim":{"command":"false"}, + "skill":{"command":"false"}, + "completion":{"command":"false"}, + "__complete":{"command":"false"}, + "github":{"command":"npx"} + }}`) + if err := os.WriteFile(customPath, raw, 0600); err != nil { + t.Fatalf("write fallback file: %v", err) + } + + cfg := &Config{ + Servers: map[string]ServerConfig{}, + FallbackSources: []string{customPath}, + } + if err := MergeFallbackServers(cfg); err != nil { + t.Fatalf("MergeFallbackServers() error = %v", err) + } + if _, ok := cfg.Servers["github"]; !ok { + t.Fatalf("cfg.Servers = %#v, want github", cfg.Servers) + } + for _, name := range []string{"add", "shim", "skill", "completion", "__complete"} { + if _, ok := cfg.Servers[name]; ok { + t.Fatalf("cfg.Servers unexpectedly contains reserved name %q: %#v", name, cfg.Servers) + } + } +} diff --git a/internal/daemon/runtime_config_test.go b/internal/daemon/runtime_config_test.go index 7c398a7..1616ef6 100644 --- a/internal/daemon/runtime_config_test.go +++ b/internal/daemon/runtime_config_test.go @@ -2418,3 +2418,45 @@ func TestRememberRuntimeEphemeralServerDoesNotClosePersistentNameCollision(t *te t.Fatalf("poolClose called for persistent collision entry: %#v", closed) } } + +func TestCurrentRuntimeConfigStampIgnoresFallbackEditsWhenManagedServersExist(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + t.Setenv("CODEX_HOME", filepath.Join(home, ".codex")) + + fallbackPath := filepath.Join(home, ".cursor", "mcp.json") + if err := os.MkdirAll(filepath.Dir(fallbackPath), 0700); err != nil { + t.Fatalf("mkdir fallback dir: %v", err) + } + if err := os.WriteFile(fallbackPath, []byte(`{"mcpServers":{"cursor":{"command":"one"}}}`), 0600); err != nil { + t.Fatalf("write fallback file: %v", err) + } + + managed := &config.Config{ + Servers: map[string]config.ServerConfig{ + "github": {Command: "echo"}, + }, + ServerOrigins: map[string]config.ServerOrigin{ + "github": config.NewServerOrigin(config.ServerOriginKindMCPXConfig, paths.ConfigFile()), + }, + } + before := currentRuntimeConfigStamp(managed, home) + if err := os.WriteFile(fallbackPath, []byte(`{"mcpServers":{"cursor":{"command":"two"}}}`), 0600); err != nil { + t.Fatalf("rewrite fallback file: %v", err) + } + after := currentRuntimeConfigStamp(managed, home) + if before != after { + t.Fatalf("stamp changed after fallback edit with managed servers: %q -> %q", before.Digest, after.Digest) + } + + empty := &config.Config{Servers: map[string]config.ServerConfig{}} + beforeEmpty := currentRuntimeConfigStamp(empty, home) + if err := os.WriteFile(fallbackPath, []byte(`{"mcpServers":{"cursor":{"command":"three"}}}`), 0600); err != nil { + t.Fatalf("rewrite fallback file: %v", err) + } + afterEmpty := currentRuntimeConfigStamp(empty, home) + if beforeEmpty == afterEmpty { + t.Fatal("stamp did not change after fallback edit with empty config.toml") + } +}