diff --git a/make-pdf/src/browseClient.ts b/make-pdf/src/browseClient.ts index 099760c8c..9ab8d6e9c 100644 --- a/make-pdf/src/browseClient.ts +++ b/make-pdf/src/browseClient.ts @@ -10,7 +10,11 @@ * Binary resolution order (Codex round 2 #4, v1.24-aligned): * 1. $GSTACK_BROWSE_BIN env override (preferred, matches v1.24 GSTACK_*_BIN pattern) * 2. $BROWSE_BIN env override (back-compat alias) - * 3. sibling dir: dirname(argv[0])/../browse/dist/browse[.exe] + * 3. sibling dir: dirname(execPath)/../browse/dist/browse[.exe] + * (execPath, NOT argv[0]: in a bun-compiled binary argv[0] is the raw + * invocation string — often relative, so dirname() yields "." and the + * sibling candidates resolve against the CWD instead of the install + * dir; #2156. execPath is always the absolute binary path.) * 4. ~/.claude/skills/gstack/browse/dist/browse[.exe] * 5. PATH lookup via Bun.which('browse') — handles Windows PATHEXT natively * 6. error with setup hint @@ -101,14 +105,21 @@ export function findExecutable(base: string): string | null { * Locate the browse binary. Throws a BrowseClientError with a * canonical setup message if not found. See header for resolution order. */ -export function resolveBrowseBin(env: NodeJS.ProcessEnv = process.env): string { +export function resolveBrowseBin( + env: NodeJS.ProcessEnv = process.env, + // Injectable for tests: under `bun test` the process path is the bun + // runtime, so the compiled-binary shapes are unreachable without a seam. + selfPath: string = process.execPath || process.argv[0], +): string { // 1 + 2: env overrides (GSTACK_BROWSE_BIN preferred, BROWSE_BIN back-compat). const overrideRaw = env.GSTACK_BROWSE_BIN ?? env.BROWSE_BIN; const override = resolveOverride(overrideRaw, env); if (override) return override; - // 3: sibling — make-pdf and browse co-located in dist/. - const selfDir = path.dirname(process.argv[0]); + // 3: sibling — make-pdf and browse co-located in dist/. execPath, not + // argv[0] (#2156): see the header — argv[0] in a compiled binary is the + // invocation string, and a relative one resolved candidates against CWD. + const selfDir = path.dirname(selfPath); const siblingCandidates = [ path.resolve(selfDir, "../browse/dist/browse"), path.resolve(selfDir, "../../browse/dist/browse"), diff --git a/make-pdf/test/browseClient.test.ts b/make-pdf/test/browseClient.test.ts index b59068e8b..4bf6f1687 100644 --- a/make-pdf/test/browseClient.test.ts +++ b/make-pdf/test/browseClient.test.ts @@ -168,3 +168,51 @@ describe("BrowseClientError", () => { expect(err.name).toBe("BrowseClientError"); }); }); + +describe("resolveBrowseBin — sibling resolution from execPath (#2156)", () => { + // In a bun-compiled binary argv[0] is the raw invocation string (often + // relative), so the old dirname(argv[0]) built sibling candidates against + // the CWD. Under `bun test` the process path is the bun runtime, so these + // shapes are only reachable through the selfPath seam. + + test("sibling browse next to the install dir is found via selfPath", () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), "mkpdf-sib-")); + try { + const distDir = path.join(base, "browse", "dist"); + fs.mkdirSync(distDir, { recursive: true }); + const sibling = path.join(distDir, "browse"); + fs.writeFileSync(sibling, "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + const selfPath = path.join(base, "make-pdf", "dist", "pdf"); + // Receipts: pre-fix code ignores the selfPath seam entirely, so it can + // never produce this sibling — it either finds a global install (wrong + // value) or throws (PATH is empty). Red on v1.68.3.0 either way. + const resolved = resolveBrowseBin({ PATH: "" }, selfPath); + expect(resolved).toBe(path.resolve(path.join(base, "make-pdf"), "../browse/dist/browse")); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + test("a decoy browse DIRECTORY near selfPath never shadows a real PATH binary", () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), "mkpdf-decoy-")); + try { + // The ~/.claude/skills/browse alias-directory shape from #2156: a + // directory named exactly like the third sibling candidate. + fs.mkdirSync(path.join(base, "browse"), { recursive: true }); + const pathDir = path.join(base, "pathbin"); + fs.mkdirSync(pathDir, { recursive: true }); + const onPath = path.join(pathDir, "browse"); + fs.writeFileSync(onPath, "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + const selfPath = path.join(base, "tools", "pdf"); + // os.homedir() ignores a $HOME override under bun, so the global-install + // probe may legitimately win on boxes with a real ~/.claude install. The + // invariant under test is narrower: the decoy DIRECTORY never wins, and + // whatever wins is a regular file. + const resolved = resolveBrowseBin({ PATH: pathDir }, selfPath); + expect(resolved).not.toBe(path.join(base, "browse")); + expect(fs.statSync(resolved).isFile()).toBe(true); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); +});