From 2a5ca60f3054c8eb5ce9346f314964486e8f4902 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 4 Apr 2026 08:44:34 -0700 Subject: [PATCH 1/2] fix: normalize `mcpServers` config key to `mcp` and transform external MCP formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Auto-normalize top-level `mcpServers` → `mcp` (used by Claude Code, Cursor, `.mcp.json`) - Transform external MCP server format: string `command` + `args` + `env` → array `command` + `environment` - Handle remote servers without explicit `type` field (infer from `url` presence) - Always delete `mcpServers` to prevent strict schema rejection (even when `mcp` exists) - Normalize typed entries that use external field names (`env`, string `command`) - Add 5 tests: string command, array command, both-keys conflict, remote server, typed entry normalization Closes #638 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/opencode/src/config/config.ts | 105 ++++++++++-- packages/opencode/test/config/config.test.ts | 171 +++++++++++++++++++ 2 files changed, 262 insertions(+), 14 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 7e75fe95b2..0f9d763ffe 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -143,10 +143,10 @@ export namespace Config { for (const dir of unique(directories)) { // altimate_change start - support both .altimate-code and .opencode config dirs if (dir.endsWith(".altimate-code") || dir.endsWith(".opencode") || dir === Flag.OPENCODE_CONFIG_DIR) { - // altimate_change end + // altimate_change end // altimate_change start - support altimate-code.json config filename for (const file of ["altimate-code.json", "opencode.jsonc", "opencode.json"]) { - // altimate_change end + // altimate_change end log.debug(`loading config from ${path.join(dir, file)}`) result = mergeConfigConcatArrays(result, await loadFile(path.join(dir, file))) // to satisfy the type checker @@ -214,7 +214,7 @@ export namespace Config { if (existsSync(managedDir)) { // altimate_change start - support altimate-code.json config filename for (const file of ["altimate-code.json", "opencode.jsonc", "opencode.json"]) { - // altimate_change end + // altimate_change end result = mergeConfigConcatArrays(result, await loadFile(path.join(managedDir, file))) } } @@ -1237,7 +1237,9 @@ export namespace Config { enabled: z .boolean() .optional() - .describe("Enable session tracing (default: true). Traces are saved locally and can be viewed with `altimate-code trace`."), + .describe( + "Enable session tracing (default: true). Traces are saved locally and can be viewed with `altimate-code trace`.", + ), dir: z .string() .optional() @@ -1247,7 +1249,9 @@ export namespace Config { .int() .nonnegative() .optional() - .describe("Maximum number of trace files to keep. 0 for unlimited. Oldest files are removed when exceeded (default: 100)."), + .describe( + "Maximum number of trace files to keep. 0 for unlimited. Oldest files are removed when exceeded (default: 100).", + ), exporters: z .array( z.object({ @@ -1292,13 +1296,17 @@ export namespace Config { env_fingerprint_skill_selection: z .boolean() .optional() - .describe("Use environment fingerprint to select relevant skills once per session (default: false). Set to true to enable LLM-based skill filtering."), + .describe( + "Use environment fingerprint to select relevant skills once per session (default: false). Set to true to enable LLM-based skill filtering.", + ), // altimate_change end // altimate_change start - auto MCP discovery toggle auto_mcp_discovery: z .boolean() .default(true) - .describe("Auto-discover MCP servers from VS Code, Claude Code, Copilot, and Gemini configs at startup. Set to false to disable."), + .describe( + "Auto-discover MCP servers from VS Code, Claude Code, Copilot, and Gemini configs at startup. Set to false to disable.", + ), // altimate_change end }) .optional(), @@ -1351,6 +1359,65 @@ export namespace Config { return load(text, { path: filepath }) } + // altimate_change start — shared normalization for external MCP config formats + /** + * Normalize a raw config object to handle common misconfigurations: + * 1. Top-level "mcpServers" key → "mcp" (used by Claude Code, Cursor, etc.) + * 2. Individual server entries in external format (string command + args + env) + * → altimate-code format (command array + environment) + * + * Returns a new object with normalized config, leaving the original unchanged. + * This prevents disk mutation when configs are written back via updateGlobal(). + */ + function normalizeMcpConfig(data: Record, source: string): Record { + const result = { ...data } + // Normalize top-level key — always delete mcpServers to prevent strict schema rejection + if ("mcpServers" in result) { + if (!("mcp" in result)) { + result.mcp = result.mcpServers + log.warn("'mcpServers' is not a valid config key; use 'mcp' instead — auto-correcting", { path: source }) + } else { + log.debug("Both 'mcp' and 'mcpServers' exist; ignoring 'mcpServers'", { path: source }) + } + delete result.mcpServers + } + // Normalize individual MCP server entries from external formats + if (result.mcp && typeof result.mcp === "object" && !Array.isArray(result.mcp)) { + const servers = { ...(result.mcp as Record) } + for (const [name, entry] of Object.entries(servers)) { + if (!entry || typeof entry !== "object") continue + // Build a normalized entry — Handles both untyped and typed entries with external fields + if (entry.command || entry.args) { + const cmd = Array.isArray(entry.command) + ? entry.command.map(String) + : [ + String(entry.command), + ...(Array.isArray(entry.args) + ? entry.args.map(String) + : typeof entry.args === "string" + ? [entry.args] + : []), + ] + const transformed: Record = { type: "local", command: cmd } + if (entry.env && typeof entry.env === "object") transformed.environment = entry.env + if (entry.environment && typeof entry.environment === "object") transformed.environment = entry.environment + if (typeof entry.timeout === "number") transformed.timeout = entry.timeout + if (typeof entry.enabled === "boolean") transformed.enabled = entry.enabled + servers[name] = transformed + } else if (entry.url && typeof entry.url === "string") { + const transformed: Record = { type: "remote", url: entry.url } + if (entry.headers && typeof entry.headers === "object") transformed.headers = entry.headers + if (typeof entry.timeout === "number") transformed.timeout = entry.timeout + if (typeof entry.enabled === "boolean") transformed.enabled = entry.enabled + servers[name] = transformed + } + } + result.mcp = servers + } + return result + } + // altimate_change end + async function load(text: string, options: { path: string } | { dir: string; source: string }) { const original = text const source = "path" in options ? options.path : options.source @@ -1364,12 +1431,15 @@ export namespace Config { if (!data || typeof data !== "object" || Array.isArray(data)) return data const copy = { ...(data as Record) } const hadLegacy = "theme" in copy || "keybinds" in copy || "tui" in copy - if (!hadLegacy) return copy - delete copy.theme - delete copy.keybinds - delete copy.tui - log.warn("tui keys in opencode config are deprecated; move them to tui.json", { path: source }) - return copy + if (hadLegacy) { + delete copy.theme + delete copy.keybinds + delete copy.tui + log.warn("tui keys in opencode config are deprecated; move them to tui.json", { path: source }) + } + // altimate_change start — normalize mcpServers to mcp (common key used by other AI tools) + return normalizeMcpConfig(copy, source) + // altimate_change end })() const parsed = Info.safeParse(normalized) @@ -1489,7 +1559,14 @@ export namespace Config { }) } - const parsed = Info.safeParse(data) + // altimate_change start — normalize mcpServers to mcp in parseConfig + const normalized = + data && typeof data === "object" && !Array.isArray(data) + ? normalizeMcpConfig(data as Record, filepath) + : data + // altimate_change end + + const parsed = Info.safeParse(normalized) if (parsed.success) return parsed.data throw new InvalidError({ diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index d90b0ce4ad..68cd890a2b 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -343,6 +343,150 @@ test("handles file inclusion with replacement tokens", async () => { }) }) +test("normalizes mcpServers key to mcp", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + gitlab: { + command: "npx", + args: ["-y", "@modelcontextprotocol/server-gitlab"], + env: { + GITLAB_PERSONAL_ACCESS_TOKEN: "my-token", + GITLAB_API_URL: "https://gitlab.com/api/v4", + }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + expect(config.mcp).toBeDefined() + expect(config.mcp!.gitlab).toBeDefined() + const gitlab = config.mcp!.gitlab as any + expect(gitlab.type).toBe("local") + expect(gitlab.command).toEqual(["npx", "-y", "@modelcontextprotocol/server-gitlab"]) + expect(gitlab.environment).toEqual({ + GITLAB_PERSONAL_ACCESS_TOKEN: "my-token", + GITLAB_API_URL: "https://gitlab.com/api/v4", + }) + }, + }) +}) + +test("normalizes mcpServers with array command format", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myserver: { + command: ["node", "server.js"], + env: { PORT: "3000" }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myserver as any + expect(server.type).toBe("local") + expect(server.command).toEqual(["node", "server.js"]) + expect(server.environment).toEqual({ PORT: "3000" }) + }, + }) +}) + +test("does not overwrite existing mcp key with mcpServers", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcp: { + existing: { + type: "local", + command: ["node", "existing.js"], + }, + }, + mcpServers: { + shouldbeignored: { + command: "node", + args: ["ignored.js"], + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + // When both mcp and mcpServers exist, mcpServers is silently dropped + // and mcp is preserved. No validation error. + const config = await Config.get() + expect(config.mcp).toBeDefined() + expect(config.mcp!.existing).toBeDefined() + expect((config.mcp! as any).shouldbeignored).toBeUndefined() + }, + }) +}) + +test("normalizes remote server without type field", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myremote: { + url: "https://example.com/mcp", + headers: { Authorization: "Bearer token" }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myremote as any + expect(server.type).toBe("remote") + expect(server.url).toBe("https://example.com/mcp") + expect(server.headers).toEqual({ Authorization: "Bearer token" }) + }, + }) +}) + +test("normalizes typed entry with external field names", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myserver: { + type: "local", + command: "npx", + args: ["-y", "some-server"], + env: { TOKEN: "abc" }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + // Even with type: "local", string command + args + env should be normalized + const config = await Config.get() + const server = config.mcp!.myserver as any + expect(server.type).toBe("local") + expect(server.command).toEqual(["npx", "-y", "some-server"]) + expect(server.environment).toEqual({ TOKEN: "abc" }) + }, + }) +}) + test("validates config schema and throws on invalid fields", async () => { await using tmp = await tmpdir({ init: async (dir) => { @@ -2087,3 +2231,30 @@ describe("OPENCODE_CONFIG_CONTENT token substitution", () => { } }) }) + +test("normalizes native config with string command and args", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcp: { + myserver: { + type: "local", + command: "node", + args: ["server.js"], + env: { PORT: "3000" }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myserver as any + expect(server.type).toBe("local") + expect(server.command).toEqual(["node", "server.js"]) + expect(server.environment).toEqual({ PORT: "3000" }) + }, + }) +}) From 4ca0b3fb6d80869cc518c036c1d2d0f76fd01fff Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 4 Apr 2026 08:55:00 -0700 Subject: [PATCH 2/2] test: add adversarial tests for `mcpServers` config normalization - Empty `mcpServers` object - Non-object `mcpServers` value (string) - Null server entries removed during normalization - Numeric/boolean args coerced to strings - String `args` field wrapped in array - Disabled server entry (`{ enabled: false }`) - `environment` takes precedence over `env` - `timeout` and `enabled` fields preserved - Mixed local and remote servers - Special characters in server names Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/opencode/src/config/config.ts | 7 +- packages/opencode/test/config/config.test.ts | 243 +++++++++++++++++++ 2 files changed, 248 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 0f9d763ffe..e687bb74f7 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -1385,8 +1385,11 @@ export namespace Config { if (result.mcp && typeof result.mcp === "object" && !Array.isArray(result.mcp)) { const servers = { ...(result.mcp as Record) } for (const [name, entry] of Object.entries(servers)) { - if (!entry || typeof entry !== "object") continue - // Build a normalized entry — Handles both untyped and typed entries with external fields + if (!entry || typeof entry !== "object") { + delete servers[name] + continue + } + // Build a normalized entry — handles both untyped and typed entries with external fields if (entry.command || entry.args) { const cmd = Array.isArray(entry.command) ? entry.command.map(String) diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 68cd890a2b..633367ad39 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -487,6 +487,249 @@ test("normalizes typed entry with external field names", async () => { }) }) +// --- Adversarial tests for mcpServers normalization --- + +test("mcpServers normalization: empty mcpServers object loads without error", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { mcpServers: {} }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + // Empty mcp is fine — no servers, no crash + expect(config.mcp).toEqual({}) + }, + }) +}) + +test("mcpServers normalization: non-object mcpServers value is ignored", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { mcpServers: "not-an-object" }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + // String value for mcp will fail schema validation (expects record) + await expect(Config.get()).rejects.toThrow() + }, + }) +}) + +test("mcpServers normalization: null/undefined server entries are removed", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + valid: { command: "node", args: ["server.js"] }, + nullEntry: null, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + expect(config.mcp).toBeDefined() + // Valid entry normalized, null entry removed + const valid = config.mcp!.valid as any + expect(valid.type).toBe("local") + expect(valid.command).toEqual(["node", "server.js"]) + expect((config.mcp! as any).nullEntry).toBeUndefined() + }, + }) +}) + +test("mcpServers normalization: special characters in server names are handled", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + "my-server_v2.0": { command: "node", args: ["server.js"] }, + "server@prod": { url: "https://prod.example.com/mcp" }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + expect(config.mcp).toBeDefined() + const local = config.mcp!["my-server_v2.0"] as any + expect(local.type).toBe("local") + expect(local.command).toEqual(["node", "server.js"]) + const remote = config.mcp!["server@prod"] as any + expect(remote.type).toBe("remote") + expect(remote.url).toBe("https://prod.example.com/mcp") + // Object prototype should not be polluted + expect(({} as any).command).toBeUndefined() + }, + }) +}) + +test("mcpServers normalization: numeric and boolean args are coerced to strings", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myserver: { + command: "node", + args: [42, true, "normal"], + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myserver as any + expect(server.command).toEqual(["node", "42", "true", "normal"]) + }, + }) +}) + +test("mcpServers normalization: string args field is wrapped in array", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myserver: { + command: "npx", + args: "--verbose", + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myserver as any + expect(server.command).toEqual(["npx", "--verbose"]) + }, + }) +}) + +test("mcpServers normalization: disabled server entry is valid", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + disabled: { enabled: false }, + valid: { command: "node", args: ["server.js"] }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + // Both entries are valid — disabled is a valid schema shape + const valid = config.mcp!.valid as any + expect(valid.type).toBe("local") + const disabled = config.mcp!.disabled as any + expect(disabled.enabled).toBe(false) + }, + }) +}) + +test("mcpServers normalization: environment takes precedence over env when both present", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myserver: { + command: "node", + args: ["server.js"], + env: { FROM_ENV: "old" }, + environment: { FROM_ENVIRONMENT: "new" }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myserver as any + // environment should win over env since it's checked second + expect(server.environment).toEqual({ FROM_ENVIRONMENT: "new" }) + }, + }) +}) + +test("mcpServers normalization: timeout and enabled are preserved", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + myserver: { + command: "node", + args: ["server.js"], + timeout: 10000, + enabled: false, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const server = config.mcp!.myserver as any + expect(server.timeout).toBe(10000) + expect(server.enabled).toBe(false) + }, + }) +}) + +test("mcpServers normalization: mixed local and remote servers", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await writeConfig(dir, { + mcpServers: { + localServer: { + command: "npx", + args: ["-y", "@mcp/server-fs"], + env: { HOME: "/tmp" }, + }, + remoteServer: { + url: "https://api.example.com/mcp", + headers: { "X-Api-Key": "secret" }, + }, + }, + }) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + const local = config.mcp!.localServer as any + expect(local.type).toBe("local") + expect(local.command).toEqual(["npx", "-y", "@mcp/server-fs"]) + expect(local.environment).toEqual({ HOME: "/tmp" }) + + const remote = config.mcp!.remoteServer as any + expect(remote.type).toBe("remote") + expect(remote.url).toBe("https://api.example.com/mcp") + expect(remote.headers).toEqual({ "X-Api-Key": "secret" }) + }, + }) +}) + test("validates config schema and throws on invalid fields", async () => { await using tmp = await tmpdir({ init: async (dir) => {