From fc2c3baf11eec384377d09ac544a011ca3697713 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Tue, 30 Jun 2026 09:11:01 +0200 Subject: [PATCH] fix(mcp): align Claude Code user/project MCP command strings The user-scope ~/.claude.json gortex stanza was written with an absolute os.Executable() path while the project .mcp.json template uses the bare "gortex" command. Claude Code stores OAuth tokens per endpoint (command + args), so the two differing strings trip its "conflicting scopes" diagnostic. - Add agents.ResolveGortexCommand: prefer bare "gortex" when it resolves on PATH to the same running binary, falling back to the absolute path only when gortex is not on PATH. The global install now matches the portable project template. - Heal stale stanzas: upsertGlobalMCPConfig uses UpsertMCPServerWithMigration, and IsGortexAuthoredMCPEntry now recognizes the legacy absolute-path form so an older entry is rewritten in place instead of left behind. - Project-mode init warns and skips writing .mcp.json when gortex is already registered at user scope (override with --force), and warns about the duplication when a .mcp.json already exists. --- internal/agents/claudecode/adapter.go | 73 ++++++++++--- internal/agents/claudecode/adapter_test.go | 62 +++++++++++ internal/agents/mcp.go | 107 ++++++++++++++++++- internal/agents/mcp_test.go | 116 +++++++++++++++++++++ 4 files changed, 339 insertions(+), 19 deletions(-) diff --git a/internal/agents/claudecode/adapter.go b/internal/agents/claudecode/adapter.go index 2bf70a33..3b52cba9 100644 --- a/internal/agents/claudecode/adapter.go +++ b/internal/agents/claudecode/adapter.go @@ -1,6 +1,7 @@ package claudecode import ( + "encoding/json" "fmt" "io" "os" @@ -111,11 +112,28 @@ func (a *Adapter) Apply(env agents.Env, opts agents.ApplyOpts) (*agents.Result, } // 1. Project .mcp.json — create if absent, skip otherwise. - mcpAction, err := agents.WriteIfNotExists(w, filepath.Join(env.Root, ".mcp.json"), ProjectMCPJSON, opts) - if err != nil { - return res, fmt.Errorf(".mcp.json: %w", err) + // + // If gortex is already registered at user scope (~/.claude.json), a + // project .mcp.json adds a second registration under the same name. + // Claude Code keys OAuth tokens per endpoint and flags this as a + // "conflicting scopes" diagnostic. The user-scope entry already + // serves this repo machine-wide, so skip the project file unless + // --force. (A pre-existing .mcp.json is left in place — we never + // delete it — but we warn so the user can resolve the duplication.) + mcpPath := filepath.Join(env.Root, ".mcp.json") + if !opts.Force && env.Home != "" && userScopeGortexRegistered(env.Home) && !pathExists(mcpPath) { + logWarn(w, "gortex is already registered at user scope (%s); skipping project .mcp.json to avoid a Claude Code \"conflicting scopes\" warning. Re-run with --force to write it anyway (e.g. for teammates without a global install).", userClaudeJSONPath(env.Home)) + res.Files = append(res.Files, agents.FileAction{Path: mcpPath, Action: agents.ActionSkip, Reason: "gortex already registered at user scope"}) + } else { + if !opts.Force && env.Home != "" && userScopeGortexRegistered(env.Home) && pathExists(mcpPath) { + logWarn(w, "gortex is registered at both user scope (%s) and project scope (%s); Claude Code may warn about conflicting scopes — keep one with `claude mcp remove gortex -s user` or `-s project`.", userClaudeJSONPath(env.Home), mcpPath) + } + mcpAction, err := agents.WriteIfNotExists(w, mcpPath, ProjectMCPJSON, opts) + if err != nil { + return res, fmt.Errorf(".mcp.json: %w", err) + } + res.Files = append(res.Files, mcpAction) } - res.Files = append(res.Files, mcpAction) // 2. MCP permissions in .claude/settings.json — merge, not create. permAction, err := installPermissions(w, filepath.Join(env.Root, ".claude", "settings.json"), opts) @@ -602,23 +620,26 @@ func installGlobalSubAgents(w io.Writer, home string, opts agents.ApplyOpts) ([] // existing file is malformed JSON, it's backed up before we // overwrite. func upsertGlobalMCPConfig(w io.Writer, path string, opts agents.ApplyOpts) (agents.FileAction, error) { - exe, err := os.Executable() - if err != nil { - // Fall back to bare "gortex" on PATH. Reasonable for - // homebrew / go install deployments. - exe = "gortex" - } + // Prefer the bare "gortex" command when it resolves on PATH to the + // binary we're running, so this user-scope entry matches the + // portable project .mcp.json template byte-for-byte. Claude Code + // keys OAuth tokens per endpoint, so a user-scope stanza that + // disagrees with a project-scope one trips its "conflicting scopes" + // diagnostic. Falls back to the absolute path only when gortex is + // not on PATH (e.g. a Windows install whose dir isn't on PATH). entry := map[string]any{ - "command": exe, + "command": agents.ResolveGortexCommand(), "args": []string{"mcp"}, "env": map[string]any{}, } - // Try a direct merge first. MergeJSON handles malformed JSON - // with a timestamped backup already. + // Try a direct merge first. MergeJSON handles malformed JSON with a + // timestamped backup already. UpsertMCPServerWithMigration rewrites + // a stale Gortex-authored stanza (including the older absolute-path + // form) in place without clobbering a user's hand-rolled wrapper. action, err := agents.MergeJSON(w, path, func(root map[string]any, existed bool) (bool, error) { _ = existed - return agents.UpsertMCPServer(root, "gortex", entry, agents.ApplyOpts{Force: opts.Force}), nil + return agents.UpsertMCPServerWithMigration(root, "gortex", entry, agents.ApplyOpts{Force: opts.Force}), nil }, opts) if err != nil { return agents.FileAction{}, err @@ -637,6 +658,30 @@ func upsertGlobalMCPConfig(w io.Writer, path string, opts agents.ApplyOpts) (age return action, nil } +// userScopeGortexRegistered reports whether ~/.claude.json already +// registers a "gortex" MCP server at user scope. A project .mcp.json +// written on top of that produces a second registration under the same +// name, which Claude Code flags as a "conflicting scopes" warning +// because it stores OAuth tokens per endpoint. A missing or malformed +// file is treated as "not registered" — we only suppress the project +// write on positive evidence of a user-scope entry. +func userScopeGortexRegistered(home string) bool { + data, err := os.ReadFile(userClaudeJSONPath(home)) + if err != nil { + return false + } + var root map[string]any + if err := json.Unmarshal(data, &root); err != nil { + return false + } + servers, ok := root["mcpServers"].(map[string]any) + if !ok { + return false + } + _, ok = servers["gortex"] + return ok +} + // Paths — user-level files. func userClaudeJSONPath(home string) string { diff --git a/internal/agents/claudecode/adapter_test.go b/internal/agents/claudecode/adapter_test.go index a39610a0..fdb012dd 100644 --- a/internal/agents/claudecode/adapter_test.go +++ b/internal/agents/claudecode/adapter_test.go @@ -440,3 +440,65 @@ func TestClaudeCodeDryRunWritesNothing(t *testing.T) { } } } + +// TestProjectModeSkipsMCPWhenUserScopeRegistered is the regression +// guard for issue #201: a project `gortex init` must NOT create a +// project .mcp.json when gortex is already registered at user scope — +// that double-registration is what trips Claude Code's "conflicting +// scopes" diagnostic. --force overrides the skip (so a maintainer can +// still commit a .mcp.json for teammates without a global install). +func TestProjectModeSkipsMCPWhenUserScopeRegistered(t *testing.T) { + SetConfigDirOverride("") + + env, buf := agentstest.NewEnv(t) + a := New() + + // Seed a user-scope gortex registration in ~/.claude.json. + claudeJSON := userClaudeJSONPath(env.Home) + if err := os.MkdirAll(filepath.Dir(claudeJSON), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + seed := `{"mcpServers":{"gortex":{"command":"gortex","args":["mcp"],"env":{}}}}` + if err := os.WriteFile(claudeJSON, []byte(seed), 0o644); err != nil { + t.Fatalf("seed user config: %v", err) + } + + mcpPath := filepath.Join(env.Root, ".mcp.json") + + // Without --force: the project .mcp.json must be skipped and the + // user warned. + if _, err := a.Apply(env, agents.ApplyOpts{}); err != nil { + t.Fatalf("apply: %v", err) + } + if _, err := os.Stat(mcpPath); err == nil { + t.Errorf("project .mcp.json was written despite a user-scope gortex registration") + } + if !strings.Contains(buf.String(), "already registered at user scope") { + t.Errorf("expected a conflicting-scopes warning, got: %q", buf.String()) + } + + // With --force: the project .mcp.json is written anyway. + if _, err := a.Apply(env, agents.ApplyOpts{Force: true}); err != nil { + t.Fatalf("apply --force: %v", err) + } + if _, err := os.Stat(mcpPath); err != nil { + t.Errorf("--force should still write project .mcp.json: %v", err) + } +} + +// TestProjectModeWritesMCPWhenNoUserScope confirms the common path is +// unchanged: with no user-scope gortex registration, the project +// .mcp.json is created as before. +func TestProjectModeWritesMCPWhenNoUserScope(t *testing.T) { + SetConfigDirOverride("") + + env, _ := agentstest.NewEnv(t) + a := New() + + if _, err := a.Apply(env, agents.ApplyOpts{}); err != nil { + t.Fatalf("apply: %v", err) + } + if _, err := os.Stat(filepath.Join(env.Root, ".mcp.json")); err != nil { + t.Errorf("project .mcp.json should be written when no user-scope entry exists: %v", err) + } +} diff --git a/internal/agents/mcp.go b/internal/agents/mcp.go index 2ecabcb2..1c0b83d7 100644 --- a/internal/agents/mcp.go +++ b/internal/agents/mcp.go @@ -2,7 +2,11 @@ package agents import ( "encoding/json" + "os" + "os/exec" + "path/filepath" "reflect" + "strings" ) // UpsertMCPServer merges a gortex-flavored MCP server stanza into a @@ -79,17 +83,18 @@ func UpsertMCPServerWithMigration(root map[string]any, serverName string, entry } // IsGortexAuthoredMCPEntry returns true for MCP server stanzas that -// look like Gortex wrote them — `command == "gortex"` and the args -// list starts with the `mcp` subcommand. Used by global-mode -// installers to migrate their own legacy stanzas without clobbering -// user-customized servers. +// look like Gortex wrote them — a command naming the gortex binary +// (bare "gortex" or an absolute path whose basename is gortex) and an +// args list starting with the `mcp` subcommand. Used by global-mode +// installers to migrate their own legacy stanzas — including the older +// absolute-path form — without clobbering user-customized servers. func IsGortexAuthoredMCPEntry(entry any) bool { m, ok := entry.(map[string]any) if !ok { return false } cmd, _ := m["command"].(string) - if cmd != "gortex" { + if !commandIsGortex(cmd) { return false } args, ok := m["args"].([]any) @@ -100,6 +105,98 @@ func IsGortexAuthoredMCPEntry(entry any) bool { return first == "mcp" } +// commandIsGortex reports whether an MCP stanza's command string names +// the gortex binary — either the bare "gortex"/"gortex.exe" name or an +// absolute path whose basename is gortex (the legacy os.Executable() +// form a pre-fix `gortex install` baked into ~/.claude.json). Lets the +// global installer recognize and migrate its own older stanza in place +// instead of leaving a stale absolute path that disagrees with the +// bare-`gortex` project .mcp.json template. +func commandIsGortex(cmd string) bool { + return gortexCommandBase(cmd) == "gortex" +} + +// gortexCommandBase extracts the binary base name from a command +// string, tolerating both / and \ separators so a path written on one +// OS is still recognized when parsed on another (matters for +// cross-platform tests; in production the path is always native). The +// trailing extension (.exe on Windows) is stripped. +func gortexCommandBase(cmd string) string { + if cmd == "" { + return "" + } + base := cmd + if i := strings.LastIndexAny(base, `/\`); i >= 0 { + base = base[i+1:] + } + return strings.TrimSuffix(base, filepath.Ext(base)) +} + +// ResolveGortexCommand returns the command string an installer should +// bake into a gortex MCP server stanza. It prefers the bare "gortex" +// name — portable across machines and byte-identical to the project +// .mcp.json template — but only when "gortex" on PATH resolves to the +// same binary that is currently running. Matching the project template +// matters for Claude Code specifically: it stores OAuth tokens per +// endpoint (command + args), so a user-scope entry that disagrees with +// a project-scope entry trips its "conflicting scopes" diagnostic. +// +// When the running binary is not reachable on PATH (e.g. a Windows +// install whose program directory is not on PATH) it falls back to the +// absolute os.Executable() path so the entry still launches. Under +// `go run` (a transient temp build) or any other ambiguity it falls +// back to the bare name rather than bake a path that won't exist later. +func ResolveGortexCommand() string { + exe, exeErr := os.Executable() + lp, lpErr := exec.LookPath("gortex") + return resolveGortexCommandFrom(exe, exeErr, lp, lpErr, sameFile) +} + +// resolveGortexCommandFrom is the pure decision core of +// ResolveGortexCommand, split out so the PATH/executable inputs can be +// injected in tests. +func resolveGortexCommandFrom(exe string, exeErr error, lookPath string, lookErr error, same func(a, b string) bool) string { + exeUsable := exeErr == nil && exe != "" && gortexCommandBase(exe) == "gortex" && !isUnderTempDir(exe) + if lookErr == nil && lookPath != "" { + // On PATH: collapse to the bare name when it points at the + // binary we are running (or we cannot trust os.Executable), + // so the entry matches the portable project template. + if !exeUsable || same(lookPath, exe) { + return "gortex" + } + } + if exeUsable { + // Not on PATH, but we know exactly where we live: pin the + // absolute path so the entry launches. + return exe + } + return "gortex" +} + +// isUnderTempDir reports whether p lives under the OS temp directory — +// the tell-tale of a `go run` / `go test` transient build that must not +// be baked into a long-lived config. +func isUnderTempDir(p string) bool { + return strings.HasPrefix(p, filepath.Clean(os.TempDir())+string(os.PathSeparator)) +} + +// sameFile reports whether two paths reference the same on-disk binary, +// resolving symlinks and falling back to os.SameFile. A pure string +// match short-circuits the stat calls. +func sameFile(a, b string) bool { + if a == b { + return true + } + if ra, err := filepath.EvalSymlinks(a); err == nil { + if rb, err := filepath.EvalSymlinks(b); err == nil && ra == rb { + return true + } + } + fa, e1 := os.Stat(a) + fb, e2 := os.Stat(b) + return e1 == nil && e2 == nil && os.SameFile(fa, fb) +} + // MCPEntriesEqual compares two MCP stanzas by their JSON-marshaled // form. Round-tripping is the simplest way to handle the []string vs // []any drift between freshly-built entries and entries decoded from diff --git a/internal/agents/mcp_test.go b/internal/agents/mcp_test.go index 2105d557..9fdd159e 100644 --- a/internal/agents/mcp_test.go +++ b/internal/agents/mcp_test.go @@ -2,6 +2,9 @@ package agents import ( "encoding/json" + "errors" + "os" + "path/filepath" "testing" ) @@ -159,3 +162,116 @@ func TestUpsertMCPServerWithMigrationPreservesUserCustomization(t *testing.T) { t.Errorf("opts.Force must overwrite user-customized entry") } } + +// TestIsGortexAuthoredMCPEntryAbsolutePath verifies the broadened +// detection: the legacy absolute-path command form a pre-fix `gortex +// install` baked into ~/.claude.json (via os.Executable()) is now +// recognized as Gortex-authored, so the next install can migrate it to +// the canonical bare-`gortex` shape. A user's own wrapper script is +// still left alone. Both / and \ separators are handled regardless of +// the host OS. +func TestIsGortexAuthoredMCPEntryAbsolutePath(t *testing.T) { + cases := []struct { + name string + raw string + want bool + }{ + { + name: "unix absolute path to gortex (legacy os.Executable form)", + raw: `{"command":"/usr/local/bin/gortex","args":["mcp"]}`, + want: true, + }, + { + name: "windows absolute path to gortex.exe (the issue #201 form)", + raw: `{"command":"C:\\Users\\daoti\\AppData\\Local\\Programs\\gortex\\gortex.exe","args":["mcp"]}`, + want: true, + }, + { + name: "absolute path to a non-gortex binary", + raw: `{"command":"/usr/local/bin/something-else","args":["mcp"]}`, + want: false, + }, + { + name: "user wrapper whose basename merely contains gortex", + raw: `{"command":"/opt/scripts/my-gortex.sh","args":["mcp"]}`, + want: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var v any + if err := json.Unmarshal([]byte(tc.raw), &v); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := IsGortexAuthoredMCPEntry(v); got != tc.want { + t.Errorf("IsGortexAuthoredMCPEntry(%s) = %v, want %v", tc.raw, got, tc.want) + } + }) + } +} + +// TestResolveGortexCommandFrom pins the command-resolution policy that +// keeps the user-scope ~/.claude.json entry in agreement with the +// project .mcp.json template (and so avoids Claude Code's "conflicting +// scopes" diagnostic) while staying robust when gortex isn't on PATH. +func TestResolveGortexCommandFrom(t *testing.T) { + sameTrue := func(a, b string) bool { return true } + sameFalse := func(a, b string) bool { return false } + notFound := errors.New("executable file not found in $PATH") + + cases := []struct { + name string + exe string + exeErr error + lookPath string + lookErr error + same func(a, b string) bool + want string + }{ + { + name: "on PATH and same binary -> bare gortex (matches project template)", + exe: "/usr/local/bin/gortex", + lookPath: "/usr/local/bin/gortex", + same: sameTrue, + want: "gortex", + }, + { + name: "installed but not on PATH -> pin absolute path", + exe: `C:\Users\daoti\AppData\Local\Programs\gortex\gortex.exe`, + lookPath: "", + lookErr: notFound, + same: sameFalse, + want: `C:\Users\daoti\AppData\Local\Programs\gortex\gortex.exe`, + }, + { + name: "on PATH but a different gortex -> pin the running binary, not the PATH one", + exe: "/opt/build/gortex", + lookPath: "/usr/local/bin/gortex", + same: sameFalse, + want: "/opt/build/gortex", + }, + { + name: "go run transient temp build + gortex on PATH -> bare gortex", + exe: filepath.Join(os.TempDir(), "go-build1234", "agents.test"), + lookPath: "/usr/local/bin/gortex", + same: sameFalse, + want: "gortex", + }, + { + name: "nothing resolvable -> bare gortex last resort", + exe: "", + exeErr: errors.New("os.Executable failed"), + lookErr: notFound, + same: sameFalse, + want: "gortex", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := resolveGortexCommandFrom(tc.exe, tc.exeErr, tc.lookPath, tc.lookErr, tc.same) + if got != tc.want { + t.Errorf("resolveGortexCommandFrom() = %q, want %q", got, tc.want) + } + }) + } +}