Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions internal/agents/claude/adapter.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
}
Comment on lines +102 to +108

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | πŸ”΄ Critical | ⚑ Quick win

Allow a missing user config to initialize the merge.

readUserConfigBase returns nil, nil for a nonexistent file, but UnmarshalJSONObject(nil) fails. First-time injection therefore aborts instead of creating ~/.claude.jsonβ€”including TestInjectClaudeTightensModeOnNoOpRun.

Return an existence flag from readUserConfigBase, or only validate JSON when a base file exists; continue rejecting an existing empty or malformed file.

Also applies to: 141-146

πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/agents/claude/adapter.go` around lines 106 - 112, Update the
config-loading flow around readUserConfigBase and the UnmarshalJSONObject
validation so a nonexistent user config is treated as an empty merge base and
allows initialization. Preserve rejection of existing empty or malformed files
by performing JSON validation only when the base file exists, using an existence
flag or equivalent distinction from readUserConfigBase, and apply the same
behavior to the other referenced call site.

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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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")
}
Expand Down
9 changes: 7 additions & 2 deletions internal/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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
Expand Down
23 changes: 16 additions & 7 deletions internal/cli/run_component_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/cli/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
97 changes: 90 additions & 7 deletions internal/components/mcp/inject.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -16,25 +18,33 @@ 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
}

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())
}
Expand Down Expand Up @@ -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
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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")
Expand Down
Loading
Loading