fix(config): resolve {file:path} template shadowing bug

[conradkoh]

Summary

Fixes a bug where {file:path} template expansions fail when the same file path appears earlier in a commented line.

Closes #5731

Root Cause

The config loader performs string-based template expansion before JSON parsing. When multiple instances of the same {file:path} exist in the config file, findIndex always returns the index of the first occurrence. If the first occurrence is in a commented line, the loader skips all subsequent (valid) instances of that same file reference.

Example of the Bug

{
  "agent": {
    // "experimental": {
    //   "prompt": "{file:./prompts/precise.md}"  // ← findIndex finds this first
    // },
    "precise": {
      "prompt": "{file:./prompts/precise.md}"  // ← This never gets expanded
    }
  }
}

Location

packages/opencode/src/config/config.ts, the load function

Problematic Code

for (const match of fileMatches) {
  const lineIndex = lines.findIndex((line) => line.includes(match))  // BUG: Always finds FIRST occurrence
  if (lineIndex !== -1 && lines[lineIndex].trim().startsWith("//")) {
    continue // Skip if line is commented
  }
  // ... file reading and replacement ...
}

Solution

This PR includes two commits:

1. Minimal Fix (Commit 1)

• Track all match positions and line numbers upfront
• Process matches in reverse order to preserve string indices
• Check each specific occurrence's line for comments

This ensures that commented-out file references don't prevent valid references from being expanded.

2. Parse-First Refactor (Commit 2)

Refactors the architecture to be more robust and maintainable:

• Parse JSONC first, then expand templates in the object tree
• Recursive tree walker for template expansion
• Eliminates fragile comment detection and string injection risks

Benefits:

• JSONC parser handles comments correctly (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

Testing

• ✅ Added reproduction test case demonstrating the shadowing bug
• ✅ Added test for multiple agents with different file references
• ✅ All 21 config tests pass
• ✅ Full test suite: 305 pass (1 pre-existing failure unrelated to changes)
• ✅ TypeScript type checks pass

Changes

• packages/opencode/src/config/config.ts: Config loading implementation
• packages/opencode/test/config/config.test.ts: New test cases

Architectural Issues Resolved

The original approach had several problems:

1. Comment Detection is Fragile: Manual check using startsWith("//") doesn't understand JSON comment rules
2. Duplicate Path Handling: Can't distinguish between multiple instances of the same file path
3. Injection Vulnerability: File contents are injected into raw JSON string using JSON.stringify().slice(1, -1)
4. No Context Awareness: Template expansion happens without understanding the JSON structure

The new parse-first architecture resolves all of these issues.

Related

• Addresses issue reported in Agent file prompts not loading for some file names #5731 (comment)

[rekram1-node]

/review

[conradkoh]

There are 3 distinct commits - it might be a change bigger than wanted.

1. Just fix the issue
2. Rework replacement to do config parsing first then replacement (previous was replacement in config then parsing)
3. Performance optimizations for walking the json

The first commit should be a standalone fix and should be working as is. If the change is too big, can discard the other 2 commits.

[rekram1-node]

Did ur agent write that comment? Haha

[conradkoh]

haha nope it didn't! I wrote it! it's my first time contributing here -- I came across some articles talking about the conservative approach to contributions! wanted to give a head's up so that it's easier to review!

[conradkoh]

but the PR is description and code are 100% agent written 😬

[rekram1-node]

Haha sounds good

[conradkoh]

@rekram1-node updated the code according with the generate command ran.

[conradkoh]

hey @rekram1-node just wanted to check - is there anything else needed from me to get this merged? just making sure that it isn't getting stalled by something I should have done but didn't do.