diff --git a/PRD.md b/PRD.md index daac8ec07..a6a9e5b5e 100644 --- a/PRD.md +++ b/PRD.md @@ -210,7 +210,7 @@ The installer supports configuring the Gentleman ecosystem into ANY AI coding ag | Agent | Config Location | Ecosystem Support | Priority | |-------|-----------------|-------------------|----------| | Claude Code (Anthropic) | `~/.claude/` | Full: plugins, skills, MCP, CLAUDE.md, settings, hooks, theme, statusline | P0 | -| OpenCode | `~/.config/opencode/` | Full: plugins, skills, MCP, agents, commands, theme | P0 | +| OpenCode | `~/.config/opencode/` | Full: plugins, skills, MCP, agents, commands (no theme: opencode.json's strict schema rejects a top-level theme key, #497) | P0 | | Gemini CLI (Google) | `~/.gemini/` | Partial: MCP, system instructions, skills via system.md | P1 | | Codex (OpenAI) | `~/.codex/` | Partial: MCP, instructions, config.toml | P1 | | Aider | `~/.aider/` or `.aider.conf.yml` | Partial: conventions via config, limited MCP | P2 | @@ -231,7 +231,7 @@ The installer supports configuring the Gentleman ecosystem into ANY AI coding ag | Tier | What Gets Configured | Agents | |------|---------------------|--------| -| **Full** | Engram plugin + MCP servers + skills + SDD orchestrator + GGA integration + persona + theme + permissions + statusline + hooks | Claude Code, OpenCode | +| **Full** | Engram plugin + MCP servers + skills + SDD orchestrator + GGA integration + persona + theme (Claude only; OpenCode's strict schema rejects a top-level theme key, #497) + permissions + statusline + hooks | Claude Code, OpenCode | | **Good** | Skills + MCP servers + SDD (inline mode, no sub-agents) + GGA as review provider + persona rules | Cursor, VSCode | | **Partial** | Skills via system instructions + MCP where supported + GGA provider config + persona | Gemini CLI, Codex, Windsurf, JetBrains, Zed | | **Minimal** | Persona and coding conventions via project/workspace rules | Xcode, Antigravity, any emerging agent | @@ -827,7 +827,7 @@ graph TD end subgraph OC_CONFIG["OpenCode (~/.config/opencode/)"] - OC_JSON[opencode.json
Agents, MCP servers,
Engram plugin, theme] + OC_JSON[opencode.json
Agents, MCP servers,
Engram plugin
no theme: strict schema rejects it] OC_SKILLS_DIR[skill/
SDD skills + coding skills] OC_COMMANDS[commands/
SDD slash commands] OC_PLUGINS[plugins/
engram.ts] @@ -1244,7 +1244,7 @@ When the installer completes with "Dev Stack + Polish" (`full-gentleman`) preset - `~/.claude.json` — Context7 MCP server configured **OpenCode:** -- `~/.config/opencode/opencode.json` — Agents (gentleman, sdd-orchestrator), MCP servers (engram, context7), Engram plugin, Gentleman theme +- `~/.config/opencode/opencode.json` — Agents (gentleman, sdd-orchestrator), MCP servers (engram, context7), Engram plugin (OpenCode's strict schema rejects a top-level theme key, so no theme is written here) - `~/.config/opencode/skills/` — All selected skills mirrored - `~/.config/opencode/commands/` — SDD slash commands - `~/.config/opencode/plugins/` — Engram TypeScript plugin diff --git a/docs/components.md b/docs/components.md index ae5f153c9..e6e2e1441 100644 --- a/docs/components.md +++ b/docs/components.md @@ -15,7 +15,7 @@ | Persona | `persona` | Managed Gentleman/neutral persona injection, or unmanaged custom persona mode | | Permissions | `permissions` | Security-first defaults and guardrails. Applied to Claude Code and OpenCode (the two adapters with permissions overlay support). Default sensitive-paths deny list: `~/.ssh/*`, `~/.ssh/**/*`, `**/*.pem`, `**/*.key`, `**/.env*`, `~/.credentials/*`, `~/.aws/credentials`, `~/.config/gh/hosts.yml`, `~/Library/Keychains/*`, `**/secrets/*`, `**/*.p12`, `**/*.pfx` | | GGA | `gga` | Gentleman Guardian Angel — AI provider switcher | -| Theme | `theme` | Gentleman Kanagawa theme overlay | +| Theme | `theme` | Gentleman Kanagawa setting for supported agents; OpenCode is a no-op because `opencode.json` rejects a top-level `theme` key | ## GGA Behavior diff --git a/e2e/e2e_test.sh b/e2e/e2e_test.sh index 4a90105c4..9c614e1f0 100755 --- a/e2e/e2e_test.sh +++ b/e2e/e2e_test.sh @@ -1023,16 +1023,75 @@ test_oc_permissions_injection() { fi } +test_jsonc_root_key_helper() { + log_test "JSONC root-key assertion accepts valid configs without a root theme" + local fixture missing_fixture + fixture=$(mktemp -d) + missing_fixture="$fixture/missing.json" + + assert_jsonc_has_no_root_key "$missing_fixture" "theme" "Absent opencode.json is accepted" + + cat > "$fixture/nested.json" <<'EOF' +{ + // Nested theme is valid for an OpenCode agent definition. + "agent": { + "reviewer": { + /* Comment-like text in strings must remain intact. */ + "prompt": "https://example.test//path/*literal*/", + "theme": "nested-theme", + }, + }, +} +EOF + + assert_jsonc_has_no_root_key "$fixture/nested.json" "theme" "Nested theme and JSONC syntax are accepted" + + cat > "$fixture/root.json" <<'EOF' +{ + "theme": "root-theme", + "agent": {"reviewer": {"theme": "nested-theme"}}, +} +EOF + if jsonc_has_no_root_key "$fixture/root.json" "theme"; then + log_fail "JSONC root-key assertion accepted root theme" + else + log_pass "JSONC root-key assertion rejects root theme" + fi + + printf '{"agent": ' > "$fixture/malformed.json" + if jsonc_has_no_root_key "$fixture/malformed.json" "theme"; then + log_fail "JSONC root-key assertion accepted malformed JSONC" + else + log_pass "JSONC root-key assertion rejects malformed JSONC" + fi + + if ( + command() { + if [ "$1" = "-v" ] && [ "$2" = "node" ]; then + return 1 + fi + builtin command "$@" + } + jsonc_has_no_root_key "$fixture/nested.json" "theme" + ); then + log_fail "JSONC root-key assertion accepted a missing Node.js runtime" + else + log_pass "JSONC root-key assertion fails closed when Node.js is unavailable" + fi + + rm -rf "$fixture" +} + test_oc_theme_injection() { - log_test "OpenCode: theme injection" + log_test "OpenCode: theme injection skipped (opencode.json rejects top-level theme)" cleanup_test_env if $BINARY install --agent opencode --component theme --persona neutral 2>&1; then local settings="$HOME/.config/opencode/opencode.json" - assert_file_exists "$settings" "OpenCode opencode.json" - assert_file_contains "$settings" '"theme"' "Has theme key" - assert_file_contains "$settings" 'gentleman-kanagawa' "Has gentleman-kanagawa theme" - assert_valid_json "$settings" "opencode.json is valid JSON" + # OpenCode's opencode.json schema is strict and rejects an unrecognized + # top-level "theme" key (ConfigInvalidError), so Gentle AI must not write + # it there. Theme injection is a no-op for OpenCode — see issue #497. + assert_jsonc_has_no_root_key "$settings" "theme" "opencode.json has no root theme key" else log_fail "OpenCode theme install command failed" fi @@ -1093,7 +1152,7 @@ test_full_preset_opencode() { # opencode.json should have all overlays merged assert_file_exists "$settings" "OpenCode opencode.json" assert_file_contains "$settings" '"permission"' "Has permission config" - assert_file_contains "$settings" '"theme"' "Has theme" + assert_jsonc_has_no_root_key "$settings" "theme" "opencode.json has no root theme key" assert_file_contains "$settings" '"mcp"' "Has MCP servers" assert_file_contains "$settings" '"context7"' "Has context7 MCP" assert_valid_json "$settings" "opencode.json is valid JSON" @@ -1471,21 +1530,21 @@ test_idempotent_skills_claude() { } test_idempotent_theme_opencode() { - log_test "Idempotency: theme on OpenCode (run twice, same result)" + log_test "Idempotency: theme on OpenCode is a stable no-op" cleanup_test_env - $BINARY install --agent opencode --component theme --persona neutral 2>&1 || true - local first_hash - first_hash=$(md5sum "$HOME/.config/opencode/opencode.json" 2>/dev/null | cut -d' ' -f1) - - $BINARY install --agent opencode --component theme --persona neutral 2>&1 || true - local second_hash - second_hash=$(md5sum "$HOME/.config/opencode/opencode.json" 2>/dev/null | cut -d' ' -f1) + local settings="$HOME/.config/opencode/opencode.json" + # Both no-op runs must succeed and leave the settings without a theme key. + if $BINARY install --agent opencode --component theme --persona neutral 2>&1; then + assert_jsonc_has_no_root_key "$settings" "theme" "No root theme key after first run" + else + log_fail "First OpenCode theme install exited non-zero (should be a clean no-op)" + fi - if [ "$first_hash" = "$second_hash" ] && [ -n "$first_hash" ]; then - log_pass "Idempotent: same theme config after two runs" + if $BINARY install --agent opencode --component theme --persona neutral 2>&1; then + assert_jsonc_has_no_root_key "$settings" "theme" "No root theme key after second run" else - log_fail "Theme config changed between runs ($first_hash vs $second_hash)" + log_fail "Second OpenCode theme install exited non-zero (should be a clean no-op)" fi } @@ -1629,7 +1688,7 @@ test_edge_multiple_json_overlays() { local settings="$HOME/.config/opencode/opencode.json" assert_file_contains "$settings" '"permission"' "Permission config present after 3 merges" - assert_file_contains "$settings" '"theme"' "Theme present after 3 merges" + assert_file_not_contains "$settings" '"theme"' "Theme not merged into OpenCode (no-op)" assert_file_contains "$settings" '"mcp"' "MCP servers present after 3 merges" assert_file_contains "$settings" '"context7"' "Context7 present after 3 merges" assert_valid_json "$settings" "Final merged JSON is valid" @@ -2270,6 +2329,7 @@ if [ "${RUN_FULL_E2E:-0}" = "1" ]; then test_oc_skills_full test_oc_context7_injection test_oc_permissions_injection + test_jsonc_root_key_helper test_oc_theme_injection # Category 4: Full preset integration diff --git a/e2e/lib.sh b/e2e/lib.sh index 271f04f42..4fd88d0ec 100755 --- a/e2e/lib.sh +++ b/e2e/lib.sh @@ -280,6 +280,104 @@ assert_valid_json() { fi } +# jsonc_has_no_root_key FILE KEY +# Returns success when a JSONC object does not contain KEY at its root while +# allowing the same key in nested objects. +jsonc_has_no_root_key() { + local file="$1" + local key="$2" + if [ ! -f "$file" ]; then + return 0 + fi + if ! command -v node >/dev/null 2>&1; then + printf 'Node.js is required for JSONC root-key assertions. Check the E2E image setup.\n' >&2 + return 2 + fi + node - "$file" "$key" <<'JS' +const fs = require("fs"); + +const raw = fs.readFileSync(process.argv[2], "utf8"); +function nextToken(source, index) { + while (index < source.length) { + const char = source[index]; + if (/\s/.test(char)) { + index += 1; + } else if (char === "/" && source[index + 1] === "/") { + index = source.indexOf("\n", index + 2); + if (index < 0) return ""; + } else if (char === "/" && source[index + 1] === "*") { + index = source.indexOf("*/", index + 2); + if (index < 0) throw new Error("unterminated block comment"); + index += 2; + } else { + return char; + } + } + return ""; +} + +try { + const normalized = []; + let inString = false; + let escaped = false; + + for (let index = 0; index < raw.length; index += 1) { + const char = raw[index]; + if (inString) { + normalized.push(char); + if (char === '"' && !escaped) inString = false; + escaped = char === "\\" && !escaped; + } else if (char === '"') { + inString = true; + normalized.push(char); + } else if (char === "/" && raw[index + 1] === "/") { + index = raw.indexOf("\n", index + 2); + if (index < 0) break; + normalized.push("\n"); + } else if (char === "/" && raw[index + 1] === "*") { + const end = raw.indexOf("*/", index + 2); + if (end < 0) throw new Error("unterminated block comment"); + for (const commentChar of raw.slice(index, end + 2)) { + if (commentChar === "\r" || commentChar === "\n") normalized.push(commentChar); + } + index = end + 1; + } else if (char !== "," || !/[}\]]/.test(nextToken(raw, index + 1))) { + normalized.push(char); + } + } + + const root = JSON.parse(normalized.join("")); + if (root === null || Array.isArray(root) || typeof root !== "object") { + throw new Error("root is not an object"); + } + process.exit(Object.hasOwn(root, process.argv[3]) ? 1 : 0); +} catch (error) { + console.error(`Invalid JSONC in ${process.argv[2]}: ${error.message}`); + process.exit(2); +} +JS +} + +# assert_jsonc_has_no_root_key FILE KEY LABEL +# Checks that a JSONC object does not contain KEY at its root while allowing +# the same key in nested objects. +assert_jsonc_has_no_root_key() { + local file="$1" + local key="$2" + local label="${3:-$file has no root key '$key'}" + if [ ! -f "$file" ]; then + log_pass "$label (file does not exist)" + return 0 + fi + if jsonc_has_no_root_key "$file" "$key"; then + log_pass "$label" + return 0 + else + log_fail "Unexpected root key '$key' or invalid JSONC in $file" + return 1 + fi +} + # json_files_equal FILE1 FILE2 # Returns 0 if both files contain semantically equal JSON (key order ignored). # Uses python3 for comparison (available in CI and most dev machines). diff --git a/internal/agents/interface.go b/internal/agents/interface.go index 037d2778d..74a96486f 100644 --- a/internal/agents/interface.go +++ b/internal/agents/interface.go @@ -64,3 +64,24 @@ type Adapter interface { type EffectiveCodeGraphWiringDetector interface { EffectiveCodeGraphWiring(homeDir string) (path string, configured bool) } + +// ThemeInjectionController lets adapters opt out when their settings schema +// rejects a top-level "theme" key. Other adapters support theme injection. +type ThemeInjectionController interface { + SupportsThemeInjection() bool +} + +// ThemeSettingsMigrator is an optional adapter capability for repairing legacy +// theme settings before current injection behavior is applied. +type ThemeSettingsMigrator interface { + MigrateThemeSettings(homeDir string) (path string, changed bool, err error) +} + +// SupportsThemeInjection reports whether an adapter permits theme injection. +// Adapters opt out through ThemeInjectionController; all others support it. +// Keeping this decision centralized aligns injection with post-apply +// verification. +func SupportsThemeInjection(adapter Adapter) bool { + controller, ok := adapter.(ThemeInjectionController) + return !ok || controller.SupportsThemeInjection() +} diff --git a/internal/agents/opencode/adapter.go b/internal/agents/opencode/adapter.go index 389ed1b8a..0850c4bc5 100644 --- a/internal/agents/opencode/adapter.go +++ b/internal/agents/opencode/adapter.go @@ -2,6 +2,7 @@ package opencode import ( "context" + "fmt" "os" "os/exec" "path/filepath" @@ -158,6 +159,39 @@ func isEffectiveCodeGraphEntry(value any) bool { // --- Optional capabilities --- +// SupportsThemeInjection returns false because OpenCode's strict opencode.json +// schema rejects a top-level "theme" key (ConfigInvalidError), breaking runtime config updates such as profile activation. +func (a *Adapter) SupportsThemeInjection() bool { + return false +} + +func (a *Adapter) MigrateThemeSettings(homeDir string) (string, bool, error) { + settingsPath := a.SettingsPath(homeDir) + info, err := os.Stat(settingsPath) + if err != nil { + if os.IsNotExist(err) { + return "", false, nil + } + return "", false, fmt.Errorf("stat OpenCode settings %q: %w", settingsPath, err) + } + raw, err := os.ReadFile(settingsPath) + if err != nil { + return "", false, fmt.Errorf("read OpenCode settings %q: %w", settingsPath, err) + } + updated, changed, err := filemerge.RemoveTopLevelJSONKey(raw, "theme") + if err != nil { + return "", false, fmt.Errorf("migrate OpenCode settings %q: %w", settingsPath, err) + } + if !changed { + return settingsPath, false, nil + } + result, err := filemerge.WriteFileAtomic(settingsPath, updated, info.Mode().Perm()) + if err != nil { + return "", false, fmt.Errorf("write migrated OpenCode settings %q: %w", settingsPath, err) + } + return settingsPath, result.Changed, nil +} + func (a *Adapter) SupportsOutputStyles() bool { return false } diff --git a/internal/agents/opencode/adapter_test.go b/internal/agents/opencode/adapter_test.go index 2a3ab66c4..a186901e4 100644 --- a/internal/agents/opencode/adapter_test.go +++ b/internal/agents/opencode/adapter_test.go @@ -219,6 +219,12 @@ func TestEffectiveCodeGraphWiring(t *testing.T) { } } +func TestSupportsThemeInjection(t *testing.T) { + if NewAdapter().SupportsThemeInjection() { + t.Fatalf("SupportsThemeInjection() = true, want false; opencode.json schema rejects a top-level theme key") + } +} + func TestConfigPathIgnoresRelativeXDGConfigHome(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) diff --git a/internal/catalog/components.go b/internal/catalog/components.go index 3495f250d..9bb56e023 100644 --- a/internal/catalog/components.go +++ b/internal/catalog/components.go @@ -16,7 +16,7 @@ var mvpComponents = []Component{ {ID: model.ComponentPersona, Name: "Persona", Description: "Managed agent behavior and conversation tone"}, {ID: model.ComponentPermission, Name: "Permissions", Description: "Security-first defaults and guardrails"}, {ID: model.ComponentGGA, Name: "GGA", Description: "Gentleman Guardian Angel — AI provider switcher"}, - {ID: model.ComponentTheme, Name: "OpenCode Theme", Description: "Visual polish: OpenCode color theme"}, + {ID: model.ComponentTheme, Name: "Theme", Description: "Gentleman Kanagawa setting for supported agents"}, {ID: model.ComponentClaudeTheme, Name: "Claude Code Theme", Description: "Visual polish: Claude Code color theme"}, {ID: model.ComponentOpenCodeGentleLogo, Name: "OpenCode Logo", Description: "Visual polish: OpenCode home logo plugin"}, } diff --git a/internal/cli/run.go b/internal/cli/run.go index ab7eeb1ea..7ca51047e 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1571,8 +1571,11 @@ func componentPathsWithWorkspaceScoped(homeDir, workspaceDir string, scope Insta paths = append(paths, gga.ConfigPath(homeDir)) paths = append(paths, gga.AgentsTemplatePath(homeDir)) case model.ComponentTheme: + // Include existing settings in backup and verification for migration, but never require or create a missing file. if p := adapter.SettingsPath(homeDir); p != "" { - paths = append(paths, p) + if _, err := os.Stat(p); agents.SupportsThemeInjection(adapter) || err == nil { + paths = append(paths, p) + } } case model.ComponentClaudeTheme: if adapter.Agent() == model.AgentClaudeCode { diff --git a/internal/cli/run_component_paths_test.go b/internal/cli/run_component_paths_test.go index cbf2ad79e..dbc6aec10 100644 --- a/internal/cli/run_component_paths_test.go +++ b/internal/cli/run_component_paths_test.go @@ -46,6 +46,29 @@ func TestComponentPathsSDDIncludesOpenCodeSettingsAndCommands(t *testing.T) { } } +func TestComponentPathsThemeOpenCodeTracksOnlyExistingSettings(t *testing.T) { + home := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", "") + adapters := resolveAdapters([]model.AgentID{model.AgentOpenCode}) + settingsPath := filepath.Join(home, ".config", "opencode", "opencode.json") + + paths := componentPaths(home, model.Selection{}, adapters, model.ComponentTheme) + if containsPath(paths, settingsPath) { + t.Fatalf("componentPaths(theme) includes missing OpenCode settings %q", settingsPath) + } + if err := os.MkdirAll(filepath.Dir(settingsPath), 0o755); err != nil { + t.Fatalf("MkdirAll(settings dir) error = %v", err) + } + if err := os.WriteFile(settingsPath, []byte("{}\n"), 0o600); err != nil { + t.Fatalf("WriteFile(settings) error = %v", err) + } + + paths = componentPaths(home, model.Selection{}, adapters, model.ComponentTheme) + if !containsPath(paths, settingsPath) { + t.Fatalf("componentPaths(theme) missing existing OpenCode settings %q", settingsPath) + } +} + func TestComponentPathsSDDIncludesClaudeLazyWorkflow(t *testing.T) { home := t.TempDir() adapters := resolveAdapters([]model.AgentID{model.AgentClaudeCode}) diff --git a/internal/components/filemerge/json_merge_test.go b/internal/components/filemerge/json_merge_test.go index 4bd647e57..e26479b5e 100644 --- a/internal/components/filemerge/json_merge_test.go +++ b/internal/components/filemerge/json_merge_test.go @@ -66,6 +66,23 @@ func TestMergeJSONObjectsSupportsJSONCBase(t *testing.T) { } } +func TestJSONScannersStopAtIsolatedSlash(t *testing.T) { + for _, tt := range []struct { + name string + scan func([]byte, int) int + }{ + {"comment", skipJSONComment}, + {"value delimiter", scanJSONValueDelimiter}, + {"trivia", skipJSONTrivia}, + } { + t.Run(tt.name, func(t *testing.T) { + if got := tt.scan([]byte("/"), 0); got != 0 { + t.Fatalf("scan isolated slash = %d, want 0", got) + } + }) + } +} + func TestMergeJSONObjectsMalformedBaseReturnsOverlayOnly(t *testing.T) { // Real user machines (e.g. Windows) may have a malformed ~/.cursor/mcp.json. // The installer should recover by treating the broken base as {} and continuing. diff --git a/internal/components/filemerge/json_remove.go b/internal/components/filemerge/json_remove.go new file mode 100644 index 000000000..f285d7614 --- /dev/null +++ b/internal/components/filemerge/json_remove.go @@ -0,0 +1,190 @@ +package filemerge + +import ( + "bytes" + "encoding/json" + "fmt" + "strconv" +) + +// RemoveTopLevelJSONKey removes root keys without rewriting unrelated bytes. +func RemoveTopLevelJSONKey(raw []byte, key string) ([]byte, bool, error) { + if err := validateJSONCObject(raw); err != nil { + return nil, false, fmt.Errorf("unmarshal json object: %w", err) + } + if i := skipJSONTrivia(raw, 0); i >= len(raw) || raw[i] != '{' { + return nil, false, fmt.Errorf("json root is not an object") + } + for updated := raw; ; { + next, removed := removeTopLevelJSONKeyOnce(updated, []byte(strconv.Quote(key))) + if !removed { + if err := validateJSONCObject(updated); err != nil { + return nil, false, fmt.Errorf("validate updated json object: %w", err) + } + return updated, !bytes.Equal(updated, raw), nil + } + updated = next + } +} + +func validateJSONCObject(raw []byte) error { + normalized := bytes.Clone(raw) + for i := 0; i < len(raw); i++ { + if raw[i] == '"' { + i = scanJSONString(raw, i) - 1 + continue + } + if i+1 >= len(raw) || raw[i] != '/' || (raw[i+1] != '/' && raw[i+1] != '*') { + continue + } + end := skipJSONComment(raw, i) + if end > len(raw) { + return fmt.Errorf("unterminated block comment") + } + for j := i; j < end; j++ { + if raw[j] != '\n' && raw[j] != '\r' { + normalized[j] = ' ' + } + } + i = end - 1 + } + + return json.Unmarshal(stripTrailingCommas(normalized), new(map[string]any)) +} + +func removeTopLevelJSONKeyOnce(raw, quotedKey []byte) ([]byte, bool) { + i, previousComma := skipJSONTrivia(raw, 0)+1, -1 + for { + memberStart := i + i = skipJSONTrivia(raw, i) + if i >= len(raw) || raw[i] == '}' { + return raw, false + } + keyStart := i + keyEnd := scanJSONString(raw, keyStart) + i = skipJSONTrivia(raw, keyEnd) + 1 // validated input guarantees the colon + delimiter := scanJSONValueDelimiter(raw, skipJSONTrivia(raw, i)) + if bytes.Equal(raw[keyStart:keyEnd], quotedKey) { + removeStart, removeEnd := memberStart, delimiter + trivia := raw[memberStart:keyStart] + preserveComment := previousComma < 0 && bytes.IndexByte(trivia, '/') >= 0 + if preserveComment { + commentEnd := 0 + for offset := 0; offset < len(trivia); offset++ { + if trivia[offset] != '/' { + continue + } + next := skipJSONComment(trivia, offset) + if next <= offset { + return raw, false + } + commentEnd = skipJSONLineEnding(trivia, next) + offset = commentEnd - 1 + } + removeStart = memberStart + commentEnd + } + if raw[delimiter] == ',' { + removeEnd++ + } else if previousComma >= 0 { + removeStart = previousComma + } + if preserveComment { + removeEnd = skipJSONLineEnding(raw, removeEnd) + } + updated := bytes.Clone(raw[:removeStart]) + updated = append(updated, raw[removeEnd:]...) + return updated, true + } + if raw[delimiter] == '}' { + return raw, false + } + previousComma, i = delimiter, delimiter+1 + } +} +func scanJSONString(raw []byte, start int) int { + escaped := false + for i := start + 1; i < len(raw); i++ { + if escaped { + escaped = false + } else if raw[i] == '\\' { + escaped = true + } else if raw[i] == '"' { + return i + 1 + } + } + return len(raw) +} + +func scanJSONValueDelimiter(raw []byte, start int) int { + depth := 0 + for i := start; i < len(raw); i++ { + switch raw[i] { + case '"': + i = scanJSONString(raw, i) - 1 + case '/': + next := skipJSONComment(raw, i) + if next <= i { + return i + } + i = next - 1 + case '{', '[': + depth++ + case ']', '}': + depth-- + if depth < 0 { + return i + } + case ',': + if depth == 0 { + return i + } + } + } + return len(raw) - 1 +} +func skipJSONTrivia(raw []byte, start int) int { + for start < len(raw) { + switch raw[start] { + case ' ', '\t', '\r', '\n': + start++ + case '/': + next := skipJSONComment(raw, start) + if next <= start { + return start + } + start = next + default: + return start + } + } + return start +} + +func skipJSONLineEnding(raw []byte, start int) int { + if start+1 < len(raw) && raw[start] == '\r' && raw[start+1] == '\n' { + return start + 2 + } + if start < len(raw) && raw[start] == '\n' { + return start + 1 + } + return start +} + +func skipJSONComment(raw []byte, start int) int { + if start+1 >= len(raw) || raw[start] != '/' { + return start + } + if raw[start+1] == '/' { + if end := bytes.IndexByte(raw[start+2:], '\n'); end >= 0 { + return start + 2 + end + } + return len(raw) + } + if raw[start+1] == '*' { + if end := bytes.Index(raw[start+2:], []byte("*/")); end >= 0 { + return start + 4 + end + } + return len(raw) + 1 + } + return start +} diff --git a/internal/components/theme/inject.go b/internal/components/theme/inject.go index 0ede06559..31d7db0ff 100644 --- a/internal/components/theme/inject.go +++ b/internal/components/theme/inject.go @@ -40,17 +40,46 @@ var gentlemanClaudeTheme = claudeTheme{ } func Inject(homeDir string, adapter agents.Adapter) (InjectionResult, error) { + result := InjectionResult{} + if migrator, ok := adapter.(agents.ThemeSettingsMigrator); ok { + path, changed, err := migrator.MigrateThemeSettings(homeDir) + if err != nil { + return InjectionResult{}, err + } + if changed { + result.Changed = true + result.Files = appendUniquePath(result.Files, path) + } + } + if !agents.SupportsThemeInjection(adapter) { + return result, nil + } + settingsPath := adapter.SettingsPath(homeDir) if settingsPath == "" { - return InjectionResult{}, nil + return result, nil } writeResult, err := mergeJSONFile(settingsPath, themeOverlayJSON) if err != nil { - return InjectionResult{}, err + return result, err } - return InjectionResult{Changed: writeResult.Changed, Files: []string{settingsPath}}, nil + result.Changed = result.Changed || writeResult.Changed + result.Files = appendUniquePath(result.Files, settingsPath) + return result, nil +} + +func appendUniquePath(paths []string, path string) []string { + if path == "" { + return paths + } + for _, existing := range paths { + if existing == path { + return paths + } + } + return append(paths, path) } func InjectClaudeTheme(homeDir string, adapter agents.Adapter) (InjectionResult, error) { diff --git a/internal/components/theme/inject_test.go b/internal/components/theme/inject_test.go index ee011ad5f..d417dc886 100644 --- a/internal/components/theme/inject_test.go +++ b/internal/components/theme/inject_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "runtime" "testing" "github.com/gentleman-programming/gentle-ai/internal/agents" @@ -14,6 +15,25 @@ import ( func claudeAdapter() agents.Adapter { return claude.NewAdapter() } func opencodeAdapter() agents.Adapter { return opencode.NewAdapter() } +type migratingThemeAdapter struct { + agents.Adapter + path string + settingsPath string + calls int +} + +func (a *migratingThemeAdapter) MigrateThemeSettings(_ string) (string, bool, error) { + a.calls++ + return a.path, true, nil +} + +func (a *migratingThemeAdapter) SettingsPath(homeDir string) string { + if a.settingsPath != "" { + return a.settingsPath + } + return a.Adapter.SettingsPath(homeDir) +} + func TestInjectMergesThemeOverlayIntoAdapterSettings(t *testing.T) { home := t.TempDir() settingsPath := filepath.Join(home, ".claude", "settings.json") @@ -69,12 +89,12 @@ func TestInjectMergesThemeOverlayIntoAdapterSettings(t *testing.T) { func TestInjectCreatesAdapterSettingsWhenMissing(t *testing.T) { home := t.TempDir() - result, err := Inject(home, opencodeAdapter()) + result, err := Inject(home, claudeAdapter()) if err != nil { t.Fatalf("Inject() error = %v", err) } - settingsPath := filepath.Join(home, ".config", "opencode", "opencode.json") + settingsPath := filepath.Join(home, ".claude", "settings.json") if !result.Changed { t.Fatalf("Inject() changed = false") } @@ -82,6 +102,43 @@ func TestInjectCreatesAdapterSettingsWhenMissing(t *testing.T) { t.Fatalf("files = %#v, want only %q", result.Files, settingsPath) } + data, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("ReadFile(settings) error = %v", err) + } + var root struct { + Theme string `json:"theme"` + } + if err := json.Unmarshal(data, &root); err != nil { + t.Fatalf("Unmarshal(settings) error = %v", err) + } + if root.Theme == "" { + t.Fatalf("theme = %q, want a non-empty theme identifier in the created settings", root.Theme) + } +} + +func TestInjectContinuesThemeInjectionAfterMigration(t *testing.T) { + home := t.TempDir() + settingsPath := filepath.Join(home, ".claude", "settings.json") + adapter := &migratingThemeAdapter{ + Adapter: claudeAdapter(), + path: settingsPath, + } + + result, err := Inject(home, adapter) + if err != nil { + t.Fatalf("Inject() error = %v", err) + } + if adapter.calls != 1 { + t.Fatalf("MigrateThemeSettings() calls = %d, want 1", adapter.calls) + } + if !result.Changed { + t.Fatal("Inject() changed = false, want migration and theme injection") + } + if len(result.Files) != 1 || result.Files[0] != settingsPath { + t.Fatalf("Inject() files = %#v, want unique settings path %q", result.Files, settingsPath) + } + data, err := os.ReadFile(settingsPath) if err != nil { t.Fatalf("ReadFile(settings) error = %v", err) @@ -97,6 +154,154 @@ func TestInjectCreatesAdapterSettingsWhenMissing(t *testing.T) { } } +func TestInjectPreservesMigrationResultWhenThemeInjectionFails(t *testing.T) { + home := t.TempDir() + migrationPath := filepath.Join(home, "migration.json") + injectionPath := filepath.Join(home, "blocked-settings") + if err := os.Mkdir(injectionPath, 0o755); err != nil { + t.Fatalf("Mkdir(injection path) error = %v", err) + } + adapter := &migratingThemeAdapter{ + Adapter: claudeAdapter(), + path: migrationPath, + settingsPath: injectionPath, + } + + result, err := Inject(home, adapter) + if err == nil { + t.Fatal("Inject() error = nil, want theme injection failure") + } + if !result.Changed { + t.Fatal("Inject() changed = false, want recorded migration change") + } + if len(result.Files) != 1 || result.Files[0] != migrationPath { + t.Fatalf("Inject() files = %#v, want only migration path %q", result.Files, migrationPath) + } +} + +func TestInjectSkipsOpenCodeThemeInjection(t *testing.T) { + home := t.TempDir() + + result, err := Inject(home, opencodeAdapter()) + if err != nil { + t.Fatalf("Inject() error = %v", err) + } + if result.Changed || len(result.Files) != 0 { + t.Fatalf("Inject() = %#v, want no-op for OpenCode; opencode.json schema rejects top-level theme", result) + } + + if _, err := os.Stat(filepath.Join(home, ".config", "opencode", "opencode.json")); !os.IsNotExist(err) { + t.Fatalf("Inject() must not write opencode.json for OpenCode; stat error = %v", err) + } +} + +func TestInjectRemovesLegacyOpenCodeThemeOnly(t *testing.T) { + home := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", "") + settingsPath := filepath.Join(home, ".config", "opencode", "opencode.json") + if err := os.MkdirAll(filepath.Dir(settingsPath), 0o755); err != nil { + t.Fatalf("MkdirAll(settings dir) error = %v", err) + } + input := "{\n // Keep this comment with the schema setting.\n \"theme\": \"gentleman-kanagawa\",\n \"$schema\": \"https://opencode.ai/config.json\",\n \"agent\": {\"reviewer\": {\"theme\": \"nested-theme\", \"model\": \"test/model\"}},\n \"share\": \"disabled\"\n}\n" + want := "{\n // Keep this comment with the schema setting.\n \"$schema\": \"https://opencode.ai/config.json\",\n \"agent\": {\"reviewer\": {\"theme\": \"nested-theme\", \"model\": \"test/model\"}},\n \"share\": \"disabled\"\n}\n" + if err := os.WriteFile(settingsPath, []byte(input), 0o600); err != nil { + t.Fatalf("WriteFile(settings) error = %v", err) + } + + result, err := Inject(home, opencodeAdapter()) + if err != nil { + t.Fatalf("Inject() error = %v", err) + } + if !result.Changed || len(result.Files) != 1 || result.Files[0] != settingsPath { + t.Fatalf("Inject() = %#v, want changed OpenCode settings", result) + } + + got, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("ReadFile(settings) error = %v", err) + } + if string(got) != want { + t.Fatalf("settings after migration =\n%s\nwant only root theme removed:\n%s", got, want) + } + if runtime.GOOS != "windows" { + info, err := os.Stat(settingsPath) + if err != nil { + t.Fatalf("Stat(settings) error = %v", err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("settings mode = %o, want preserved 600", got) + } + } + + second, err := Inject(home, opencodeAdapter()) + if err != nil { + t.Fatalf("Inject() second error = %v", err) + } + if second.Changed || len(second.Files) != 0 { + t.Fatalf("Inject() second = %#v, want idempotent no-op", second) + } + + for _, tt := range []struct{ name, input, want string }{ + {"sole member after line comment", "{// keep\n\"theme\":\"x\"}", "{// keep\n}"}, + {"same-line next member after line comment", "{// keep\n\"theme\": \"x\",\"share\": \"disabled\"}", "{// keep\n\"share\": \"disabled\"}"}, + {"same-line next member after CRLF comment", "{// keep\r\n\"theme\": \"x\",\"share\": \"disabled\"}", "{// keep\r\n\"share\": \"disabled\"}"}, + {"same-line next member after block comment", "{/*keep*/\"theme\": \"x\",\"share\": \"disabled\"}", "{/*keep*/\"share\": \"disabled\"}"}, + {"next member after block comment and CRLF", "{/*keep*/\r\n\"theme\":\"x\",\r\n\"share\":\"disabled\"}", "{/*keep*/\r\n\"share\":\"disabled\"}"}, + } { + t.Run(tt.name, func(t *testing.T) { + if err := os.WriteFile(settingsPath, []byte(tt.input), 0o600); err != nil { + t.Fatalf("WriteFile(settings) error = %v", err) + } + if _, err := Inject(home, opencodeAdapter()); err != nil { + t.Fatalf("Inject() error = %v", err) + } + got, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("ReadFile(settings) error = %v", err) + } + if string(got) != tt.want { + t.Fatalf("settings after migration = %q, want %q", got, tt.want) + } + }) + } +} + +func TestInjectRejectsMalformedLegacyOpenCodeSettingsWithoutMutation(t *testing.T) { + for _, tt := range []struct{ name, input string }{ + {"incomplete value", "{\n \"theme\": \"gentleman-kanagawa\",\n \"share\":\n"}, + {"unterminated trailing block comment", "{\n \"theme\": \"gentleman-kanagawa\"\n}\n/*"}, + {"comment-spliced boolean", "{\"theme\":\"x\",\"ok\":tr/*c*/ue}"}, + {"comment-spliced number", "{\"theme\":\"x\",\"n\":1/*c*/2}"}, + {"comment-spliced null", "{\"theme\":\"x\",\"v\":n/*c*/ull}"}, + } { + t.Run(tt.name, func(t *testing.T) { + home := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", "") + settingsPath := filepath.Join(home, ".config", "opencode", "opencode.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(tt.input), 0o600); err != nil { + t.Fatalf("WriteFile(settings) error = %v", err) + } + result, err := Inject(home, opencodeAdapter()) + if err == nil { + t.Fatalf("Inject() error = nil, want malformed JSON error") + } + if result.Changed || len(result.Files) != 0 { + t.Fatalf("Inject() = %#v after error, want no reported mutation", result) + } + got, readErr := os.ReadFile(settingsPath) + if readErr != nil { + t.Fatalf("ReadFile(settings) error = %v", readErr) + } + if string(got) != tt.input { + t.Fatalf("malformed settings changed = %q, want %q", got, tt.input) + } + }) + } +} + func TestInjectClaudeThemeIsIdempotent(t *testing.T) { home := t.TempDir() diff --git a/internal/tui/testdata/preset-custom-no-opencode-next.golden b/internal/tui/testdata/preset-custom-no-opencode-next.golden index 9a09df5e7..18dd27fbb 100644 --- a/internal/tui/testdata/preset-custom-no-opencode-next.golden +++ b/internal/tui/testdata/preset-custom-no-opencode-next.golden @@ -17,7 +17,7 @@ Toggle components with enter or space. [ ] gga Gentleman Guardian Angel — AI provider switcher [ ] theme - Visual polish: OpenCode color theme + Gentleman Kanagawa setting for supported agents [ ] claude-theme Visual polish: Claude Code color theme [ ] opencode-gentle-logo diff --git a/internal/tui/testdata/preset-custom-opencode-next.golden b/internal/tui/testdata/preset-custom-opencode-next.golden index 9a09df5e7..18dd27fbb 100644 --- a/internal/tui/testdata/preset-custom-opencode-next.golden +++ b/internal/tui/testdata/preset-custom-opencode-next.golden @@ -17,7 +17,7 @@ Toggle components with enter or space. [ ] gga Gentleman Guardian Angel — AI provider switcher [ ] theme - Visual polish: OpenCode color theme + Gentleman Kanagawa setting for supported agents [ ] claude-theme Visual polish: Claude Code color theme [ ] opencode-gentle-logo