From f374387c43969622efee9c9cbae4ddb22b7aa0dd Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 21 Aug 2026 18:16:58 -0400 Subject: [PATCH] fix: resolve agent-browser via module resolution, not a hardcoded nested path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flowspec looked for its agent-browser binary only at /node_modules/.bin/agent-browser — a layout that exists under nested installs (pnpm, flowspec's own repo) but not under hoisting package managers (bun, npm), where the binary is hoisted to the consumer root. The comment promised an npx fallback that was never implemented, so as an installed dependency every flow died at step 0 with 'Browser command failed'. getAgentBrowserPath now resolves agent-browser through createRequire from this module's URL — correct for nested, hoisted, and workspace layouts — preferring the .bin shim beside the resolved package, then the package's own bin script, then bare 'agent-browser' for PATH lookup. Fixes #15 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GtAjJabU9hUkrzoWF3Xtzm --- src/runner.ts | 56 +++++++++++----- test/agent-browser-resolution.test.ts | 94 +++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 test/agent-browser-resolution.test.ts diff --git a/src/runner.ts b/src/runner.ts index 51fd76c..c73f388 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -1,5 +1,6 @@ +import { existsSync } from "node:fs"; +import { createRequire } from "node:module"; import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; import type { FlowError, FlowResult, @@ -83,26 +84,45 @@ function generateSessionName(): string { } /** - * Get the path to the agent-browser binary + * Get the path to the agent-browser binary. + * + * Resolves through Node module resolution rather than a hardcoded + * `/node_modules/.bin` path, because that layout only exists + * under nested installs (pnpm, or flowspec's own repo). Hoisting package + * managers (bun, npm) place agent-browser at the consumer root, where the + * old path pointed at nothing and every flow died with "Browser command + * failed" (#15). + * + * Preference order: the `.bin` shim beside the resolved package (same + * spawn semantics as before), then the package's own bin script, then a + * bare `agent-browser` for PATH lookup. + * + * @param fromUrl module URL to resolve from — overridable for tests */ -function getAgentBrowserPath(): string { - // Try to find agent-browser in node_modules/.bin - // This works whether we're running from src/ or from dist/ - const currentFile = import.meta.url; - const currentDir = dirname(fileURLToPath(currentFile)); +export function getAgentBrowserPath(fromUrl: string = import.meta.url): string { + try { + const require = createRequire(fromUrl); + const pkgJsonPath = require.resolve("agent-browser/package.json"); + const pkgDir = dirname(pkgJsonPath); - // From src/ it's ../node_modules/.bin/agent-browser - // From dist/ it's ../node_modules/.bin/agent-browser - const binPath = join( - currentDir, - "..", - "node_modules", - ".bin", - "agent-browser", - ); + // The .bin shim lives in the node_modules that contains the package. + const shimPath = join(dirname(pkgDir), ".bin", "agent-browser"); + if (existsSync(shimPath)) return shimPath; + + const pkg = require(pkgJsonPath) as { + bin?: string | Record; + }; + const bin = + typeof pkg.bin === "string" ? pkg.bin : pkg.bin?.["agent-browser"]; + if (bin) { + const binPath = join(pkgDir, bin); + if (existsSync(binPath)) return binPath; + } + } catch { + // Not resolvable as a module — fall through to PATH lookup. + } - // If that doesn't work, fall back to npx - return binPath; + return "agent-browser"; } /** diff --git a/test/agent-browser-resolution.test.ts b/test/agent-browser-resolution.test.ts new file mode 100644 index 0000000..98f9f17 --- /dev/null +++ b/test/agent-browser-resolution.test.ts @@ -0,0 +1,94 @@ +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { afterEach, describe, expect, it } from "vitest"; +import { getAgentBrowserPath } from "../src/runner.js"; + +/** + * Regression tests for #15: flowspec hardcoded its own nested + * node_modules/.bin path, which doesn't exist under hoisting package + * managers (bun, npm) — so every flow failed at step 0 when flowspec was + * installed as a dependency. + */ +describe("getAgentBrowserPath", () => { + const tmpDirs: string[] = []; + + function makeTmpDir(): string { + // realpath so expectations match module resolution, which returns + // resolved paths (macOS tmpdir lives behind a /var → /private/var symlink) + const dir = realpathSync(mkdtempSync(join(tmpdir(), "flowspec-abr-"))); + tmpDirs.push(dir); + return dir; + } + + afterEach(() => { + for (const dir of tmpDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + /** Lay out /node_modules/agent-browser with a bin script. */ + function scaffoldPackage(root: string, { withShim }: { withShim: boolean }) { + const pkgDir = join(root, "node_modules", "agent-browser"); + mkdirSync(join(pkgDir, "bin"), { recursive: true }); + writeFileSync( + join(pkgDir, "package.json"), + JSON.stringify({ + name: "agent-browser", + version: "0.0.0-test", + bin: { "agent-browser": "./bin/agent-browser.js" }, + }), + ); + writeFileSync( + join(pkgDir, "bin", "agent-browser.js"), + "#!/usr/bin/env node\n", + ); + if (withShim) { + const binDir = join(root, "node_modules", ".bin"); + mkdirSync(binDir, { recursive: true }); + symlinkSync( + join("..", "agent-browser", "bin", "agent-browser.js"), + join(binDir, "agent-browser"), + ); + } + // The module URL resolution starts from — a file at the consumer root, + // as if flowspec code were running from /node_modules/flowspec/. + return pathToFileURL(join(root, "entry.js")).href; + } + + it("resolves via the real install in this repo", () => { + const result = getAgentBrowserPath(); + expect(result).not.toBe("agent-browser"); + expect(result).toContain("agent-browser"); + }); + + it("prefers the .bin shim beside the resolved package (hoisted layout)", () => { + const root = makeTmpDir(); + const fromUrl = scaffoldPackage(root, { withShim: true }); + expect(getAgentBrowserPath(fromUrl)).toBe( + join(root, "node_modules", ".bin", "agent-browser"), + ); + }); + + it("falls back to the package's own bin script when no shim exists", () => { + const root = makeTmpDir(); + const fromUrl = scaffoldPackage(root, { withShim: false }); + expect(getAgentBrowserPath(fromUrl)).toBe( + join(root, "node_modules", "agent-browser", "bin", "agent-browser.js"), + ); + }); + + it("falls back to PATH lookup when agent-browser is not resolvable", () => { + const root = makeTmpDir(); + const fromUrl = pathToFileURL(join(root, "entry.js")).href; + expect(getAgentBrowserPath(fromUrl)).toBe("agent-browser"); + }); +});