From e5220fbc6516c4ef7c6a5722d0cc08be6e3a1cf9 Mon Sep 17 00:00:00 2001 From: Denver2828 Date: Wed, 29 Jul 2026 21:07:51 -0300 Subject: [PATCH] fix(mcp): register Claude Code Context7 MCP in ~/.claude.json Claude Code only reads user-scope MCP servers from ~/.claude.json. Register Context7 there with a non-destructive merge (corrupt base aborts, 0600 enforced, stale merges re-read and retry), clean the inert settings.json block best-effort, verify the registry post-install, and never delete the user registry on uninstall even when it empties. Closes #1868 --- internal/agents/claude/adapter.go | 58 +++++ internal/cli/run.go | 9 +- internal/cli/run_component_paths_test.go | 23 +- internal/cli/sync.go | 2 +- internal/components/mcp/inject.go | 97 ++++++++- internal/components/mcp/inject_test.go | 200 +++++++++++++----- internal/components/uninstall/service.go | 50 ++++- internal/components/uninstall/service_test.go | 48 +++++ 8 files changed, 414 insertions(+), 73 deletions(-) diff --git a/internal/agents/claude/adapter.go b/internal/agents/claude/adapter.go index edd227128..0ad2d0712 100644 --- a/internal/agents/claude/adapter.go +++ b/internal/agents/claude/adapter.go @@ -1,12 +1,15 @@ package claude import ( + "bytes" "context" + "fmt" "os" "os/exec" "path/filepath" "github.com/gentleman-programming/gentle-ai/v2/internal/agents/capabilitymanifest" + "github.com/gentleman-programming/gentle-ai/v2/internal/components/filemerge" "github.com/gentleman-programming/gentle-ai/v2/internal/installcmd" "github.com/gentleman-programming/gentle-ai/v2/internal/model" "github.com/gentleman-programming/gentle-ai/v2/internal/system" @@ -83,6 +86,61 @@ func (a *Adapter) InstallCommand(profile system.PlatformProfile) ([][]string, er // --- Config paths --- +// UserConfigPath returns ~/.claude.json, the only user-scope file Claude Code +// reads MCP servers from; it also carries the OAuth session — never reset it. +func UserConfigPath(homeDir string) string { + return filepath.Join(homeDir, ".claude.json") +} + +// MergeUserConfig merges overlayJSON into ~/.claude.json: an unparsable base +// aborts instead of being reset to {}, the file always ends at 0600, and a +// base that moves underneath the merge is re-read and retried (issue #1868). +func MergeUserConfig(homeDir string, overlayJSON []byte) (filemerge.WriteResult, string, error) { + configPath := UserConfigPath(homeDir) + const maxAttempts = 4 + for attempt := 1; ; attempt++ { + raw, err := readUserConfigBase(configPath) + if err != nil { + return filemerge.WriteResult{}, configPath, err + } + if _, parseErr := filemerge.UnmarshalJSONObject(raw); parseErr != nil { + return filemerge.WriteResult{}, configPath, fmt.Errorf("refusing to modify %q: it holds the Claude Code session and could not be parsed as JSON: %w", configPath, parseErr) + } + merged, err := filemerge.MergeJSONObjects(raw, overlayJSON) + if err != nil { + return filemerge.WriteResult{}, configPath, err + } + current, err := readUserConfigBase(configPath) + if err != nil { + return filemerge.WriteResult{}, configPath, err + } + if !bytes.Equal(current, raw) { + if attempt < maxAttempts { + continue + } + return filemerge.WriteResult{}, configPath, fmt.Errorf("gave up merging into %q after %d attempts: the file kept changing underneath the merge", configPath, maxAttempts) + } + writeResult, err := filemerge.WriteFileAtomic(configPath, merged, 0o600) + if err != nil { + return filemerge.WriteResult{}, configPath, err + } + // WriteFileAtomic skips byte-identical writes (and their mode); + // the OAuth-bearing file must end at 0600 regardless. + if chmodErr := os.Chmod(configPath, 0o600); chmodErr != nil { + return writeResult, configPath, fmt.Errorf("tighten mode of %q: %w", configPath, chmodErr) + } + return writeResult, configPath, nil + } +} + +func readUserConfigBase(configPath string) ([]byte, error) { + raw, err := os.ReadFile(configPath) + if err != nil && !os.IsNotExist(err) { + return nil, fmt.Errorf("read %q: %w", configPath, err) + } + return raw, nil +} + func (a *Adapter) GlobalConfigDir(homeDir string) string { return filepath.Join(homeDir, ".claude") } diff --git a/internal/cli/run.go b/internal/cli/run.go index 646cf5522..c49520502 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -14,6 +14,7 @@ import ( "time" "github.com/gentleman-programming/gentle-ai/v2/internal/agents" + "github.com/gentleman-programming/gentle-ai/v2/internal/agents/claude" codexagent "github.com/gentleman-programming/gentle-ai/v2/internal/agents/codex" "github.com/gentleman-programming/gentle-ai/v2/internal/agents/kimi" "github.com/gentleman-programming/gentle-ai/v2/internal/assets" @@ -1349,7 +1350,7 @@ func (s componentApplyStep) Run() error { case model.ComponentContext7: for _, adapter := range adapters { targetDir := componentInjectionDirScoped(s.homeDir, s.workspaceDir, s.scope, adapter) - if _, err := mcp.Inject(targetDir, adapter); err != nil { + if _, err := mcp.Inject(s.homeDir, targetDir, adapter); err != nil { return fmt.Errorf("inject context7 for %q: %w", adapter.Agent(), err) } } @@ -1828,7 +1829,11 @@ func componentPathsWithWorkspaceScoped(homeDir, workspaceDir string, scope Insta switch adapter.MCPStrategy() { case model.StrategySeparateMCPFiles: if adapter.Agent() == model.AgentClaudeCode { - if p := adapter.SettingsPath(targetDir); p != "" { + if targetDir == homeDir { + // Context7 injection writes ~/.claude.json (issue #1868). + paths = append(paths, claude.UserConfigPath(homeDir)) + } else if p := adapter.SettingsPath(targetDir); p != "" { + // Workspace scope keeps the scoped settings merge. paths = append(paths, p) } break diff --git a/internal/cli/run_component_paths_test.go b/internal/cli/run_component_paths_test.go index e30b57980..4786a2984 100644 --- a/internal/cli/run_component_paths_test.go +++ b/internal/cli/run_component_paths_test.go @@ -355,19 +355,28 @@ func TestComponentPathsContext7KimiIncludesMCPConfig(t *testing.T) { } } -func TestComponentPathsContext7ClaudeUsesSettingsFile(t *testing.T) { +// TestComponentPathsContext7ClaudeUsesUserRegistry pins Claude Context7 to +// the file injection actually writes: ~/.claude.json (issue #1868). +// settings.json is only mutated best-effort and may not exist, and the legacy +// managed ~/.claude/mcp/context7.json is removed by injection, so verifying +// either would fail on a healthy install. +func TestComponentPathsContext7ClaudeUsesUserRegistry(t *testing.T) { home := t.TempDir() adapters := resolveAdapters([]model.AgentID{model.AgentClaudeCode}) paths := componentPaths(home, model.Selection{}, adapters, model.ComponentContext7) - want := filepath.Join(home, ".claude", "settings.json") - if !containsPath(paths, want) { - t.Fatalf("componentPaths(context7,claude) missing %q\npaths=%v", want, paths) + registry := filepath.Join(home, ".claude.json") + if !containsPath(paths, registry) { + t.Fatalf("componentPaths(context7,claude) missing %q\npaths=%v", registry, paths) } - legacy := filepath.Join(home, ".claude", "mcp", "context7.json") - if containsPath(paths, legacy) { - t.Fatalf("componentPaths(context7,claude) should not verify legacy path %q\npaths=%v", legacy, paths) + for _, absent := range []string{ + filepath.Join(home, ".claude", "mcp", "context7.json"), + filepath.Join(home, ".claude", "settings.json"), + } { + if containsPath(paths, absent) { + t.Fatalf("componentPaths(context7,claude) must not require %q\npaths=%v", absent, paths) + } } } diff --git a/internal/cli/sync.go b/internal/cli/sync.go index ed9582d3e..f6bfdf1ae 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -897,7 +897,7 @@ func (s componentSyncStep) Run() error { case model.ComponentContext7: for _, adapter := range adapters { targetDir := componentInjectionDir(s.homeDir, s.workspaceDir, adapter) - res, err := mcp.Inject(targetDir, adapter) + res, err := mcp.Inject(s.homeDir, targetDir, adapter) if err != nil { return fmt.Errorf("sync context7 for %q: %w", adapter.Agent(), err) } diff --git a/internal/components/mcp/inject.go b/internal/components/mcp/inject.go index 4ac73dd1c..cfd6ef53f 100644 --- a/internal/components/mcp/inject.go +++ b/internal/components/mcp/inject.go @@ -4,8 +4,10 @@ import ( "encoding/json" "fmt" "os" + "strings" "github.com/gentleman-programming/gentle-ai/v2/internal/agents" + "github.com/gentleman-programming/gentle-ai/v2/internal/agents/claude" "github.com/gentleman-programming/gentle-ai/v2/internal/components/filemerge" "github.com/gentleman-programming/gentle-ai/v2/internal/model" "github.com/gentleman-programming/gentle-ai/v2/internal/versions" @@ -16,7 +18,12 @@ type InjectionResult struct { Files []string } -func Inject(homeDir string, adapter agents.Adapter) (InjectionResult, error) { +// Inject registers the Context7 MCP server for the adapter. targetDir is the +// scoped injection root (the home directory for user scope, the workspace for +// workspace scope). Claude Code user-scope registration goes to ~/.claude.json, +// the only user-scope file Claude Code reads MCP servers from (issue #1868); +// workspace scope keeps the scoped settings merge. +func Inject(homeDir, targetDir string, adapter agents.Adapter) (InjectionResult, error) { if !adapter.SupportsMCP() { return InjectionResult{}, nil } @@ -24,17 +31,20 @@ func Inject(homeDir string, adapter agents.Adapter) (InjectionResult, error) { switch adapter.MCPStrategy() { case model.StrategySeparateMCPFiles: if adapter.Agent() == model.AgentClaudeCode { - return injectMergeIntoSettings(homeDir, adapter) + if targetDir == homeDir { + return injectClaudeUserConfig(homeDir, adapter) + } + return injectMergeIntoSettings(targetDir, adapter) } - return injectSeparateFile(homeDir, adapter) + return injectSeparateFile(targetDir, adapter) case model.StrategyMergeIntoSettings: - return injectMergeIntoSettings(homeDir, adapter) + return injectMergeIntoSettings(targetDir, adapter) case model.StrategyMCPConfigFile: - return injectMCPConfigFile(homeDir, adapter) + return injectMCPConfigFile(targetDir, adapter) case model.StrategyTOMLFile: - return injectTOMLFile(homeDir, adapter) + return injectTOMLFile(targetDir, adapter) case model.StrategyMergeIntoYAML: - return injectYAMLFile(homeDir, adapter) + return injectYAMLFile(targetDir, adapter) default: return InjectionResult{}, fmt.Errorf("mcp injector does not support MCP strategy %d for agent %q", adapter.MCPStrategy(), adapter.Agent()) } @@ -246,6 +256,79 @@ func migrateOpenClawLegacyMCPServers(baseJSON []byte) ([]byte, error) { return append(migrated, '\n'), nil } +// injectClaudeUserConfig registers Context7 in ~/.claude.json, the only +// user-scope location Claude Code reads MCP servers from — settings.json +// silently ignores the top-level mcpServers key earlier versions wrote +// (issue #1868). +func injectClaudeUserConfig(homeDir string, adapter agents.Adapter) (InjectionResult, error) { + writeResult, configPath, err := claude.MergeUserConfig(homeDir, DefaultContext7OverlayJSON()) + if err != nil { + return InjectionResult{}, err + } + + changed := writeResult.Changed + files := []string{configPath} + // Best-effort: the block is inert, so a settings.json that cannot be + // rewritten must not fail the injection that already succeeded above. + settingsPath := adapter.SettingsPath(homeDir) + if settingsChanged, cleanupErr := removeInertSettingsMCPServers(settingsPath); cleanupErr == nil && settingsChanged { + changed = true + files = append(files, settingsPath) + } + + return InjectionResult{Changed: changed, Files: files}, nil +} + +// removeInertSettingsMCPServers deletes the inert top-level mcpServers key +// from settings.json once the real registration lives in ~/.claude.json — +// but only when the block holds nothing beyond the managed context7 entry. +// An unparsable settings file is left untouched. +// isManagedSettingsContext7Entry reports whether the inert settings.json entry +// matches the managed shape, so cleanup never deletes a user-authored server. +func isManagedSettingsContext7Entry(entry any) bool { + server, ok := entry.(map[string]any) + if !ok { + return false + } + return server["command"] == "npx" && strings.Contains(fmt.Sprint(server["args"]), "context7-mcp") +} + +func removeInertSettingsMCPServers(settingsPath string) (bool, error) { + if settingsPath == "" { + return false, nil + } + raw, err := osReadFile(settingsPath) + if err != nil { + return false, err + } + root, err := filemerge.UnmarshalJSONObject(raw) + if err != nil { + return false, nil + } + servers, ok := root["mcpServers"].(map[string]any) + if !ok { + return false, nil + } + for name, entry := range servers { + if name != "context7" || !isManagedSettingsContext7Entry(entry) { + return false, nil + } + } + delete(root, "mcpServers") + + encoded, err := json.MarshalIndent(root, "", " ") + if err != nil { + return false, fmt.Errorf("marshal cleaned settings json: %w", err) + } + + writeResult, err := filemerge.WriteFileAtomic(settingsPath, append(encoded, '\n'), 0o644) + if err != nil { + return false, err + } + + return writeResult.Changed, nil +} + // injectMCPConfigFile writes to a dedicated mcp.json config file (Cursor pattern). func injectMCPConfigFile(homeDir string, adapter agents.Adapter) (InjectionResult, error) { path := adapter.MCPConfigPath(homeDir, "context7") diff --git a/internal/components/mcp/inject_test.go b/internal/components/mcp/inject_test.go index 7d2057979..9dac310a3 100644 --- a/internal/components/mcp/inject_test.go +++ b/internal/components/mcp/inject_test.go @@ -2,8 +2,10 @@ package mcp import ( "encoding/json" + "fmt" "os" "path/filepath" + "runtime" "strings" "testing" @@ -17,6 +19,7 @@ import ( "github.com/gentleman-programming/gentle-ai/v2/internal/agents/openclaw" "github.com/gentleman-programming/gentle-ai/v2/internal/agents/opencode" "github.com/gentleman-programming/gentle-ai/v2/internal/agents/vscode" + "github.com/gentleman-programming/gentle-ai/v2/internal/versions" ) func cursorAdapter(t *testing.T) agents.Adapter { @@ -171,7 +174,7 @@ func assertKimiContext7Schema(t *testing.T, path string) { func TestInjectOpenCodeMergesContext7AndIsIdempotent(t *testing.T) { home := t.TempDir() - first, err := Inject(home, opencodeAdapter()) + first, err := Inject(home, home, opencodeAdapter()) if err != nil { t.Fatalf("Inject() first error = %v", err) } @@ -179,7 +182,7 @@ func TestInjectOpenCodeMergesContext7AndIsIdempotent(t *testing.T) { t.Fatalf("Inject() first changed = false") } - second, err := Inject(home, opencodeAdapter()) + second, err := Inject(home, home, opencodeAdapter()) if err != nil { t.Fatalf("Inject() second error = %v", err) } @@ -243,7 +246,7 @@ func TestInjectOpenClawMergesContext7UnderMCPDotServersAndMigratesLegacyMCPServe t.Fatalf("WriteFile(openclaw.json) error = %v", err) } - first, err := Inject(home, adapter) + first, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject(openclaw) first error = %v", err) } @@ -251,7 +254,7 @@ func TestInjectOpenClawMergesContext7UnderMCPDotServersAndMigratesLegacyMCPServe t.Fatalf("Inject(openclaw) first changed = false") } - second, err := Inject(home, adapter) + second, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject(openclaw) second error = %v", err) } @@ -324,7 +327,7 @@ func TestInjectOpenCodeAndKilocodePreserveContext7Headers(t *testing.T) { t.Fatalf("WriteFile(opencode.json) error = %v", err) } - first, err := Inject(home, tt.adapter) + first, err := Inject(home, home, tt.adapter) if err != nil { t.Fatalf("Inject() first error = %v", err) } @@ -352,7 +355,7 @@ func TestInjectOpenCodeAndKilocodePreserveContext7Headers(t *testing.T) { t.Fatal("opencode.json missing existing mcp.engram entry") } - second, err := Inject(home, tt.adapter) + second, err := Inject(home, home, tt.adapter) if err != nil { t.Fatalf("Inject() second error = %v", err) } @@ -388,7 +391,7 @@ func TestInjectOpenCodeAndKilocodeRecoverMalformedSettingsAndDiscardInvalidHeade t.Fatalf("WriteFile(opencode.json) error = %v", err) } - first, err := Inject(home, tt.adapter) + first, err := Inject(home, home, tt.adapter) if err != nil { t.Fatalf("Inject() first error = %v", err) } @@ -402,7 +405,7 @@ func TestInjectOpenCodeAndKilocodeRecoverMalformedSettingsAndDiscardInvalidHeade t.Fatalf("mcp.context7.headers = %#v; want invalid headers discarded", context7["headers"]) } - second, err := Inject(home, tt.adapter) + second, err := Inject(home, home, tt.adapter) if err != nil { t.Fatalf("Inject() second error = %v", err) } @@ -441,7 +444,7 @@ func TestInjectOpenCodePreservesOtherMCPEntriesWhenReplacingContext7(t *testing. t.Fatalf("WriteFile(opencode.json) error = %v", err) } - _, err := Inject(home, adapter) + _, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject() error = %v", err) } @@ -476,17 +479,16 @@ func TestInjectOpenCodePreservesOtherMCPEntriesWhenReplacingContext7(t *testing. } } -func TestInjectClaudeMergesContext7IntoSettingsAndIsIdempotent(t *testing.T) { +func TestInjectClaudeWritesUserConfigAndIsIdempotent(t *testing.T) { home := t.TempDir() - settingsPath := filepath.Join(home, ".claude", "settings.json") - if err := os.MkdirAll(filepath.Dir(settingsPath), 0o755); err != nil { - t.Fatalf("MkdirAll(settings dir) error = %v", err) - } - if err := os.WriteFile(settingsPath, []byte(`{"theme":"dark"}`), 0o644); err != nil { - t.Fatalf("WriteFile(settings) error = %v", err) + userConfigPath := filepath.Join(home, ".claude.json") + userConfig := `{"oauthAccount":{"emailAddress":"user@example.com"},"projects":{"/repo":{"allowedTools":[]}},"mcpServers":{"codegraph":{"command":"codegraph","args":["serve","--mcp"]}}}` + // Seeded intentionally loose: injection must tighten it to 0600. + if err := os.WriteFile(userConfigPath, []byte(userConfig), 0o644); err != nil { + t.Fatalf("WriteFile(user config) error = %v", err) } - first, err := Inject(home, claudeAdapter()) + first, err := Inject(home, home, claudeAdapter()) if err != nil { t.Fatalf("Inject() first error = %v", err) } @@ -494,7 +496,18 @@ func TestInjectClaudeMergesContext7IntoSettingsAndIsIdempotent(t *testing.T) { t.Fatalf("Inject() first changed = false") } - second, err := Inject(home, claudeAdapter()) + afterFirst, err := os.ReadFile(userConfigPath) + if err != nil { + t.Fatalf("ReadFile(user config after first) error = %v", err) + } + + // Loosen the mode: the no-op run must still re-tighten 0600. + if runtime.GOOS != "windows" { + if err := os.Chmod(userConfigPath, 0o644); err != nil { + t.Fatalf("Chmod(loosen) error = %v", err) + } + } + second, err := Inject(home, home, claudeAdapter()) if err != nil { t.Fatalf("Inject() second error = %v", err) } @@ -502,32 +515,113 @@ func TestInjectClaudeMergesContext7IntoSettingsAndIsIdempotent(t *testing.T) { t.Fatalf("Inject() second changed = true") } - context7 := readMCPServersContext7Entry(t, settingsPath) - if got := context7["command"]; got != "npx" { - t.Fatalf("mcpServers.context7.command = %#v; want npx", got) + raw, err := os.ReadFile(userConfigPath) + if err != nil { + t.Fatalf("ReadFile(user config) error = %v", err) + } + if string(raw) != string(afterFirst) { + t.Fatalf("second Inject() must leave ~/.claude.json byte-identical; got diff") + } + if info, statErr := os.Stat(userConfigPath); statErr != nil { + t.Fatalf("Stat(user config) error = %v", statErr) + } else if mode := info.Mode().Perm(); runtime.GOOS != "windows" && mode != 0o600 { + t.Fatalf("~/.claude.json mode = %o; want 0600 (holds the OAuth session)", mode) + } + root := map[string]any{} + if err := json.Unmarshal(raw, &root); err != nil { + t.Fatalf("Unmarshal(user config) error = %v", err) + } + if _, ok := root["oauthAccount"].(map[string]any); !ok { + t.Fatalf("oauthAccount must be preserved; got %s", raw) + } + if _, ok := root["projects"].(map[string]any); !ok { + t.Fatalf("projects must be preserved; got %s", raw) + } + servers, _ := root["mcpServers"].(map[string]any) + if codegraph, _ := servers["codegraph"].(map[string]any); codegraph["command"] != "codegraph" { + t.Fatalf("existing codegraph registration must be preserved; got %#v", servers["codegraph"]) + } + context7, _ := servers["context7"].(map[string]any) + if context7["command"] != "npx" { + t.Fatalf("mcpServers.context7.command = %#v; want npx", context7["command"]) + } + if args := fmt.Sprintf("%v", context7["args"]); !strings.Contains(args, versions.Context7MCP) { + t.Fatalf("context7.args = %s; want pinned version %s", args, versions.Context7MCP) } if _, err := os.Stat(filepath.Join(home, ".claude", "mcp", "context7.json")); !os.IsNotExist(err) { t.Fatalf("Claude Context7 must not be written to ~/.claude/mcp/context7.json; stat err = %v", err) } } -func TestInjectClaudeLeavesLegacyContext7FileForExplicitUninstallCleanup(t *testing.T) { - home := t.TempDir() - legacyPath := filepath.Join(home, ".claude", "mcp", "context7.json") - if err := os.MkdirAll(filepath.Dir(legacyPath), 0o755); err != nil { - t.Fatalf("MkdirAll(legacy dir) error = %v", err) +// TestInjectClaudeSettingsInertBlockCleanup: the inert settings.json block is +// removed when it only holds the managed context7 entry, and left untouched +// when it carries servers gentle-ai does not manage. +func TestInjectClaudeSettingsInertBlockCleanup(t *testing.T) { + cases := []struct { + name string + settings string + wantKeyRemoved bool + }{ + {"managed-only block is removed", `{"theme":"dark","mcpServers":{"context7":{"command":"npx","args":["--","context7-mcp"]}}}`, true}, + {"foreign block is left alone", `{"theme":"dark","mcpServers":{"context7":{"command":"npx","args":["--","context7-mcp"]},"stranded":{"command":"stranded-server"}}}`, false}, + {"user-authored context7 is left alone", `{"theme":"dark","mcpServers":{"context7":{"command":"my-own-proxy"}}}`, false}, } - if err := os.WriteFile(legacyPath, DefaultContext7ServerJSON(), 0o644); err != nil { - t.Fatalf("WriteFile(legacy context7) error = %v", err) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + settingsPath := filepath.Join(home, ".claude", "settings.json") + if err := os.MkdirAll(filepath.Dir(settingsPath), 0o755); err != nil { + t.Fatalf("MkdirAll(settings dir) error = %v", err) + } + if err := os.WriteFile(settingsPath, []byte(tc.settings), 0o644); err != nil { + t.Fatalf("WriteFile(settings) error = %v", err) + } + + if _, err := Inject(home, home, claudeAdapter()); err != nil { + t.Fatalf("Inject() error = %v", err) + } + + readMCPServersContext7Entry(t, filepath.Join(home, ".claude.json")) + + settingsRaw, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("ReadFile(settings) error = %v", err) + } + settings := map[string]any{} + if err := json.Unmarshal(settingsRaw, &settings); err != nil { + t.Fatalf("Unmarshal(settings) error = %v", err) + } + _, hasKey := settings["mcpServers"] + if tc.wantKeyRemoved && hasKey { + t.Fatalf("inert mcpServers key must be removed; got %s", settingsRaw) + } + if !tc.wantKeyRemoved && !hasKey { + t.Fatalf("foreign mcpServers block must be left untouched; got %s", settingsRaw) + } + if settings["theme"] != "dark" { + t.Fatalf("settings.theme = %#v; want dark preserved", settings["theme"]) + } + }) } +} - if _, err := Inject(home, claudeAdapter()); err != nil { - t.Fatalf("Inject() error = %v", err) +func TestInjectClaudeRefusesCorruptUserConfig(t *testing.T) { + home := t.TempDir() + userConfigPath := filepath.Join(home, ".claude.json") + corrupt := []byte("{ this is not json") + if err := os.WriteFile(userConfigPath, corrupt, 0o600); err != nil { + t.Fatalf("WriteFile(corrupt user config) error = %v", err) + } + if _, err := Inject(home, home, claudeAdapter()); err == nil { + t.Fatalf("Inject() error = nil; want refusal on corrupt ~/.claude.json") + } + after, err := os.ReadFile(userConfigPath) + if err != nil { + t.Fatalf("ReadFile(user config) error = %v", err) } - if _, err := os.Stat(legacyPath); err != nil { - t.Fatalf("legacy context7 file should be left for explicit uninstall cleanup: %v", err) + if string(after) != string(corrupt) { + t.Fatalf("corrupt ~/.claude.json must be left byte-identical; got %s", after) } - readMCPServersContext7Entry(t, filepath.Join(home, ".claude", "settings.json")) } func TestInjectCursorWithMalformedMCPJsonRecovery(t *testing.T) { @@ -546,7 +640,7 @@ func TestInjectCursorWithMalformedMCPJsonRecovery(t *testing.T) { t.Fatalf("WriteFile(malformed mcp.json) error = %v", err) } - result, err := Inject(home, adapter) + result, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject(cursor) with malformed mcp.json error = %v; want nil (should recover)", err) } @@ -573,7 +667,7 @@ func TestInjectCursorWithMalformedMCPJsonRecovery(t *testing.T) { func TestInjectCodexContext7TOML(t *testing.T) { home := t.TempDir() - result, err := Inject(home, codex.NewAdapter()) + result, err := Inject(home, home, codex.NewAdapter()) if err != nil { t.Fatalf("Inject(codex) error = %v", err) } @@ -611,7 +705,7 @@ func TestInjectCodexContext7TOML(t *testing.T) { func TestInjectCodexContext7Idempotent(t *testing.T) { home := t.TempDir() - first, err := Inject(home, codex.NewAdapter()) + first, err := Inject(home, home, codex.NewAdapter()) if err != nil { t.Fatalf("Inject(codex) first error = %v", err) } @@ -619,7 +713,7 @@ func TestInjectCodexContext7Idempotent(t *testing.T) { t.Fatal("Inject(codex) first changed = false; want true") } - second, err := Inject(home, codex.NewAdapter()) + second, err := Inject(home, home, codex.NewAdapter()) if err != nil { t.Fatalf("Inject(codex) second error = %v", err) } @@ -657,7 +751,7 @@ args = ["mcp", "--tools=agent"] t.Fatalf("WriteFile(config.toml) error = %v", err) } - _, err := Inject(home, codex.NewAdapter()) + _, err := Inject(home, home, codex.NewAdapter()) if err != nil { t.Fatalf("Inject(codex) error = %v", err) } @@ -697,7 +791,7 @@ args = ["mcp", "--tools=agent"] t.Fatalf("WriteFile(config.toml) error = %v", err) } - result, err := Inject(home, codex.NewAdapter()) + result, err := Inject(home, home, codex.NewAdapter()) if err != nil { t.Fatalf("Inject(codex) error = %v", err) } @@ -731,7 +825,7 @@ func TestInjectVSCodeWritesContext7ToMCPConfigFile(t *testing.T) { t.Setenv("APPDATA", filepath.Join(home, "AppData", "Roaming")) adapter := vscode.NewAdapter() - first, err := Inject(home, adapter) + first, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject() first error = %v", err) } @@ -739,7 +833,7 @@ func TestInjectVSCodeWritesContext7ToMCPConfigFile(t *testing.T) { t.Fatalf("Inject() first changed = false") } - second, err := Inject(home, adapter) + second, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject() second error = %v", err) } @@ -786,7 +880,7 @@ func TestInjectAntigravityReplacesLegacyContext7LocalConfig(t *testing.T) { t.Fatalf("WriteFile(mcp_config.json) error = %v", err) } - first, err := Inject(home, adapter) + first, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject() first error = %v", err) } @@ -796,7 +890,7 @@ func TestInjectAntigravityReplacesLegacyContext7LocalConfig(t *testing.T) { assertAntigravityContext7Schema(t, configPath) - second, err := Inject(home, adapter) + second, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject() second error = %v", err) } @@ -810,7 +904,7 @@ func TestInjectAntigravityReplacesLegacyContext7LocalConfig(t *testing.T) { func TestInjectKimiWritesContext7ToMCPConfigFile(t *testing.T) { home := t.TempDir() - first, err := Inject(home, kimiAdapter()) + first, err := Inject(home, home, kimiAdapter()) if err != nil { t.Fatalf("Inject(kimi) first error = %v", err) } @@ -818,7 +912,7 @@ func TestInjectKimiWritesContext7ToMCPConfigFile(t *testing.T) { t.Fatalf("Inject(kimi) first changed = false") } - second, err := Inject(home, kimiAdapter()) + second, err := Inject(home, home, kimiAdapter()) if err != nil { t.Fatalf("Inject(kimi) second error = %v", err) } @@ -868,7 +962,7 @@ func TestInjectKimiReplacesLegacyContext7LocalConfig(t *testing.T) { t.Fatalf("WriteFile(kimi mcp.json) error = %v", err) } - first, err := Inject(home, adapter) + first, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject(kimi) first error = %v", err) } @@ -878,7 +972,7 @@ func TestInjectKimiReplacesLegacyContext7LocalConfig(t *testing.T) { assertKimiContext7Schema(t, configPath) - second, err := Inject(home, adapter) + second, err := Inject(home, home, adapter) if err != nil { t.Fatalf("Inject(kimi) second error = %v", err) } @@ -894,7 +988,7 @@ func TestInjectKimiReplacesLegacyContext7LocalConfig(t *testing.T) { func TestInjectHermesContext7IntoYAML(t *testing.T) { home := t.TempDir() - result, err := Inject(home, hermesAdapter()) + result, err := Inject(home, home, hermesAdapter()) if err != nil { t.Fatalf("Inject(hermes) error = %v", err) } @@ -928,7 +1022,7 @@ func TestInjectHermesContext7IntoYAML(t *testing.T) { func TestInjectHermesContext7Idempotent(t *testing.T) { home := t.TempDir() - first, err := Inject(home, hermesAdapter()) + first, err := Inject(home, home, hermesAdapter()) if err != nil { t.Fatalf("Inject(hermes) first error = %v", err) } @@ -936,7 +1030,7 @@ func TestInjectHermesContext7Idempotent(t *testing.T) { t.Fatal("Inject(hermes) first changed = false") } - second, err := Inject(home, hermesAdapter()) + second, err := Inject(home, home, hermesAdapter()) if err != nil { t.Fatalf("Inject(hermes) second error = %v", err) } @@ -964,7 +1058,7 @@ func TestInjectHermesStrategyMergeIntoYAMLDispatches(t *testing.T) { home := t.TempDir() // Confirm no error is returned (the old code returned an error for strategy 4). - result, err := Inject(home, hermesAdapter()) + result, err := Inject(home, home, hermesAdapter()) if err != nil { t.Fatalf("Inject(hermes) with StrategyMergeIntoYAML returned error = %v (expected nil)", err) } @@ -992,7 +1086,7 @@ func TestInjectHermesPreservesExistingTopLevelKeys(t *testing.T) { t.Fatalf("WriteFile: %v", err) } - result, err := Inject(home, hermesAdapter()) + result, err := Inject(home, home, hermesAdapter()) if err != nil { t.Fatalf("Inject(hermes) error = %v", err) } @@ -1019,7 +1113,7 @@ func TestInjectHermesPreservesExistingTopLevelKeys(t *testing.T) { } // Second Inject must be idempotent and still preserve the original key. - second, err := Inject(home, hermesAdapter()) + second, err := Inject(home, home, hermesAdapter()) if err != nil { t.Fatalf("Inject(hermes) second error = %v", err) } diff --git a/internal/components/uninstall/service.go b/internal/components/uninstall/service.go index 3eafb9e67..7ddff8116 100644 --- a/internal/components/uninstall/service.go +++ b/internal/components/uninstall/service.go @@ -11,6 +11,7 @@ import ( "time" "github.com/gentleman-programming/gentle-ai/v2/internal/agents" + "github.com/gentleman-programming/gentle-ai/v2/internal/agents/claude" "github.com/gentleman-programming/gentle-ai/v2/internal/assets" "github.com/gentleman-programming/gentle-ai/v2/internal/backup" "github.com/gentleman-programming/gentle-ai/v2/internal/components/communitytool" @@ -722,7 +723,7 @@ func context7Targets(adapter agents.Adapter, homeDir string) []string { switch adapter.MCPStrategy() { case model.StrategySeparateMCPFiles: if adapter.Agent() == model.AgentClaudeCode { - return []string{adapter.SettingsPath(homeDir), adapter.MCPConfigPath(homeDir, "context7")} + return []string{claude.UserConfigPath(homeDir), adapter.SettingsPath(homeDir), adapter.MCPConfigPath(homeDir, "context7")} } return []string{adapter.MCPConfigPath(homeDir, "context7")} case model.StrategyMergeIntoSettings, model.StrategyMCPConfigFile: @@ -737,7 +738,7 @@ func context7Operations(adapter agents.Adapter, homeDir string) []operation { case model.StrategySeparateMCPFiles: if adapter.Agent() == model.AgentClaudeCode { legacyPath := adapter.MCPConfigPath(homeDir, "context7") - return []operation{rewriteJSONFile(adapter.SettingsPath(homeDir), jsonPath{"mcpServers", "context7"}), removeManagedContext7File(legacyPath), removeDirIfEmpty(filepath.Dir(legacyPath))} + return []operation{rewriteClaudeUserConfig(homeDir, jsonPath{"mcpServers", "context7"}), rewriteJSONFile(adapter.SettingsPath(homeDir), jsonPath{"mcpServers", "context7"}), removeManagedContext7File(legacyPath), removeDirIfEmpty(filepath.Dir(legacyPath))} } path := adapter.MCPConfigPath(homeDir, "context7") return []operation{removeFile(path), removeDirIfEmpty(filepath.Dir(path))} @@ -868,8 +869,51 @@ func rewriteJSONFile(path string, jsonPaths ...jsonPath) operation { } return true, true, nil } - _, err = filemerge.WriteFileAtomic(path, updated, 0o644) + // Preserve the file's existing mode: ~/.claude.json is injected + // with 0600 because it holds the OAuth session, and an uninstall + // rewrite must not widen it. + perm := os.FileMode(0o644) + if info, statErr := os.Lstat(path); statErr == nil { + perm = info.Mode().Perm() + } + _, err = filemerge.WriteFileAtomic(path, updated, perm) + if err != nil { + return false, false, err + } + return true, false, nil + }, + } +} + +// rewriteClaudeUserConfig removes managed entries from ~/.claude.json. Unlike +// rewriteJSONFile it never deletes the file when the result is empty: the +// registry belongs to Claude Code, so uninstall only ever writes the emptied +// object back with the file's mode preserved. +func rewriteClaudeUserConfig(homeDir string, jsonPaths ...jsonPath) operation { + path := claude.UserConfigPath(homeDir) + return operation{ + typeID: opRewriteFile, + path: path, + apply: func(path string) (bool, bool, error) { + raw, err := readManagedFile(path) + if err != nil { + if os.IsNotExist(err) { + return false, false, nil + } + return false, false, fmt.Errorf("read json file %q: %w", path, err) + } + updated, changed, err := removeJSONPaths(raw, jsonPaths...) if err != nil { + return false, false, fmt.Errorf("clean json file %q: %w", path, err) + } + if !changed { + return false, false, nil + } + perm := os.FileMode(0o600) + if info, statErr := os.Lstat(path); statErr == nil { + perm = info.Mode().Perm() + } + if _, err := filemerge.WriteFileAtomic(path, updated, perm); err != nil { return false, false, err } return true, false, nil diff --git a/internal/components/uninstall/service_test.go b/internal/components/uninstall/service_test.go index 47b211843..e26e4b013 100644 --- a/internal/components/uninstall/service_test.go +++ b/internal/components/uninstall/service_test.go @@ -4,12 +4,14 @@ import ( "encoding/json" "os" "path/filepath" + "runtime" "slices" "strings" "testing" "time" "github.com/gentleman-programming/gentle-ai/v2/internal/agents" + "github.com/gentleman-programming/gentle-ai/v2/internal/agents/claude" "github.com/gentleman-programming/gentle-ai/v2/internal/agents/codex" "github.com/gentleman-programming/gentle-ai/v2/internal/backup" "github.com/gentleman-programming/gentle-ai/v2/internal/components/communitytool" @@ -405,6 +407,52 @@ func TestComponentOperationsContext7ClaudeRemovesSettingsAndManagedLegacyFile(t } } +func TestComponentOperationsClaudeNeverDeleteUserRegistry(t *testing.T) { + homeDir := t.TempDir() + workspaceDir := t.TempDir() + + svc, err := NewService(homeDir, workspaceDir, "dev") + if err != nil { + t.Fatalf("NewService() error = %v", err) + } + adapter, ok := svc.registry.Get(model.AgentClaudeCode) + if !ok { + t.Fatal("Claude adapter not found in registry") + } + + // The registry holds ONLY the managed server, so removing it empties the + // file; ~/.claude.json must survive because Claude Code owns it. + registryPath := claude.UserConfigPath(homeDir) + seed := []byte(`{"mcpServers":{"context7":{"command":"npx"}}}`) + if err := os.WriteFile(registryPath, seed, 0o600); err != nil { + t.Fatalf("WriteFile(registry) error = %v", err) + } + + ops, _, err := svc.componentOperations(adapter, model.ComponentContext7) + if err != nil { + t.Fatalf("componentOperations(context7) error = %v", err) + } + for _, op := range ops { + if _, _, err := op.apply(op.path); err != nil { + t.Fatalf("operation %v on %q error = %v", op.typeID, op.path, err) + } + } + + info, err := os.Stat(registryPath) + if err != nil { + t.Fatalf("~/.claude.json must survive removing the last managed server: %v", err) + } + registry := readJSONFileForTest(t, registryPath) + if servers, ok := registry["mcpServers"].(map[string]any); ok { + if _, still := servers["context7"]; still { + t.Fatalf("registry still contains mcpServers.context7: %#v", registry) + } + } + if runtime.GOOS != "windows" && info.Mode().Perm() != 0o600 { + t.Fatalf("registry mode widened to %v, want 0600", info.Mode().Perm()) + } +} + func TestComponentOperationsContext7ClaudePreservesCustomLegacyFile(t *testing.T) { homeDir := t.TempDir() workspaceDir := t.TempDir()