diff --git a/src/agent-prompt-resolver.ts b/src/agent-prompt-resolver.ts index 4c587d5..e5f3cf9 100644 --- a/src/agent-prompt-resolver.ts +++ b/src/agent-prompt-resolver.ts @@ -1,6 +1,7 @@ import { readdir, readFile } from "node:fs/promises" import { homedir } from "node:os" import { basename, dirname, isAbsolute, join, relative } from "node:path" +import { fileURLToPath } from "node:url" import { applyEdits, type ModificationOptions, @@ -385,43 +386,97 @@ function buildPluginOverride( // `file://...` URI (used by oh-my-opencode and others) → target a real // file on disk. We resolve `./` and `~/` forms against the config's // directory and `homedir()` respectively; absolute URIs are kept verbatim. - const fileUri = value.match(/^file:\/\/(.+)$/) - if (fileUri) { - const raw = fileUri[1] - if (raw.startsWith("/")) { + // + // Cross-platform: a `file:` URI is properly parsed via WHATWG URL. On + // Linux/macOS, `file:///abs/path` has three slashes (host="") and the + // pathname is the absolute path. On Windows, `file:///C:/path` has the + // drive letter as the first path segment; `fileURLToPath` handles the + // drive-letter round-trip correctly. We deliberately do NOT do + // `value.startsWith("file://")` + `match(/^file:\/\/(.+)$/)` because + // that breaks on Windows paths like `file://C:\Users\foo\x.md` + // (only two slashes, backslashes — a malformed URI that + // `new URL()` rejects, and that the manual `startsWith("/")` / + // `startsWith("~/")` / `startsWith("./")` ladder misclassifies). + if (value.startsWith("file://") || value.startsWith("file:///")) { + // Form A: `file://./...` and `file://../...` — relative to the + // config file's directory. The host is empty; the pathname is the + // raw `./...` or `../...` string. Detect this case BEFORE trying + // `new URL()`, which would otherwise resolve `./` against the + // current working directory on some platforms. + const relMatch = value.match(/^file:\/\/(\.\.?\/.*)$/) + if (relMatch) { return { kind: "plugin_override", agentName, target: "file_uri", - path: raw, + path: join(dirname(configPath), relMatch[1]), configPath, promptField, isAppend, } } - if (raw.startsWith("~/")) { + + // Form B: `file://~/...` — tilde is a convention, not a URL standard. + // Handle it explicitly so the resolver doesn't reject it on platforms + // where `~` would not round-trip through `fileURLToPath`. + if (value.startsWith("file://~/")) { return { kind: "plugin_override", agentName, target: "file_uri", - path: join(homedir(), raw.slice(2)), + path: join(homedir(), value.slice("file://~/".length)), configPath, promptField, isAppend, } } - if (raw.startsWith("./") || raw.startsWith("../")) { + + // Form C: a real file URI. `new URL()` accepts both `file:///abs/path` + // (three slashes, host="") on POSIX and `file:///C:/path` on Windows. + // We require three slashes so we don't accidentally classify a + // user-supplied value like `file://example.com/foo` (a host-based + // URI with no path) as a local file reference. + let parsedUrl: URL + try { + parsedUrl = new URL(value) + } catch { + // Malformed URI — degrade to config-raw to avoid a bad path. + return { + kind: "plugin_override", + agentName, + target: "config", + value, + configPath, + promptField, + isAppend, + } + } + if (parsedUrl.protocol === "file:") { + let absPath: string + try { + absPath = fileURLToPath(parsedUrl) + } catch { + return { + kind: "plugin_override", + agentName, + target: "config", + value, + configPath, + promptField, + isAppend, + } + } return { kind: "plugin_override", agentName, target: "file_uri", - path: join(dirname(configPath), raw), + path: absPath, configPath, promptField, isAppend, } } - // Unknown file:// form — degrade to config-raw to avoid a bad path. + // Some other scheme we don't recognise — degrade to config-raw. return { kind: "plugin_override", agentName, diff --git a/tests/agent-prompt-resolver.test.ts b/tests/agent-prompt-resolver.test.ts index 74ae2b2..a0a261c 100644 --- a/tests/agent-prompt-resolver.test.ts +++ b/tests/agent-prompt-resolver.test.ts @@ -3,6 +3,7 @@ import { randomBytes } from "node:crypto" import { mkdir, readFile, rm, writeFile } from "node:fs/promises" import { homedir, tmpdir } from "node:os" import { join, relative } from "node:path" +import { pathToFileURL } from "node:url" import { defaultAgentFilePath, materializeInlinePrompt, @@ -366,7 +367,10 @@ describe("resolveAgentPromptSource — plugin_override", () => { await writeJsonc( join(projectRoot, ".opencode", "oh-my-opencode.json"), JSON.stringify({ - agent: { foo: { prompt: `file://${target}` } }, + // pathToFileURL produces `file:///abs/path` on POSIX and + // `file:///C:/path` on Windows. Plain `file://${target}` is a + // malformed URI on Windows (only 2 slashes, backslashes). + agent: { foo: { prompt: pathToFileURL(target).href } }, }), ) const source = await resolveAgentPromptSource("foo", projectRoot, globalDir) @@ -377,6 +381,32 @@ describe("resolveAgentPromptSource — plugin_override", () => { expect(source.path).toBe(target) }) + test("malformed `file://` URI degrades to `config` target", async () => { + // Regression for the Windows CI failure: a `file://` URI that + // isn't a well-formed URL used to be mis-routed by the manual + // `match` + `startsWith("/")` ladder. The URL-based parser now + // catches malformed input and degrades to the `config` target — + // the value is left verbatim in `source.value` and no + // `file_uri` claim is made. + // (`file://[invalid` is a malformed URL on every platform because + // `[` is a reserved host-delimiter character. `file://C:\\...` + // also fails on Windows but is leniently accepted on POSIX where + // Linux's URL parser treats `C` as a hostname; the `[invalid` + // form is portable.) + await writeJsonc( + join(projectRoot, ".opencode", "oh-my-opencode.json"), + JSON.stringify({ + agent: { foo: { prompt: "file://[invalid" } }, + }), + ) + const source = await resolveAgentPromptSource("foo", projectRoot, globalDir) + if (source.kind !== "plugin_override") { + throw new Error(`expected plugin_override, got ${source.kind}`) + } + expect(source.target).toBe("config") + expect(source.value).toBe("file://[invalid") + }) + test("`file://~/...` URI expands tilde to the home directory", async () => { await writeJsonc( join(projectRoot, ".opencode", "oh-my-opencode.json"), diff --git a/tests/e2e/prompt-shapes.test.ts b/tests/e2e/prompt-shapes.test.ts index af06167..9b4dd99 100644 --- a/tests/e2e/prompt-shapes.test.ts +++ b/tests/e2e/prompt-shapes.test.ts @@ -37,8 +37,9 @@ import { rmSync, writeFileSync, } from "node:fs" -import { tmpdir } from "node:os" +import { homedir, tmpdir } from "node:os" import { join } from "node:path" +import { pathToFileURL } from "node:url" import { materializeInlinePrompt, resolveAgentPromptSource, @@ -242,6 +243,12 @@ describe("prompt source shape: file:// URI in a plugin override file", () => { let kasperStateDir: string let pluginConfigPath: string let promptFilePath: string + // Cross-platform `file://` URI for `promptFilePath`. Plain + // `file://${path}` template strings break on Windows because the + // result has only 2 slashes and backslashes (`file://C:\...`) — + // a malformed URI. `pathToFileURL` produces the canonical form + // (`file:///C:/...` on Windows, `file:///tmp/...` on POSIX). + let promptFileUri: string beforeEach(() => { const p = setupTmpProject("file-uri") @@ -252,6 +259,7 @@ describe("prompt source shape: file:// URI in a plugin override file", () => { pluginConfigPath = join(projectDir, ".opencode", "oh-my-openagent.json") // The referenced prompt file lives somewhere on disk. promptFilePath = join(projectDir, "external-prompts", "uri-agent.md") + promptFileUri = pathToFileURL(promptFilePath).href mkdirSync(join(projectDir, "external-prompts"), { recursive: true }) writeFileSync( promptFilePath, @@ -268,7 +276,7 @@ describe("prompt source shape: file:// URI in a plugin override file", () => { writeFileSync( pluginConfigPath, JSON.stringify({ - agent: { "uri-agent": { prompt_append: `file://${promptFilePath}` } }, + agent: { "uri-agent": { prompt_append: promptFileUri } }, }), "utf-8", ) @@ -300,7 +308,7 @@ describe("prompt source shape: file:// URI in a plugin override file", () => { writeFileSync( pluginConfigPath, JSON.stringify({ - agent: { "uri-agent": { prompt_append: `file://${promptFilePath}` } }, + agent: { "uri-agent": { prompt_append: promptFileUri } }, }), "utf-8", ) @@ -322,7 +330,7 @@ describe("prompt source shape: file:// URI in a plugin override file", () => { writeFileSync( pluginConfigPath, JSON.stringify({ - agent: { "uri-agent": { prompt_append: `file://${promptFilePath}` } }, + agent: { "uri-agent": { prompt_append: promptFileUri } }, }), "utf-8", ) @@ -347,18 +355,13 @@ describe("prompt source shape: file:// URI in a plugin override file", () => { // The plugin config is untouched — the URI is still the same. const configAfter = JSON.parse(readFileSync(pluginConfigPath, "utf-8")) - expect(configAfter.agent["uri-agent"].prompt_append).toBe( - `file://${promptFilePath}`, - ) + expect(configAfter.agent["uri-agent"].prompt_append).toBe(promptFileUri) }, ) test("file:// URI with a ~/... path resolves to $HOME", async () => { const homeRel = `file://~/kasper-e2e-uri-home-test-${Date.now()}.md` - const expandedPath = join( - process.env.HOME ?? "/home/user", - homeRel.replace(/^file:\/\/~\//, ""), - ) + const expandedPath = join(homedir(), homeRel.replace(/^file:\/\/~\//, "")) writeFileSync( expandedPath, "# Home URI Agent\n\nFrom the home directory.\n", diff --git a/tests/path-utils.test.ts b/tests/path-utils.test.ts index dfa5e0c..e835485 100644 --- a/tests/path-utils.test.ts +++ b/tests/path-utils.test.ts @@ -14,19 +14,26 @@ describe("expandTilde", () => { }) test("expands '~/...' against the supplied home", () => { - expect(expandTilde("~/work/team.md", "/home/x")).toBe( - "/home/x/work/team.md", + // The implementation uses `path.join`, so the result uses the + // platform's native separator. Use `homedir()` for the home + // argument so the assertion is portable: both sides of the + // comparison are produced by the same `path.join` call. (Earlier + // revisions used `posix.join` here, but that only matches on POSIX + // — on Windows `path.join("/home/x", "...")` returns + // `\home\x\...`, not the forward-slash form.) + expect(expandTilde("~/work/team.md", homedir())).toBe( + join(homedir(), "work/team.md"), ) }) test("returns absolute paths unchanged", () => { - expect(expandTilde("/etc/opencode/AGENTS.md", "/home/x")).toBe( + expect(expandTilde("/etc/opencode/AGENTS.md", homedir())).toBe( "/etc/opencode/AGENTS.md", ) }) test("returns relative paths unchanged", () => { - expect(expandTilde("./prompts", "/home/x")).toBe("./prompts") + expect(expandTilde("./prompts", homedir())).toBe("./prompts") }) test("defaults to os.homedir() when no home is supplied", () => { @@ -99,7 +106,12 @@ describe("candidateGlobalOpencodeDirs", () => { process.env.XDG_CONFIG_HOME = "/custom/xdg" try { const dirs = candidateGlobalOpencodeDirs() - expect(dirs[0]).toBe("/custom/xdg/opencode") + // The implementation does `join(process.env.XDG_CONFIG_HOME, + // "opencode")`. Assert with the same `path.join` call so the + // comparison uses the platform-native separator on Windows + // (where `join("/custom/xdg", "opencode")` returns + // `\custom\xdg\opencode`). + expect(dirs[0]).toBe(join("/custom/xdg", "opencode")) // Always ends with ~/.opencode as the fallback. expect(dirs[dirs.length - 1]).toBe(join(homedir(), ".opencode")) } finally {