Skip to content

fix: Node bundle build regression from @ngrok/ngrok + README skill list cleanup (v0.16.2.1)#951

Closed
ignsm wants to merge 4 commits intogarrytan:mainfrom
ignsm:fix/ngrok-external-in-node-bundle
Closed

fix: Node bundle build regression from @ngrok/ngrok + README skill list cleanup (v0.16.2.1)#951
ignsm wants to merge 4 commits intogarrytan:mainfrom
ignsm:fix/ngrok-external-in-node-bundle

Conversation

@ignsm
Copy link
Copy Markdown

@ignsm ignsm commented Apr 10, 2026

Summary

Two small fixes landed in one narrow PR. Both surfaced today while upgrading a local install from 0.15.2.1 to 0.16.2.0.

Build fix (the loud one): ./setup was printing an ugly error: cannot write multiple output files without an output directory during the Windows Node.js fallback bundle step. The rest of setup still worked, but every install saw the error. Root cause: when /pair-agent landed, it added @ngrok/ngrok, which ships prebuilt native .node binaries (darwin-universal 19MB, darwin-arm64 9MB, win32-x64, etc.). browse/scripts/build-node-server.sh uses bun build --outfile to produce a single server-node.mjs. --outfile only supports one output file, but bun needs three (the JS bundle + two native .node files) because it tries to inline ngrok. Fix: add --external "@ngrok/ngrok" to the build command, same way we already externalize playwright, playwright-core, diff, and bun:sqlite. Windows Node users who want pair-agent tunneling install @ngrok/ngrok separately, same model as playwright.

Verified by running bash browse/scripts/build-node-server.sh standalone — now produces a clean single server-node.mjs (0.31 MB, 23 modules, 113ms) with no errors. Both runtime call sites in browse/src/server.ts already wrap await import('@ngrok/ngrok') in try/catch, so if a Windows Node user runs without ngrok installed they get a clear 500 {"error": "Failed to start tunnel: Cannot find module..."} — not a silent failure.

README cleanup (the quiet one): The README had two separate skill-list blocks — one in the install command (line 49), one in the CLAUDE.md example (line 396). They drifted out of sync. Install had /connect-chrome (legacy name), /plan-devex-review, /devex-review but was missing /pair-agent. CLAUDE.md example had /open-gstack-browser (canonical) and /pair-agent but was missing /plan-devex-review and /devex-review. Now both blocks list the same 34 skills in the same order, using /open-gstack-browser as the canonical name. If you paste either block into a new project's CLAUDE.md, you get the full set of skills ./setup actually installs.

Follow-up captured: /checkpoint and /health are real user-facing skills with good frontmatter descriptions, live on disk at checkpoint/SKILL.md and health/SKILL.md, and get linked by ./setup on every install. But they are still missing from the README everywhere — not in either list block, not in the big skill table. Kept out of this PR's scope because a proper fix needs two new table rows with matching voice, not just list edits. Added to TODOS.md under a new ## Documentation section as a P2 follow-up.

Bisection

Four clean commits, each independently revertable:

  1. 714eb5fafix: externalize @ngrok/ngrok in Node server bundle build (build-node-server.sh only, +9/-0)
  2. a93d913cdocs: align README skill lists — drop /connect-chrome, add /pair-agent /plan-devex-review /devex-review (README.md only, +3/-2)
  3. 16800796docs: capture README gap for /checkpoint and /health as follow-up TODO (TODOS.md only, +14/-0)
  4. 298e6250chore: bump version and changelog (v0.16.2.1) (VERSION + CHANGELOG.md)

Test Coverage

No new code paths — this is a build-script flag, two string edits in a markdown file, and a TODOS.md entry. The build-node-server.sh change was manually re-verified end-to-end by running ./setup --no-prefix on the full gstack checkout. Setup completes cleanly, all 34 skills (including the three new ones from the 0.16.2.0 bump: /devex-review, /plan-devex-review, /pair-agent) get linked into ~/.claude/skills/, and the Node bundle step reports Node server bundle ready: browse/dist/server-node.mjs with no errors.

Tests: no new tests added. No application logic changed.

Pre-Landing Review

No issues found. 26-line diff across 3 substantive files plus the VERSION/CHANGELOG bookkeeping commit. Both dynamic import('@ngrok/ngrok') call sites in browse/src/server.ts (lines 1568 and 2347) are wrapped in try/catch, so marking the module external does not regress error handling. No new attack surface: --external is a compile-time bun directive with a hardcoded module name, not user input.

Test Suite Status

bun test on this branch shows 7 pre-existing failures, all confirmed unrelated to this PR's diff:

  1. host-config-export > detect finds claude — environmental, fails in sandboxed shells where claude binary is not on PATH. Not touched by this branch.
  2. golden-file regression > Claude ship skill matches golden baselineverified pre-existing on origin/main: diff $(git show origin/main:test/fixtures/golden/claude-ship-SKILL.md) $(git show origin/main:ship/SKILL.md) shows the golden fixture has not been regenerated after recent ship/SKILL.md.tmpl changes (vendoring deprecation section added but golden not updated).
  3. golden-file regression > Codex ship skill matches golden baseline — same root cause as refactor: reorganize codebase into modular structure #2.
  4. golden-file regression > Factory ship skill matches golden baseline — same root cause as refactor: reorganize codebase into modular structure #2.
  5. gstack-team-init > removes vendored copy when present — fails with error: Command failed: git add .claude/skills/gstack/ because the test fixture path is ignored by .gitignore. Not touched by this branch.
  6. gstack-team-init > does not duplicate .gitignore entry on re-run — same root cause as fix: harden /browse lifecycle and ref safety #5.
  7. gstack-uninstall > integration tests > clean system outputs nothing to remove — environmental, fails in clean test fixture setup. Not touched by this branch.

