mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-08 21:49:45 +08:00
fix: ngrok Windows build + close CI error-swallowing gap (v0.18.0.1) (#1024)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
28
browse/test/build.test.ts
Normal file
28
browse/test/build.test.ts
Normal file
@@ -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/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user