fix: resolve agent-browser via module resolution (fixes #15) - #19
Conversation
…ted path flowspec looked for its agent-browser binary only at <flowspec>/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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GtAjJabU9hUkrzoWF3Xtzm
There was a problem hiding this comment.
Nitpick review — approve
This change updates the core execution logic in src/runner.ts and introduces a new test suite to cover the modifications. The additions provide direct test coverage for the new behavior without introducing any apparent risks. The code is clean and ready to merge.
0 inline comment(s).
Verdict was
approve, posted as a comment — this repo does not allow GitHub Actions to approve PRs.
|
Warning Review limit reached
Next review available in: 16 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🎉 This PR is included in version 0.2.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Fixes #15 — flowspec hardcoded
<flowspec>/node_modules/.bin/agent-browser, which only exists under nested installs (pnpm, or flowspec's own repo). Under bun/npm hoisting the binary lives at the consumer root, the hardcoded path points at nothing, and every flow fails at step 0 with "Browser command failed." The old comment promised an npx fallback that was never implemented.getAgentBrowserPathnow resolves throughcreateRequire(import.meta.url):.binshim beside the resolved package (identical spawn semantics to before),binscript,agent-browserfor PATH lookup (the fallback the old comment promised).Works for nested, hoisted, and workspace layouts. Regression tests cover all three paths plus the real install in this repo (4 new tests; full suite 475 pass / 0 fail).
Note for release flow: once #18's semantic-release lands, merging this
fix:commit auto-cuts v0.2.0 carrying the August headers/setup/CLI-adapter features to npm — unblocking bun-installed consumers (TheAITeam/test-harness) and the arcanelayer-store monitoring lanes (queso/arcanelayer-store#35).🤖 Generated with Claude Code
https://claude.ai/code/session_01GtAjJabU9hUkrzoWF3Xtzm