fix(agent-prompt-resolver): handle file:// URIs cross-platform

[andrejtonev]

No description provided.

[qodo-code-review]

PR Summary by Qodo

Fix plugin override file:// URI resolution across platforms
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Description

• Parse plugin override file:// URIs via WHATWG URL + fileURLToPath for Windows correctness.
• Support file://./, file://../, and file://~/ forms; degrade malformed URIs to config.
• Update unit/e2e tests to generate canonical file URIs and use portable path assertions.

Diagram

graph TD
  A["Plugin override value"] --> B["buildPluginOverride()"] --> C{"Starts with file://?"}
  C -->|"file://./ or file://../"| D["Resolve relative to config dir"] --> G["Source: target=file_uri"]
  C -->|"file://~/"| E["Expand via homedir()"] --> G
  C -->|"file:///..."| F["new URL() + fileURLToPath()"] --> G
  C -->|"Malformed/unsupported"| H["Source: target=config (verbatim)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Auto-correct malformed Windows-style file:// URIs

• ➕ More forgiving for users who paste Windows paths like file://C:...
• ➕ Could reduce support burden from misformatted inputs
• ➖ Silently accepts invalid URIs and may mask configuration mistakes
• ➖ Ambiguous behavior (host vs path) and potential security footguns from over-normalization

2. Support host-based file URIs (UNC/SMB) for file://server/share/path

• ➕ Enables network-path prompts in enterprise/Windows environments
• ➕ Aligns with broader file URI semantics where hosts can be meaningful
• ➖ Requires additional platform-specific mapping rules and test coverage
• ➖ Out of scope for current behavior, which intends local-disk targeting only

Recommendation: The PR’s approach (explicitly handling ./, ../, and ~ conventions, then using WHATWG URL + fileURLToPath for canonical file:/// URIs, with a conservative fallback to target=config on malformed inputs) is the safest cross-platform strategy. Auto-correction would increase ambiguity, and UNC/host-based support is a separate feature that should be introduced intentionally with clear semantics.

Files changed (4) +120 / -25

Bug fix (1) +65 / -10

agent-prompt-resolver.tsParse file:// plugin override URIs via URL/fileURLToPath with safe fallbacks +65/-10

Parse file:// plugin override URIs via URL/fileURLToPath with safe fallbacks

• Replaces string/startsWith-based file:// parsing with a cross-platform classifier: explicit handling for file://./ and file://../ relative to the config directory, file://~/ expansion via homedir(), and canonical file:///... parsing via new URL() + fileURLToPath. Malformed or unsupported URIs now degrade to target=config to avoid mis-routing to an unintended path.

src/agent-prompt-resolver.ts

Tests (3) +55 / -15

agent-prompt-resolver.test.tsUse pathToFileURL in file URI tests and add malformed-URI regression coverage +31/-1

Use pathToFileURL in file URI tests and add malformed-URI regression coverage

• Updates the absolute file URI test to generate a canonical file:///... URL using pathToFileURL (portable across Windows/POSIX). Adds a regression test asserting that malformed file:// URIs are treated as target=config and preserved verbatim.

tests/agent-prompt-resolver.test.ts

prompt-shapes.test.tsMake e2e file:// URI fixtures canonical and home-path assertions portable +14/-11

Make e2e file:// URI fixtures canonical and home-path assertions portable

• Switches e2e plugin override fixtures from template-string file:// URIs to pathToFileURL().href to avoid Windows-malformed URIs. Uses homedir() for ~ expansion expectations and ensures the plugin config value remains unchanged after writes.

tests/e2e/prompt-shapes.test.ts

path-utils.test.tsUse posix.join in assertions to keep path-utils tests platform-independent +10/-3

Use posix.join in assertions to keep path-utils tests platform-independent

• Adjusts tests to assert POSIX-style joined paths (posix.join) where fixtures use forward slashes, while leaving the implementation free to use platform-native separators. This prevents Windows-only failures due to backslash normalization differences.

tests/path-utils.test.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. POSIX join breaks tilde test 🐞 Bug ☼ Reliability

Description

tests/path-utils.test.ts asserts POSIX-separated results using posix.join(), but both
expandTilde() and candidateGlobalOpencodeDirs() build paths with platform-native
node:path.join(), which yields backslashes on Windows. As a result, the tests can fail on Windows
CI even though the implementation is correct for that OS.

Code

tests/path-utils.test.ts[R17-22]

+    // Use posix.join to assert the cross-platform contract: the result
+    // is `home + "/" + suffix`. On Windows, plain `join` would produce
+    // a backslash separator that doesn't match the test fixture.
    expect(expandTilde("~/work/team.md", "/home/x")).toBe(
-      "/home/x/work/team.md",
+      posix.join("/home/x", "work/team.md"),
    )

Evidence

The cited implementation details show that expandTilde() constructs its return value with
join(home, ...), which follows host OS path separator rules, while candidateGlobalOpencodeDirs()
similarly uses join(process.env.XDG_CONFIG_HOME, "opencode") when building candidate directories;
on Windows these join() calls produce backslash-separated strings, so assertions that compute
expected values via posix.join() (and therefore hard-code forward slashes) won’t match,
demonstrating why the tests are non-portable.

src/path-utils.ts[27-30]
src/path-utils.ts[68-74]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/path-utils.test.ts` currently computes expected path strings using `posix.join()`, but the code under test (`expandTilde()` and `candidateGlobalOpencodeDirs()`) uses platform-native `node:path.join()`. This mismatch causes Windows to produce backslash-separated paths that fail POSIX-only assertions.

## Issue Context
The implementations appear to be correctly returning platform-native filesystem paths (including Windows-style separators on win32). Update the tests to be portable by either computing expected values with `join()` (or otherwise matching the implementation’s platform semantics) or normalizing both actual and expected values before asserting; only change the implementation to POSIX-normalize if the intended product contract is explicitly “POSIX paths,” which would be atypical for Windows filesystem paths.

## Fix Focus Areas
- tests/path-utils.test.ts[17-22]
- tests/path-utils.test.ts[105-110]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---