From 6a785c57293e507e8f94cb881031c0ccf5a7d013 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 16 Apr 2026 13:49:04 -0700 Subject: [PATCH] fix: ngrok Windows build + close CI error-swallowing gap (v0.18.0.1) (#1024) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(browse): externalize @ngrok/ngrok so Node server bundle builds on Windows @ngrok/ngrok has a native .node addon that causes `bun build --outfile` to fail with "cannot write multiple output files without an output directory". Externalize it alongside the existing runtime deps (playwright, diff, bun:sqlite), matching the exact pattern used for every other dynamic import in server.ts. Adds a policy comment explaining when to extend the externals list so the next native dep doesn't repeat this failure. Two community contributors independently converged on this fix: - @tomasmontbrun-hash (#1019) - @scarson (#1013) Also fixes issues #1010 and #960. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(package.json): subshell cleanup so || true stops masking build/test failures Shell operator precedence trap in both the build and test scripts: cmd1 && cmd2 && ... && rm -f .*.bun-build || true bun test ... && bun run slop:diff 2>/dev/null || true The trailing `|| true` was intended to suppress cleanup errors, but it applies to the entire `&&` chain — so ANY failure (including the build-node-server.sh failure that broke Windows installs since v0.15.12) silently exits 0. CI ran the build, the build failed, and CI reported green. Wrap the cleanup/slop-diff commands in subshells so `|| true` only scopes to the intended step: ... && (rm -f .*.bun-build || true) bun test ... && (bun run slop:diff 2>/dev/null || true) Verified: `bash -c 'false && echo A && rm -f X || true'` exits 0 (old, broken), `bash -c 'false && echo A && (rm -f X || true)'` exits 1 (new, correct). Co-Authored-By: Claude Opus 4.7 (1M context) * test(browse): add build validation test for server-node.mjs Two assertions: 1. `node --check` passes on the built `server-node.mjs` (valid ES module syntax). This catches regressions where the post-processing steps (perl regex replacements) corrupt the bundle. 2. No inlined `@ngrok/ngrok` module identifiers (ngrok_napi, platform- specific binding packages). Verifies the --external flag actually kept it external. Skips gracefully when `browse/dist/server-node.mjs` is missing — the dist dir is gitignored, so a fresh clone + `bun test` without a prior build is a valid state, not a failure. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(setup): verify @ngrok/ngrok can load on Windows Mirror the existing Playwright verification step. Since @ngrok/ngrok is now externalized in server-node.mjs (resolved at runtime from node_modules), confirm the platform-specific native binary (@ngrok/ngrok-win32-x64-msvc et al.) is installed at setup time rather than surfacing the failure later when the user runs /pair-agent. Same fallback pattern: if `node -e "require('@ngrok/ngrok')"` fails, fall back to `npm install --no-save @ngrok/ngrok` to pull the missing binary. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: bump to v0.18.0.1 for ngrok Windows fix + CI error-propagation Fixes shipped in this version: - Externalize @ngrok/ngrok so the Node server bundle builds on Windows (PRs #1019, #1013; issues #1010, #960) - Shell precedence fix so build/test failures no longer exit 0 in CI - Build validation test for server-node.mjs - Windows setup verifies @ngrok/ngrok native binary is loadable Credit: @tomasmontbrun-hash (#1019), @scarson (#1013). Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 11 +++++++++++ VERSION | 2 +- browse/scripts/build-node-server.sh | 8 +++++++- browse/test/build.test.ts | 28 ++++++++++++++++++++++++++++ package.json | 6 +++--- setup | 4 ++++ 6 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 browse/test/build.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b078e05f..3cc4f230 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [0.18.0.1] - 2026-04-16 + +### Fixed +- **Windows install no longer fails with a build error.** If you installed gstack on Windows (or a fresh Linux box), `./setup` was dying with `cannot write multiple output files without an output directory`. The Windows-compat Node server bundle now builds cleanly, so `/browse`, `/canary`, `/pair-agent`, `/open-gstack-browser`, `/setup-browser-cookies`, and `/design-review` all work on Windows again. If you were stuck on gstack v0.15.11-era features without knowing it, this is why. Thanks to @tomasmontbrun-hash (#1019) and @scarson (#1013) for independently tracking this down, and to the issue reporters on #1010 and #960. +- **CI stops lying about green builds.** The `build` and `test` scripts in `package.json` had a shell precedence trap where a trailing `|| true` swallowed failures from the *entire* command chain, not just the cleanup step it was meant for. That's how the Windows build bug above shipped in the first place — CI ran the build, the build failed, and CI reported success anyway. Now build and test failures actually fail. Silent CI is the worst kind of CI. +- **`/pair-agent` on Windows surfaces install problems at install time, not tunnel time.** `./setup` now verifies Node can load `@ngrok/ngrok` on Windows, just like it already did for Playwright. If the native binary didn't install, you find out now instead of the first time you try to pair an agent. + +### For contributors +- New `browse/test/build.test.ts` validates `server-node.mjs` is well-formed ES module syntax and that `@ngrok/ngrok` was actually externalized (not inlined). Gracefully skips when no prior build has run. +- Added a policy comment in `browse/scripts/build-node-server.sh` explaining when and why to externalize a dependency. If you add a dep with a native addon or a dynamic `await import()`, the comment tells you where to plug it in. + ## [0.18.0.0] - 2026-04-15 ### Added diff --git a/VERSION b/VERSION index 42b43e04..d6bda5aa 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.18.0.0 +0.18.0.1 diff --git a/browse/scripts/build-node-server.sh b/browse/scripts/build-node-server.sh index 539e391c..3ab652ac 100755 --- a/browse/scripts/build-node-server.sh +++ b/browse/scripts/build-node-server.sh @@ -14,13 +14,19 @@ DIST_DIR="$GSTACK_DIR/browse/dist" echo "Building Node-compatible server bundle..." # Step 1: Transpile server.ts to a single .mjs bundle (externalize runtime deps) +# +# Externalize packages with native addons, dynamic imports, or runtime resolution. +# If you add a new dependency that uses `await import()` or has a .node addon, +# add it here. Otherwise `bun build --outfile` will fail with +# "cannot write multiple output files without an output directory". bun build "$SRC_DIR/server.ts" \ --target=node \ --outfile "$DIST_DIR/server-node.mjs" \ --external playwright \ --external playwright-core \ --external diff \ - --external "bun:sqlite" + --external "bun:sqlite" \ + --external "@ngrok/ngrok" # Step 2: Post-process # Replace import.meta.dir with a resolvable reference diff --git a/browse/test/build.test.ts b/browse/test/build.test.ts new file mode 100644 index 00000000..050f3576 --- /dev/null +++ b/browse/test/build.test.ts @@ -0,0 +1,28 @@ +import { describe, test, expect } from 'bun:test'; +import { execSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; + +const DIST_DIR = path.resolve(__dirname, '..', 'dist'); +const SERVER_NODE = path.join(DIST_DIR, 'server-node.mjs'); + +describe('build: server-node.mjs', () => { + test('passes node --check if present', () => { + if (!fs.existsSync(SERVER_NODE)) { + // browse/dist is gitignored; no build has run in this checkout. + // Skip rather than fail so plain `bun test` without a prior build passes. + return; + } + expect(() => execSync(`node --check ${SERVER_NODE}`, { stdio: 'pipe' })).not.toThrow(); + }); + + test('does not inline @ngrok/ngrok (must be external)', () => { + if (!fs.existsSync(SERVER_NODE)) return; + const bundle = fs.readFileSync(SERVER_NODE, 'utf-8'); + // Dynamic imports of externalized packages show up as string literals in the bundle, + // not as inlined module code. The heuristic: ngrok's native binding loader would + // reference its own internals. If any ngrok internal identifier appears, the module + // got inlined despite the --external flag. + expect(bundle).not.toMatch(/ngrok_napi|ngrokNapi|@ngrok\/ngrok-darwin|@ngrok\/ngrok-linux|@ngrok\/ngrok-win32/); + }); +}); diff --git a/package.json b/package.json index 09c6bbc0..bbc1a6d1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "0.18.0.0", + "version": "0.18.0.1", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", @@ -8,12 +8,12 @@ "browse": "./browse/dist/browse" }, "scripts": { - "build": "bun run gen:skill-docs --host all; bun build --compile browse/src/cli.ts --outfile browse/dist/browse && bun build --compile browse/src/find-browse.ts --outfile browse/dist/find-browse && bun build --compile design/src/cli.ts --outfile design/dist/design && bun build --compile bin/gstack-global-discover.ts --outfile bin/gstack-global-discover && bash browse/scripts/build-node-server.sh && git rev-parse HEAD > browse/dist/.version && git rev-parse HEAD > design/dist/.version && chmod +x browse/dist/browse browse/dist/find-browse design/dist/design bin/gstack-global-discover && rm -f .*.bun-build || true", + "build": "bun run gen:skill-docs --host all; bun build --compile browse/src/cli.ts --outfile browse/dist/browse && bun build --compile browse/src/find-browse.ts --outfile browse/dist/find-browse && bun build --compile design/src/cli.ts --outfile design/dist/design && bun build --compile bin/gstack-global-discover.ts --outfile bin/gstack-global-discover && bash browse/scripts/build-node-server.sh && git rev-parse HEAD > browse/dist/.version && git rev-parse HEAD > design/dist/.version && chmod +x browse/dist/browse browse/dist/find-browse design/dist/design bin/gstack-global-discover && (rm -f .*.bun-build || true)", "dev:design": "bun run design/src/cli.ts", "gen:skill-docs": "bun run scripts/gen-skill-docs.ts", "dev": "bun run browse/src/cli.ts", "server": "bun run browse/src/server.ts", - "test": "bun test browse/test/ test/ --ignore 'test/skill-e2e-*.test.ts' --ignore test/skill-llm-eval.test.ts --ignore test/skill-routing-e2e.test.ts --ignore test/codex-e2e.test.ts --ignore test/gemini-e2e.test.ts && bun run slop:diff 2>/dev/null || true", + "test": "bun test browse/test/ test/ --ignore 'test/skill-e2e-*.test.ts' --ignore test/skill-llm-eval.test.ts --ignore test/skill-routing-e2e.test.ts --ignore test/codex-e2e.test.ts --ignore test/gemini-e2e.test.ts && (bun run slop:diff 2>/dev/null || true)", "test:evals": "EVALS=1 bun test --retry 2 --concurrent --max-concurrency ${EVALS_CONCURRENCY:-15} test/skill-llm-eval.test.ts test/skill-e2e-*.test.ts test/skill-routing-e2e.test.ts test/codex-e2e.test.ts test/gemini-e2e.test.ts", "test:evals:all": "EVALS=1 EVALS_ALL=1 bun test --retry 2 --concurrent --max-concurrency ${EVALS_CONCURRENCY:-15} test/skill-llm-eval.test.ts test/skill-e2e-*.test.ts test/skill-routing-e2e.test.ts test/codex-e2e.test.ts test/gemini-e2e.test.ts", "test:e2e": "EVALS=1 bun test --retry 2 --concurrent --max-concurrency ${EVALS_CONCURRENCY:-15} test/skill-e2e-*.test.ts test/skill-routing-e2e.test.ts test/codex-e2e.test.ts test/gemini-e2e.test.ts", diff --git a/setup b/setup index b00608b8..5b974e23 100755 --- a/setup +++ b/setup @@ -292,6 +292,10 @@ if ! ensure_playwright_browser; then cd "$SOURCE_GSTACK_DIR" # Bun's node_modules already has playwright; verify Node can require it node -e "require('playwright')" 2>/dev/null || npm install --no-save playwright + # @ngrok/ngrok is externalized in server-node.mjs and resolved at runtime. + # Verify the platform-specific native binary is installed so /pair-agent + # tunnels don't fail later with a cryptic module-not-found error. + node -e "require('@ngrok/ngrok')" 2>/dev/null || npm install --no-save @ngrok/ngrok ) fi fi