This PR's diff touches only browse/scripts/build-node-server.sh, README.md, TODOS.md, VERSION, and CHANGELOG.md. None of those files relate to the failing tests' subjects (host-config-export.ts, ship/SKILL.md.tmpl, golden fixtures, gstack-team-init, gstack-uninstall). grep -l across the branch diff confirms no overlap.

Fixing these 7 tests requires regenerating golden fixtures and debugging three unrelated test harnesses — a separate, scoped effort that doesn't belong in this narrow fix. Recommend fixing in a follow-up PR.

Plan Completion

Not applicable — this PR is a follow-up to a live upgrade session, not to a pre-written plan file. Work was scoped on the fly after ./setup surfaced the build error.

TODOS

Added one new P2 entry under a new ## Documentation section: "Add /checkpoint and /health to README" (install block + CLAUDE.md example + big skill table). No existing TODOs closed by this PR.

Test plan

  • bash browse/scripts/build-node-server.sh exits 0 and produces a single server-node.mjs in browse/dist/ (verified — 23 modules, 0.31 MB)
  • ./setup --no-prefix completes without errors and registers all 34 skills (verified end-to-end on the upgrade session that surfaced this)
  • grep "^## \[" CHANGELOG.md shows a contiguous version sequence (verified — 0.16.2.1, 0.16.2.0, 0.16.1.0, 0.16.0.0, 0.15.16.0, 0.15.15.1, ...)
  • Both README skill-list blocks contain the same 34 skills (verified via diff of extracted lists)
  • Windows Node fallback runtime path: run server-node.mjs on a Windows machine without @ngrok/ngrok installed and confirm the /tunnel/start endpoint returns 500 {"error": "Failed to start tunnel: Cannot find module..."} (not regression-tested in this PR because no Windows CI exists)

🤖 Generated with Claude Code

ignsm and others added 4 commits April 10, 2026 16:47
./setup was emitting a non-fatal but ugly error during the Windows
Node.js fallback bundle step:

  Building Node-compatible server bundle...
  error: cannot write multiple output files without an output directory

Root cause: browse/src/server.ts dynamically imports @ngrok/ngrok for
/pair-agent tunneling. Bun's bundler statically sees the import and
tries to inline ngrok's prebuilt native .node binaries (darwin-universal
19MB + darwin-arm64 9MB + win32-x64 etc.) as separate asset files. But
build-node-server.sh uses --outfile, which only supports a single output
file. Bun fails with "cannot write multiple output files" before writing
anything.

Fix: mark @ngrok/ngrok as --external in the Node bundle build, matching
how we already handle playwright and playwright-core. Windows Node
users who want pair-agent tunneling install @ngrok/ngrok separately —
same model as playwright.

Verified: bash browse/scripts/build-node-server.sh now produces a
clean single server-node.mjs (0.31 MB, 23 modules) with no errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…t /plan-devex-review /devex-review

The README had two separate skill lists that drifted out of sync:

1. Install command block (line 49) listed /connect-chrome (legacy name),
   /plan-devex-review, /devex-review but was missing /pair-agent.
2. CLAUDE.md example block (line 396) listed /open-gstack-browser,
   /pair-agent but was missing /plan-devex-review and /devex-review.

Fix: both lists now contain the same 34 skills in the same order:
- Replace legacy /connect-chrome with canonical /open-gstack-browser
  (the skill was renamed; /connect-chrome is still a symlink alias but
  shouldn't be the advertised name).
- Add /pair-agent to the install block.
- Add /plan-devex-review and /devex-review to the CLAUDE.md example.

Users pasting either block into a new project's CLAUDE.md now get the
full set of available skills without gaps.

Note: /checkpoint and /health are real user-facing skills that exist on
disk and get linked by ./setup but are still not mentioned anywhere in
the README (not in either skill list block, not in the skill table).
Left for a follow-up PR — this one stays narrowly scoped to aligning the
two existing list strings.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Both skills exist on disk, get linked by ./setup, and show up in
~/.claude/skills/ — but they're not mentioned anywhere in the README
(no install-block entry, no CLAUDE.md example entry, no skill table row).

This TODO captures the follow-up PR that would add them in all three
places. Kept out of this branch to preserve narrow scope: the two
skill-list strings are now aligned, but a proper fix for /checkpoint
and /health needs two new rows in the big skill table too, with
descriptions matching the existing table voice.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@ignsm
Copy link
Copy Markdown
Author

ignsm commented Apr 10, 2026

Closing this in favor of a cleaner split.

I opened this without searching the PR queue first (classic Search Before Building violation, sorry) and the build-fix portion duplicates two already-open PRs:

Either one fixes the ./setup build error. No need for a third PR doing the same thing.

The README cleanup portion of this PR (aligning the two skill-list blocks, dropping /connect-chrome, adding /pair-agent, /plan-devex-review, /devex-review) is genuinely unique and still worth landing. I've moved it into #953 along with the follow-up that was stacked in #952, so it's now a single clean README-only PR with no overlap against any open build-fix PR.

Recommendation for the build fix: probably #924, since keeping ngrok bundled on Windows is better UX than making users manually npm install @ngrok/ngrok every time they update gstack. But that's your call.

Apologies for the noise. Won't happen again — I'll gh pr list --search "<keywords>" before writing a fix next time.

@ignsm ignsm closed this Apr 10, 2026
@ignsm ignsm deleted the fix/ngrok-external-in-node-bundle branch April 10, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant