From 1441fdb28b2d1df1308dde3f34c5260c4ff6f54a Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 17 Aug 2026 01:31:00 +0000 Subject: [PATCH] fix(discovery): expanded OpenCode {env:} and {file:} config tokens OpenCode substitutes {env:VAR} and {file:path} in config text at load time, but OMP's OpenCode discovery ran the generic ${VAR}-only expandEnvVarsDeep, leaving those tokens literal. An MCP header like `Bearer {env:MCP_KEY}` reached the server verbatim and returned 401. The OpenCode loader now applies OpenCode's own substitution to raw config text before parsing: {env:VAR} -> env value or empty string, {file:path} -> trimmed, JSON-escaped file contents resolved relative to the config dir / ~ / absolute, skipping tokens on // comment lines. Fixes #8778 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/discovery/opencode.ts | 59 ++++++++++++++++++- .../test/discovery/opencode.test.ts | 46 +++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4901d84f7..526bc6d36 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed OpenCode MCP servers 401ing when config used OpenCode's `{env:VAR}`/`{file:path}` substitution (e.g. `Bearer {env:MCP_KEY}` headers); the OpenCode loader now expands those tokens the way OpenCode does instead of only `${VAR}` ([#8778](https://github.com/can1357/oh-my-pi/issues/8778)). + ## [17.3.5] - 2026-08-16 ### Added diff --git a/packages/coding-agent/src/discovery/opencode.ts b/packages/coding-agent/src/discovery/opencode.ts index 2a422d4e9..3c22fa3c8 100644 --- a/packages/coding-agent/src/discovery/opencode.ts +++ b/packages/coding-agent/src/discovery/opencode.ts @@ -15,6 +15,7 @@ * * Priority: 55 (tool-specific provider) */ +import * as os from "node:os"; import * as path from "node:path"; import { isRecord, logger, parseFrontmatter } from "@oh-my-pi/pi-utils"; import { JSONC } from "bun"; @@ -33,7 +34,6 @@ import { buildExtensionModuleItems, createSourceMeta, discoverExtensionModulePaths, - expandEnvVarsDeep, getProjectPath, getUserPath, loadFilesFromDir, @@ -63,7 +63,7 @@ async function loadJsonConfig( let parsed: unknown; try { - parsed = JSONC.parse(content); + parsed = JSONC.parse(await substituteConfigVars(content, configPath)); } catch { onInvalid(configPath); return null; @@ -75,6 +75,59 @@ async function loadJsonConfig( return parsed; } +/** + * Apply OpenCode's config variable substitution to raw config text. + * + * OpenCode expands `{env:VAR}` (the env value, or an empty string when unset) + * and `{file:path}` (file contents, trimmed and JSON-escaped) at load time, + * before the JSON is parsed — see opencode `packages/opencode/src/config/variable.ts`. + * OMP loads the same config files, so it MUST honor the same syntax; the generic + * `${VAR}` expansion used elsewhere never matches, leaving a header like + * `Bearer {env:MCP_KEY}` to reach the MCP server verbatim and 401 (#8778). + * + * `{file:path}` resolves relative to the config file's directory, or from a `~`/ + * absolute path. A token on a `//` comment line is left untouched, matching + * OpenCode. A missing file expands to an empty string (OpenCode's `missing: + * "empty"` mode) rather than aborting discovery. + */ +async function substituteConfigVars(text: string, configPath: string): Promise { + const envExpanded = text.replace(/\{env:([^}]+)\}/g, (_, name: string) => Bun.env[name] ?? ""); + + const fileMatches = [...envExpanded.matchAll(/\{file:[^}]+\}/g)]; + if (fileMatches.length === 0) return envExpanded; + + const configDir = path.dirname(configPath); + let out = ""; + let cursor = 0; + for (const match of fileMatches) { + const token = match[0]; + const index = match.index ?? 0; + out += envExpanded.slice(cursor, index); + cursor = index + token.length; + + // A `{file:...}` sitting on a JSONC comment line is not a real reference. + const lineStart = envExpanded.lastIndexOf("\n", index - 1) + 1; + if (envExpanded.slice(lineStart, index).trimStart().startsWith("//")) { + out += token; + continue; + } + + let filePath = token.slice("{file:".length, -1); + if (filePath.startsWith("~/")) filePath = path.join(os.homedir(), filePath.slice(2)); + const resolved = path.isAbsolute(filePath) ? filePath : path.resolve(configDir, filePath); + + const fileContent = await readFile(resolved); + if (fileContent === null) { + logger.warn("OpenCode config references a missing file", { configPath, path: resolved }); + continue; + } + // JSON-escape so multi-line/quoted contents stay valid inside the string literal. + out += JSON.stringify(fileContent.trim()).slice(1, -1); + } + out += envExpanded.slice(cursor); + return out; +} + /** * OpenCode config sources in ascending effective precedence (lowest first): * user `opencode.json` → user `opencode.jsonc` → project-root @@ -215,7 +268,7 @@ async function loadMCPServers(ctx: LoadContext): Promise> const items: MCPServer[] = []; for (const [name, config] of mergedByName) { - const serverConfig = expandEnvVarsDeep(config) as OpenCodeMCPConfig; + const serverConfig = config as OpenCodeMCPConfig; const source = sourceByName.get(name)!; items.push(buildMCPServer(name, serverConfig, source)); } diff --git a/packages/coding-agent/test/discovery/opencode.test.ts b/packages/coding-agent/test/discovery/opencode.test.ts index 386357af3..468f357e8 100644 --- a/packages/coding-agent/test/discovery/opencode.test.ts +++ b/packages/coding-agent/test/discovery/opencode.test.ts @@ -287,4 +287,50 @@ describe("OpenCode MCP discovery", () => { expect(server?.command).toBe("server-bin"); expect(server?.args).toBeUndefined(); }); + + test("expands OpenCode {env:VAR} and {file:path} substitutions", async () => { + const secretFile = path.join(tempDir, "mcp-key.txt"); + await fs.writeFile(secretFile, "file-token\n"); + await fs.writeFile( + path.join(tempDir, "opencode.json"), + JSON.stringify({ + mcp: { + "env-server": { + type: "remote", + url: "https://mcp.example.xyz/{env:OMP_TEST_MCP_PATH}", + headers: { Authorization: "Bearer {env:OMP_TEST_MCP_KEY}" }, + }, + "file-server": { + type: "remote", + url: "https://mcp.example.xyz/mcp", + headers: { Authorization: "Bearer {file:./mcp-key.txt}" }, + }, + "missing-server": { + type: "remote", + url: "https://mcp.example.xyz/mcp", + headers: { Authorization: "Bearer {env:OMP_TEST_MCP_ABSENT}" }, + }, + }, + }), + ); + + delete Bun.env.OMP_TEST_MCP_ABSENT; + Bun.env.OMP_TEST_MCP_KEY = "secret-token"; + Bun.env.OMP_TEST_MCP_PATH = "mcp/server"; + try { + const servers = await loadOpenCodeMcpConfig(tempDir); + const byName = Object.fromEntries(servers.map(server => [server.name, server])); + + expect(byName["env-server"]).toMatchObject({ + url: "https://mcp.example.xyz/mcp/server", + headers: { Authorization: "Bearer secret-token" }, + }); + expect(byName["file-server"]?.headers).toEqual({ Authorization: "Bearer file-token" }); + // Unset env expands to empty string, matching OpenCode — never the literal token. + expect(byName["missing-server"]?.headers).toEqual({ Authorization: "Bearer " }); + } finally { + delete Bun.env.OMP_TEST_MCP_KEY; + delete Bun.env.OMP_TEST_MCP_PATH; + } + }); });