From 2216e7f0775253e38c1a822760398674330b8eac Mon Sep 17 00:00:00 2001 From: conradkoh Date: Fri, 19 Dec 2025 11:23:11 +0800 Subject: [PATCH 1/6] fix(config): resolve {file:path} shadowing when same path appears in comments Fixes a bug where {file:path} template expansions fail when the same file path appears earlier in a commented line. The issue was caused by findIndex() always returning the first occurrence of a match, regardless of which specific occurrence was being processed. Changed the logic to: 1. Track all match positions and their line numbers upfront 2. Process matches in reverse order to preserve string indices 3. Check each specific occurrence's line for comments This ensures that commented-out file references don't prevent valid references from being expanded. Adds reproduction test case demonstrating the shadowing bug. --- packages/opencode/src/config/config.ts | 34 ++++++- packages/opencode/test/config/config.test.ts | 99 ++++++++++++++++++++ 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index a01cc832a09..22170647cbf 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -855,16 +855,38 @@ export namespace Config { return process.env[varName] || "" }) - const fileMatches = text.match(/\{file:[^}]+\}/g) + const fileRegex = /\{file:[^}]+\}/g + const fileMatches = text.match(fileRegex) if (fileMatches) { const configDir = path.dirname(configFilepath) const lines = text.split("\n") + // Track positions of all matches in the original text + const matchPositions: Array<{ match: string; index: number; lineNum: number }> = [] + let currentIndex = 0 + for (const match of fileMatches) { - const lineIndex = lines.findIndex((line) => line.includes(match)) - if (lineIndex !== -1 && lines[lineIndex].trim().startsWith("//")) { - continue // Skip if line is commented + const index = text.indexOf(match, currentIndex) + if (index === -1) continue + + // Count line number + const beforeMatch = text.substring(0, index) + const lineNum = beforeMatch.split("\n").length - 1 + + matchPositions.push({ match, index, lineNum }) + currentIndex = index + match.length + } + + // Process matches in reverse order to preserve indices + for (let i = matchPositions.length - 1; i >= 0; i--) { + const { match, index, lineNum } = matchPositions[i] + const line = lines[lineNum] + + // Skip if this specific occurrence is on a commented line + if (line.trim().startsWith("//")) { + continue } + let filePath = match.replace(/^\{file:/, "").replace(/\}$/, "") if (filePath.startsWith("~/")) { filePath = path.join(os.homedir(), filePath.slice(2)) @@ -888,7 +910,9 @@ export namespace Config { }) ).trim() // escape newlines/quotes, strip outer quotes - text = text.replace(match, JSON.stringify(fileContent).slice(1, -1)) + const replacement = JSON.stringify(fileContent).slice(1, -1) + // Replace at the specific index + text = text.substring(0, index) + replacement + text.substring(index + match.length) } } diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 2ff8c01cdb0..42657d11301 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -148,6 +148,105 @@ test("handles file inclusion substitution", async () => { }) }) +test("reproduces shadowing bug: file reference fails when same path appears in comment", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const promptsDir = path.join(dir, "prompts") + await fs.mkdir(promptsDir, { recursive: true }) + await Bun.write(path.join(promptsDir, "precise.md"), "This is the precise prompt.") + + // Write JSONC with comment containing same file reference + await Bun.write( + path.join(dir, "opencode.jsonc"), + `{ + "$schema": "https://opencode.ai/config.json", + "agent": { + // "experimental": { + // "prompt": "{file:./prompts/precise.md}" + // }, + "precise": { + "prompt": "{file:./prompts/precise.md}" + } + } +}`, + ) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + // BUG: The prompt remains as "{file:./prompts/precise.md}" instead of being expanded + // because findIndex finds the commented line first + expect(config.agent?.["precise"]?.prompt).toBe("This is the precise prompt.") + }, + }) +}) + +test("handles multiple agents with different file references", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const promptsDir = path.join(dir, "prompts") + await fs.mkdir(promptsDir, { recursive: true }) + await Bun.write(path.join(promptsDir, "prompt-a.md"), "Prompt A content") + await Bun.write(path.join(promptsDir, "prompt-b.md"), "Prompt B content") + await Bun.write( + path.join(dir, "opencode.json"), + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + agent: { + agent_a: { + prompt: "{file:./prompts/prompt-a.md}", + }, + agent_b: { + prompt: "{file:./prompts/prompt-b.md}", + }, + }, + }), + ) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const config = await Config.get() + expect(config.agent?.["agent_a"]?.prompt).toBe("Prompt A content") + expect(config.agent?.["agent_b"]?.prompt).toBe("Prompt B content") + }, + }) +}) + +test("throws error when file inclusion reference fails", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write( + path.join(dir, "opencode.json"), + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + agent: { + test: { + prompt: "{file:./prompts/precise.md}", + }, + }, + }), + ) + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + try { + await Config.get() + expect.unreachable() + } catch (error: any) { + expect(error.name).toBe("ConfigInvalidError") + expect(error.data.message).toMatch(/bad file reference/) + expect(error.data.message).toMatch(/precise\.md does not exist/) + } + }, + }) +}) + test("validates config schema and throws on invalid fields", async () => { await using tmp = await tmpdir({ init: async (dir) => { From f1ce98a4e906092fbffa852cf23d2ed5b79ecc20 Mon Sep 17 00:00:00 2001 From: conradkoh Date: Fri, 19 Dec 2025 11:24:35 +0800 Subject: [PATCH 2/6] refactor(config): parse JSONC before template expansion Refactors the config loading system to parse JSONC first, then expand {env:VAR} and {file:path} templates within the resulting object tree. Benefits of this approach: - JSONC parser correctly handles comments (no manual checking needed) - No string injection vulnerabilities - Direct value replacement is safer than string manipulation - Template expansion is context-aware - Cleaner separation of concerns The new architecture: 1. Parse JSONC text into JavaScript object 2. Walk the object tree recursively 3. Expand templates in string values only This eliminates the fragile comment detection logic and makes the template expansion more robust and maintainable. --- packages/opencode/src/config/config.ts | 107 +++++++++++++------------ 1 file changed, 54 insertions(+), 53 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 22170647cbf..3ccfc79c48d 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -850,72 +850,69 @@ export namespace Config { return load(text, filepath) } - async function load(text: string, configFilepath: string) { - text = text.replace(/\{env:([^}]+)\}/g, (_, varName) => { + async function expandTemplates(value: any, configDir: string, configFilepath: string): Promise { + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + if (typeof value[i] === "string") { + value[i] = await expandString(value[i], configDir, configFilepath) + } else { + await expandTemplates(value[i], configDir, configFilepath) + } + } + return + } + + if (value && typeof value === "object") { + for (const key in value) { + if (typeof value[key] === "string") { + value[key] = await expandString(value[key], configDir, configFilepath) + } else { + await expandTemplates(value[key], configDir, configFilepath) + } + } + return + } + } + + async function expandString(str: string, configDir: string, configFilepath: string): Promise { + // Expand {env:VAR} templates + str = str.replace(/\{env:([^}]+)\}/g, (_, varName) => { return process.env[varName] || "" }) - const fileRegex = /\{file:[^}]+\}/g - const fileMatches = text.match(fileRegex) + // Expand {file:path} templates + const fileMatches = str.match(/\{file:[^}]+\}/g) if (fileMatches) { - const configDir = path.dirname(configFilepath) - const lines = text.split("\n") - - // Track positions of all matches in the original text - const matchPositions: Array<{ match: string; index: number; lineNum: number }> = [] - let currentIndex = 0 - for (const match of fileMatches) { - const index = text.indexOf(match, currentIndex) - if (index === -1) continue - - // Count line number - const beforeMatch = text.substring(0, index) - const lineNum = beforeMatch.split("\n").length - 1 - - matchPositions.push({ match, index, lineNum }) - currentIndex = index + match.length - } - - // Process matches in reverse order to preserve indices - for (let i = matchPositions.length - 1; i >= 0; i--) { - const { match, index, lineNum } = matchPositions[i] - const line = lines[lineNum] - - // Skip if this specific occurrence is on a commented line - if (line.trim().startsWith("//")) { - continue - } - let filePath = match.replace(/^\{file:/, "").replace(/\}$/, "") if (filePath.startsWith("~/")) { filePath = path.join(os.homedir(), filePath.slice(2)) } const resolvedPath = path.isAbsolute(filePath) ? filePath : path.resolve(configDir, filePath) - const fileContent = ( - await Bun.file(resolvedPath) - .text() - .catch((error) => { - const errMsg = `bad file reference: "${match}"` - if (error.code === "ENOENT") { - throw new InvalidError( - { - path: configFilepath, - message: errMsg + ` ${resolvedPath} does not exist`, - }, - { cause: error }, - ) - } - throw new InvalidError({ path: configFilepath, message: errMsg }, { cause: error }) - }) - ).trim() - // escape newlines/quotes, strip outer quotes - const replacement = JSON.stringify(fileContent).slice(1, -1) - // Replace at the specific index - text = text.substring(0, index) + replacement + text.substring(index + match.length) + const fileContent = await Bun.file(resolvedPath) + .text() + .catch((error) => { + const errMsg = `bad file reference: "${match}"` + if (error.code === "ENOENT") { + throw new InvalidError( + { + path: configFilepath, + message: errMsg + ` ${resolvedPath} does not exist`, + }, + { cause: error }, + ) + } + throw new InvalidError({ path: configFilepath, message: errMsg }, { cause: error }) + }) + str = str.replace(match, fileContent.trim()) } } + return str + } + + async function load(text: string, configFilepath: string) { + // Parse JSONC first, letting the parser handle comments correctly const errors: JsoncParseError[] = [] const data = parseJsonc(text, errors, { allowTrailingComma: true }) if (errors.length) { @@ -940,6 +937,10 @@ export namespace Config { }) } + // Expand templates in the parsed object tree + const configDir = path.dirname(configFilepath) + await expandTemplates(data, configDir, configFilepath) + const parsed = Info.safeParse(data) if (parsed.success) { if (!parsed.data.$schema) { From 065d5556c98d54aaf73fade1a1ccdebea96318cd Mon Sep 17 00:00:00 2001 From: conradkoh Date: Fri, 19 Dec 2025 11:57:38 +0800 Subject: [PATCH 3/6] perf(config): parallelize file reads during template expansion --- packages/opencode/src/config/config.ts | 107 ++++++++++++++++++------- 1 file changed, 78 insertions(+), 29 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 3ccfc79c48d..01a7a596534 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -851,12 +851,44 @@ export namespace Config { } async function expandTemplates(value: any, configDir: string, configFilepath: string): Promise { + // First pass: collect all file references + const fileRefs = new Map() // match -> resolvedPath + collectFileReferences(value, configDir, fileRefs) + + // Read all files in parallel + const fileContents = new Map() + await Promise.all( + Array.from(fileRefs.entries()).map(async ([match, resolvedPath]) => { + const content = await Bun.file(resolvedPath) + .text() + .catch((error) => { + const errMsg = `bad file reference: "${match}"` + if (error.code === "ENOENT") { + throw new InvalidError( + { + path: configFilepath, + message: errMsg + ` ${resolvedPath} does not exist`, + }, + { cause: error }, + ) + } + throw new InvalidError({ path: configFilepath, message: errMsg }, { cause: error }) + }) + fileContents.set(match, content.trim()) + }), + ) + + // Second pass: expand templates in-place + replaceTemplates(value, fileContents) + } + + function collectFileReferences(value: any, configDir: string, fileRefs: Map): void { if (Array.isArray(value)) { - for (let i = 0; i < value.length; i++) { - if (typeof value[i] === "string") { - value[i] = await expandString(value[i], configDir, configFilepath) + for (const item of value) { + if (typeof item === "string") { + extractFileReferences(item, configDir, fileRefs) } else { - await expandTemplates(value[i], configDir, configFilepath) + collectFileReferences(item, configDir, fileRefs) } } return @@ -865,47 +897,64 @@ export namespace Config { if (value && typeof value === "object") { for (const key in value) { if (typeof value[key] === "string") { - value[key] = await expandString(value[key], configDir, configFilepath) + extractFileReferences(value[key], configDir, fileRefs) } else { - await expandTemplates(value[key], configDir, configFilepath) + collectFileReferences(value[key], configDir, fileRefs) } } return } } - async function expandString(str: string, configDir: string, configFilepath: string): Promise { - // Expand {env:VAR} templates - str = str.replace(/\{env:([^}]+)\}/g, (_, varName) => { - return process.env[varName] || "" - }) - - // Expand {file:path} templates + function extractFileReferences(str: string, configDir: string, fileRefs: Map): void { const fileMatches = str.match(/\{file:[^}]+\}/g) if (fileMatches) { for (const match of fileMatches) { + if (fileRefs.has(match)) continue + let filePath = match.replace(/^\{file:/, "").replace(/\}$/, "") if (filePath.startsWith("~/")) { filePath = path.join(os.homedir(), filePath.slice(2)) } const resolvedPath = path.isAbsolute(filePath) ? filePath : path.resolve(configDir, filePath) - const fileContent = await Bun.file(resolvedPath) - .text() - .catch((error) => { - const errMsg = `bad file reference: "${match}"` - if (error.code === "ENOENT") { - throw new InvalidError( - { - path: configFilepath, - message: errMsg + ` ${resolvedPath} does not exist`, - }, - { cause: error }, - ) - } - throw new InvalidError({ path: configFilepath, message: errMsg }, { cause: error }) - }) - str = str.replace(match, fileContent.trim()) + fileRefs.set(match, resolvedPath) + } + } + } + + function replaceTemplates(value: any, fileContents: Map): void { + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + if (typeof value[i] === "string") { + value[i] = expandString(value[i], fileContents) + } else { + replaceTemplates(value[i], fileContents) + } } + return + } + + if (value && typeof value === "object") { + for (const key in value) { + if (typeof value[key] === "string") { + value[key] = expandString(value[key], fileContents) + } else { + replaceTemplates(value[key], fileContents) + } + } + return + } + } + + function expandString(str: string, fileContents: Map): string { + // Expand {env:VAR} templates + str = str.replace(/\{env:([^}]+)\}/g, (_, varName) => { + return process.env[varName] || "" + }) + + // Expand {file:path} templates + for (const [match, content] of fileContents) { + str = str.replaceAll(match, content) } return str From 290f0efe2e8030691247456985043b7ca61f2166 Mon Sep 17 00:00:00 2001 From: conradkoh Date: Fri, 19 Dec 2025 12:03:41 +0800 Subject: [PATCH 4/6] Update packages/opencode/src/config/config.ts Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- packages/opencode/src/config/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 01a7a596534..9335ad21ba6 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -850,7 +850,7 @@ export namespace Config { return load(text, filepath) } - async function expandTemplates(value: any, configDir: string, configFilepath: string): Promise { + async function expandTemplates(value: unknown, configDir: string, configFilepath: string): Promise { // First pass: collect all file references const fileRefs = new Map() // match -> resolvedPath collectFileReferences(value, configDir, fileRefs) From 31b58348510c236452ef15e3298aea85bbc73f80 Mon Sep 17 00:00:00 2001 From: conradkoh Date: Fri, 19 Dec 2025 12:05:11 +0800 Subject: [PATCH 5/6] style(config): replace any with unknown in template expansion --- packages/opencode/src/config/config.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 9335ad21ba6..020eb48aff3 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -882,7 +882,7 @@ export namespace Config { replaceTemplates(value, fileContents) } - function collectFileReferences(value: any, configDir: string, fileRefs: Map): void { + function collectFileReferences(value: unknown, configDir: string, fileRefs: Map): void { if (Array.isArray(value)) { for (const item of value) { if (typeof item === "string") { @@ -895,11 +895,13 @@ export namespace Config { } if (value && typeof value === "object") { - for (const key in value) { - if (typeof value[key] === "string") { - extractFileReferences(value[key], configDir, fileRefs) + const obj = value as Record + for (const key in obj) { + const val = obj[key] + if (typeof val === "string") { + extractFileReferences(val, configDir, fileRefs) } else { - collectFileReferences(value[key], configDir, fileRefs) + collectFileReferences(val, configDir, fileRefs) } } return @@ -922,7 +924,7 @@ export namespace Config { } } - function replaceTemplates(value: any, fileContents: Map): void { + function replaceTemplates(value: unknown, fileContents: Map): void { if (Array.isArray(value)) { for (let i = 0; i < value.length; i++) { if (typeof value[i] === "string") { @@ -935,11 +937,13 @@ export namespace Config { } if (value && typeof value === "object") { - for (const key in value) { - if (typeof value[key] === "string") { - value[key] = expandString(value[key], fileContents) + const obj = value as Record + for (const key in obj) { + const val = obj[key] + if (typeof val === "string") { + obj[key] = expandString(val, fileContents) } else { - replaceTemplates(value[key], fileContents) + replaceTemplates(val, fileContents) } } return From f98c60587ea6e4a78f4bd5ff5c125093a59754b0 Mon Sep 17 00:00:00 2001 From: conradkoh Date: Fri, 19 Dec 2025 14:06:09 +0800 Subject: [PATCH 6/6] chore: generate --- packages/plugin/package.json | 2 +- packages/sdk/js/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/plugin/package.json b/packages/plugin/package.json index 1f0132047a3..9342a3aa3ad 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -24,4 +24,4 @@ "typescript": "catalog:", "@typescript/native-preview": "catalog:" } -} \ No newline at end of file +} diff --git a/packages/sdk/js/package.json b/packages/sdk/js/package.json index 9d0a184fa07..33934417672 100644 --- a/packages/sdk/js/package.json +++ b/packages/sdk/js/package.json @@ -29,4 +29,4 @@ "publishConfig": { "directory": "dist" } -} \ No newline at end of file +}