diff --git a/make-pdf/src/browseClient.ts b/make-pdf/src/browseClient.ts index da25677e59..08213a0e51 100644 --- a/make-pdf/src/browseClient.ts +++ b/make-pdf/src/browseClient.ts @@ -158,6 +158,11 @@ export function resolveBrowseBin(env: NodeJS.ProcessEnv = process.env): string { function isExecutable(p: string): boolean { try { + // Must be a regular FILE. access(X_OK) alone is true for directories — they carry the + // execute/traverse bit on POSIX and pass the Windows check too — so discovery happily + // "found" ~/.claude/skills/browse, which is the skill's docs folder containing nothing + // but SKILL.md, and returned a directory as the browse binary. + if (!fs.statSync(p).isFile()) return false; fs.accessSync(p, fs.constants.X_OK); return true; } catch { diff --git a/make-pdf/test/browseClient.test.ts b/make-pdf/test/browseClient.test.ts index 072278e500..b59068e8ba 100644 --- a/make-pdf/test/browseClient.test.ts +++ b/make-pdf/test/browseClient.test.ts @@ -59,6 +59,41 @@ describe("findExecutable", () => { const found = findExecutable("/nonexistent/path/to/nothing"); expect(found).toBeNull(); }); + + // access(X_OK) is TRUE for directories — they carry the execute/traverse bit — so a + // bare X_OK test returned ~/.claude/skills/browse, the skill's docs folder, as "the + // browse binary". Every browse call then failed with an empty error, which surfaced + // as make-pdf reporting "Chromium failed to launch". + test("rejects a DIRECTORY even though it passes access(X_OK)", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "mkpdf-dir-")); + try { + // Prove the precondition: the directory really does pass the old test. + let passesXok = true; + try { + fs.accessSync(dir, fs.constants.X_OK); + } catch { + passesXok = false; + } + expect(passesXok).toBe(true); + + expect(findExecutable(dir)).toBeNull(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test("rejects a directory that shadows a real binary name", () => { + // The exact shape of the bug: a directory named like the thing being looked for. + const base = fs.mkdtempSync(path.join(os.tmpdir(), "mkpdf-shadow-")); + const shadow = path.join(base, "browse"); + fs.mkdirSync(shadow); + fs.writeFileSync(path.join(shadow, "SKILL.md"), "# not a binary\n"); + try { + expect(findExecutable(shadow)).toBeNull(); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); }); describe("resolveBrowseBin", () => {