diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e7725bf20..9b76bfd7b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -104,6 +104,7 @@ - Fixed advisor recovery selecting another role's fallback chain when both roles use the same model. ([#8075](https://github.com/can1357/oh-my-pi/issues/8075)) - Fixed `retry_fallback_applied` and `retry_fallback_succeeded` not being forwarded to extensions: `AgentSession.#emitExtensionEvent` had no branch for either event and `ExtensionAPI.on(...)` lacked overloads, so extension handlers could not observe model/advisor fallback transitions or successes that the TUI and RPC paths already received ([#8079](https://github.com/can1357/oh-my-pi/issues/8079)). - Fixed the CLI crashing at startup with a raw uncaught `AuthBrokerError` when a configured auth broker (`auth.broker.url` / `OMP_AUTH_BROKER_URL`) is unreachable and no fresh cached snapshot exists. Startup auth discovery now fails with an actionable message naming the broker URL and the recovery options (`omp auth-broker serve`, or resetting `auth.broker.url` / `auth.broker.token`) and exits non-zero, instead of dumping a stack trace ([#8096](https://github.com/can1357/oh-my-pi/issues/8096)). +- Fixed OpenCode discovery ignoring `opencode.jsonc` files and rejecting comments in `opencode.json`, which omitted imported settings and MCP servers. ([#8104](https://github.com/can1357/oh-my-pi/issues/8104)) ## [17.2.12] - 2026-08-08 diff --git a/packages/coding-agent/src/discovery/opencode.ts b/packages/coding-agent/src/discovery/opencode.ts index 90dbd2434..cfacb23c6 100644 --- a/packages/coding-agent/src/discovery/opencode.ts +++ b/packages/coding-agent/src/discovery/opencode.ts @@ -3,12 +3,12 @@ * * Loads configuration from OpenCode's config directories: * - User: ~/.config/opencode/ - * - Project: .opencode/ (cwd) and opencode.json (project root) + * - Project: .opencode/ (cwd) and opencode.json/opencode.jsonc (project root) * * Capabilities: * - context-files: AGENTS.md (user-level only at ~/.config/opencode/AGENTS.md) - * - mcps: From opencode.json "mcp" key - * - settings: From opencode.json + * - mcps: From opencode.json and opencode.jsonc "mcp" keys + * - settings: From opencode.json and opencode.jsonc * - skills: From skills/ subdirectories * - slash-commands: From commands/ subdirectories * - extension-modules: From plugins/ subdirectories @@ -16,7 +16,8 @@ * Priority: 55 (tool-specific provider) */ import * as path from "node:path"; -import { logger, parseFrontmatter, tryParseJson } from "@oh-my-pi/pi-utils"; +import { isRecord, logger, parseFrontmatter } from "@oh-my-pi/pi-utils"; +import { JSONC } from "bun"; import { registerProvider } from "../capability"; import { type ContextFile, contextFileCapability } from "../capability/context-file"; import { type ExtensionModule, extensionModuleCapability } from "../capability/extension-module"; @@ -42,23 +43,60 @@ import { const PROVIDER_ID = "opencode"; const DISPLAY_NAME = "OpenCode"; const PRIORITY = 55; +const CONFIG_FILENAMES = ["opencode.json", "opencode.jsonc"] as const; + +interface OpenCodeConfigSource { + path: string; + level: "user" | "project"; +} // ============================================================================= // JSON Config Loading // ============================================================================= -async function loadJsonConfig(configPath: string): Promise | null> { +async function loadJsonConfig( + configPath: string, + onInvalid: (configPath: string) => void, +): Promise | null> { const content = await readFile(configPath); if (!content) return null; - const parsed = tryParseJson>(content); - if (!parsed) { - logger.warn("Failed to parse OpenCode JSON config", { path: configPath }); + let parsed: unknown; + try { + parsed = JSONC.parse(content); + } catch { + onInvalid(configPath); + return null; + } + if (!isRecord(parsed)) { + onInvalid(configPath); return null; } return parsed; } +/** + * OpenCode config sources in ascending effective precedence (lowest first): + * user `opencode.json` → user `opencode.jsonc` → project `opencode.json` → + * project `opencode.jsonc`. This matches how OpenCode merges configs — project + * overrides user, and within a scope `opencode.jsonc` overrides `opencode.json`. + * + * Both consumers apply this order low-to-high: settings deep-merge in item + * order (last wins) and `loadMCPServers` deep-merges each server across layers + * (later overrides earlier), so higher-precedence sources win in both. + */ +function getConfigSources(ctx: LoadContext): OpenCodeConfigSource[] { + const sources: OpenCodeConfigSource[] = []; + for (const filename of CONFIG_FILENAMES) { + const configPath = getUserPath(ctx, "opencode", filename); + if (configPath) sources.push({ path: configPath, level: "user" }); + } + for (const filename of CONFIG_FILENAMES) { + sources.push({ path: path.join(ctx.cwd, filename), level: "project" }); + } + return sources; +} + // ============================================================================= // Context Files (AGENTS.md) // ============================================================================= @@ -85,10 +123,10 @@ async function loadContextFiles(ctx: LoadContext): Promise> { - const items: MCPServer[] = []; const warnings: string[] = []; - // User-level: ~/.config/opencode/opencode.json - const userConfigPath = getUserPath(ctx, "opencode", "opencode.json"); - if (userConfigPath) { - const config = await loadJsonConfig(userConfigPath); - if (config) { - const result = extractMCPServers(config, userConfigPath, "user"); - items.push(...result.items); - if (result.warnings) warnings.push(...result.warnings); + // Deep-merge each server across config layers in ascending precedence, the + // way OpenCode itself merges configs, so a partial higher-precedence override + // (e.g. project opencode.jsonc setting only mcp..timeout) inherits the + // command/url from lower-precedence layers instead of shadowing the complete + // definition and being rejected by mcpCapability.validate. + const mergedByName = new Map>(); + const sourceByName = new Map(); + + for (const source of getConfigSources(ctx)) { + const config = await loadJsonConfig(source.path, configPath => { + logger.warn("Failed to parse OpenCode config", { path: configPath }); + }); + if (!config || !isRecord(config.mcp)) continue; + + for (const name in config.mcp) { + const raw = config.mcp[name]; + if (!isRecord(raw)) { + warnings.push(`Invalid MCP config for "${name}" in ${source.path}`); + continue; + } + const previous = mergedByName.get(name); + mergedByName.set(name, previous ? mergeConfigRecords(previous, raw) : raw); + sourceByName.set(name, source); } } - // Project-level: opencode.json in project root - const projectConfigPath = path.join(ctx.cwd, "opencode.json"); - const projectConfig = await loadJsonConfig(projectConfigPath); - if (projectConfig) { - const result = extractMCPServers(projectConfig, projectConfigPath, "project"); - items.push(...result.items); - if (result.warnings) warnings.push(...result.warnings); + const items: MCPServer[] = []; + for (const [name, config] of mergedByName) { + const serverConfig = expandEnvVarsDeep(config) as OpenCodeMCPConfig; + const source = sourceByName.get(name)!; + items.push(buildMCPServer(name, serverConfig, source)); } return { items, warnings }; } -function extractMCPServers( - config: Record, - configPath: string, - level: "user" | "project", -): LoadResult { - const items: MCPServer[] = []; - const warnings: string[] = []; +/** Deep-merge two OpenCode config records; `override` wins, nested records recurse. */ +function mergeConfigRecords(base: Record, override: Record): Record { + const result: Record = { ...base }; + for (const key in override) { + const value = override[key]; + const existing = result[key]; + result[key] = isRecord(existing) && isRecord(value) ? mergeConfigRecords(existing, value) : value; + } + return result; +} - if (!config.mcp || typeof config.mcp !== "object") { - return { items, warnings }; +/** Translate one merged OpenCode MCP entry into the canonical MCPServer shape. */ +function buildMCPServer(name: string, serverConfig: OpenCodeMCPConfig, source: OpenCodeConfigSource): MCPServer { + // Determine transport from OpenCode's "type" field + let transport: "stdio" | "sse" | "http" | undefined; + if (serverConfig.type === "local") { + transport = "stdio"; + } else if (serverConfig.type === "remote") { + transport = "http"; + } else if (serverConfig.url) { + transport = "http"; + } else if (serverConfig.command) { + transport = "stdio"; } - const servers = expandEnvVarsDeep(config.mcp as Record); + const command = normalizeCommand(serverConfig.command, serverConfig.args); + const env = stringRecord(serverConfig.environment) ?? stringRecord(serverConfig.env); - for (const [name, raw] of Object.entries(servers)) { - if (!raw || typeof raw !== "object") { - warnings.push(`Invalid MCP config for "${name}" in ${configPath}`); - continue; - } - - const serverConfig = raw as OpenCodeMCPConfig; - - // Determine transport from OpenCode's "type" field - let transport: "stdio" | "sse" | "http" | undefined; - if (serverConfig.type === "local") { - transport = "stdio"; - } else if (serverConfig.type === "remote") { - transport = "http"; - } else if (serverConfig.url) { - transport = "http"; - } else if (serverConfig.command) { - transport = "stdio"; - } - - const command = normalizeCommand(serverConfig.command, serverConfig.args); - const env = stringRecord(serverConfig.environment) ?? stringRecord(serverConfig.env); - - items.push({ - name, - command: command.command, - args: command.args, - env, - url: typeof serverConfig.url === "string" ? serverConfig.url : undefined, - headers: serverConfig.headers && typeof serverConfig.headers === "object" ? serverConfig.headers : undefined, - enabled: serverConfig.enabled, - timeout: typeof serverConfig.timeout === "number" ? serverConfig.timeout : undefined, - transport, - _source: createSourceMeta(PROVIDER_ID, configPath, level), - }); - } - - return { items, warnings }; + return { + name, + command: command.command, + args: command.args, + env, + url: typeof serverConfig.url === "string" ? serverConfig.url : undefined, + headers: serverConfig.headers && typeof serverConfig.headers === "object" ? serverConfig.headers : undefined, + enabled: serverConfig.enabled, + timeout: typeof serverConfig.timeout === "number" ? serverConfig.timeout : undefined, + transport, + _source: createSourceMeta(PROVIDER_ID, source.path, source.level), + }; } // ============================================================================= @@ -342,47 +380,25 @@ async function loadSlashCommands(ctx: LoadContext): Promise> { const items: Settings[] = []; const warnings: string[] = []; - // User-level: ~/.config/opencode/opencode.json - const userConfigPath = getUserPath(ctx, "opencode", "opencode.json"); - if (userConfigPath) { - const content = await readFile(userConfigPath); - if (content) { - const parsed = tryParseJson>(content); - if (parsed) { - items.push({ - path: userConfigPath, - data: parsed, - level: "user", - _source: createSourceMeta(PROVIDER_ID, userConfigPath, "user"), - }); - } else { - warnings.push(`Invalid JSON in ${userConfigPath}`); - } - } - } + for (const source of getConfigSources(ctx)) { + const parsed = await loadJsonConfig(source.path, configPath => { + warnings.push(`Invalid JSON in ${configPath}`); + }); + if (!parsed) continue; - // Project-level: opencode.json in project root - const projectConfigPath = path.join(ctx.cwd, "opencode.json"); - const content = await readFile(projectConfigPath); - if (content) { - const parsed = tryParseJson>(content); - if (parsed) { - items.push({ - path: projectConfigPath, - data: parsed, - level: "project", - _source: createSourceMeta(PROVIDER_ID, projectConfigPath, "project"), - }); - } else { - warnings.push(`Invalid JSON in ${projectConfigPath}`); - } + items.push({ + path: source.path, + data: parsed, + level: source.level, + _source: createSourceMeta(PROVIDER_ID, source.path, source.level), + }); } return { items, warnings }; diff --git a/packages/coding-agent/test/discovery/opencode.test.ts b/packages/coding-agent/test/discovery/opencode.test.ts index ac3f90b03..fde8fc237 100644 --- a/packages/coding-agent/test/discovery/opencode.test.ts +++ b/packages/coding-agent/test/discovery/opencode.test.ts @@ -1,8 +1,9 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test, vi } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import { type MCPServer, mcpCapability } from "@oh-my-pi/pi-coding-agent/capability/mcp"; +import { type Settings, settingsCapability } from "@oh-my-pi/pi-coding-agent/capability/settings"; import { loadCapability } from "@oh-my-pi/pi-coding-agent/discovery"; import { removeWithRetries } from "@oh-my-pi/pi-utils"; @@ -14,17 +15,172 @@ async function loadOpenCodeMcpConfig(cwd: string): Promise { return result.items; } +async function loadOpenCodeSettings(cwd: string): Promise { + const result = await loadCapability(settingsCapability.id, { + cwd, + providers: ["opencode"], + }); + return result.items; +} + describe("OpenCode MCP discovery", () => { let tempDir = ""; beforeEach(async () => { tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-opencode-mcp-")); + vi.spyOn(os, "homedir").mockReturnValue(tempDir); }); afterEach(async () => { + vi.restoreAllMocks(); await removeWithRetries(tempDir); }); + test("discovers commented JSONC config at user and project scopes", async () => { + const projectDir = path.join(tempDir, "project"); + const userConfigDir = path.join(tempDir, ".config", "opencode"); + await fs.mkdir(projectDir); + await fs.mkdir(userConfigDir, { recursive: true }); + + await fs.writeFile( + path.join(userConfigDir, "opencode.jsonc"), + `{ + // User-level OpenCode config + "model": "user-model", + "mcp": { + "user-jsonc": { + "type": "local", + "command": ["user-server"] + } + } + }`, + ); + await fs.writeFile( + path.join(projectDir, "opencode.jsonc"), + `{ + // Project-level OpenCode config + "model": "project-model", + "mcp": { + "project-jsonc": { + "type": "local", + "command": ["project-server"] + } + } + }`, + ); + + const [servers, discoveredSettings] = await Promise.all([ + loadOpenCodeMcpConfig(projectDir), + loadOpenCodeSettings(projectDir), + ]); + + expect(servers).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "user-jsonc", command: "user-server" }), + expect.objectContaining({ name: "project-jsonc", command: "project-server" }), + ]), + ); + expect(discoveredSettings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ level: "user", data: expect.objectContaining({ model: "user-model" }) }), + expect.objectContaining({ level: "project", data: expect.objectContaining({ model: "project-model" }) }), + ]), + ); + }); + + test("resolves same-named MCP servers by OpenCode precedence", async () => { + const projectDir = path.join(tempDir, "project"); + const userConfigDir = path.join(tempDir, ".config", "opencode"); + await fs.mkdir(projectDir); + await fs.mkdir(userConfigDir, { recursive: true }); + + // Lower precedence: user scope enables "shared" with the user command. + await fs.writeFile( + path.join(userConfigDir, "opencode.json"), + JSON.stringify({ + mcp: { shared: { type: "local", command: ["user-server"], enabled: true } }, + }), + ); + // Higher precedence: project opencode.json disables it with a different command. + await fs.writeFile( + path.join(projectDir, "opencode.json"), + JSON.stringify({ + mcp: { shared: { type: "local", command: ["project-json-server"], enabled: false } }, + }), + ); + // Highest precedence within the project scope: opencode.jsonc wins outright. + await fs.writeFile( + path.join(projectDir, "opencode.jsonc"), + `{ "mcp": { "shared": { "type": "local", "command": ["project-jsonc-server"], "enabled": false } } }`, + ); + + const servers = await loadOpenCodeMcpConfig(projectDir); + const shared = servers.filter(server => server.name === "shared"); + + expect(shared).toHaveLength(1); + expect(shared[0]).toMatchObject({ command: "project-jsonc-server", enabled: false }); + }); + + test("inherits lower-precedence fields on partial overrides", async () => { + const projectDir = path.join(tempDir, "project"); + const userConfigDir = path.join(tempDir, ".config", "opencode"); + await fs.mkdir(projectDir); + await fs.mkdir(userConfigDir, { recursive: true }); + + // User scope carries the full definition. + await fs.writeFile( + path.join(userConfigDir, "opencode.json"), + JSON.stringify({ + mcp: { + github: { + type: "local", + command: ["gh-server"], + environment: { TOKEN: "user-token" }, + }, + }, + }), + ); + // Project scope overrides only a single field; command/env must survive. + await fs.writeFile( + path.join(projectDir, "opencode.jsonc"), + `{ "mcp": { "github": { "timeout": 5000, "environment": { "REGION": "eu" } } } }`, + ); + + const servers = await loadOpenCodeMcpConfig(projectDir); + const github = servers.filter(server => server.name === "github"); + + expect(github).toHaveLength(1); + expect(github[0]).toMatchObject({ + command: "gh-server", + transport: "stdio", + timeout: 5000, + env: { TOKEN: "user-token", REGION: "eu" }, + }); + }); + + test("parses comments in opencode.json", async () => { + await fs.writeFile( + path.join(tempDir, "opencode.json"), + `{ + // OpenCode parses either extension as JSONC. + "mcp": { + "commented-json": { + "type": "local", + "command": ["commented-server"] + } + } + }`, + ); + + const servers = await loadOpenCodeMcpConfig(tempDir); + + expect(servers).toEqual([ + expect.objectContaining({ + name: "commented-json", + command: "commented-server", + }), + ]); + }); test("normalizes array commands and OpenCode environment fields", async () => { await fs.writeFile( path.join(tempDir, "opencode.json"),