Skip to content

fix(build): externalize socks + sharp in node-server bundle (Bun on Windows)#2260

Open
Spooks444 wants to merge 3 commits into
garrytan:mainfrom
Spooks444:main
Open

fix(build): externalize socks + sharp in node-server bundle (Bun on Windows)#2260
Spooks444 wants to merge 3 commits into
garrytan:mainfrom
Spooks444:main

Conversation

@Spooks444

Copy link
Copy Markdown

Summary

On Windows, bun run build (via setup) failed at the Node server bundle step (browse/scripts/build-node-server.sh). Bun's bundler refuses to inline the native .node addons socks and sharp, aborting the whole build.

This marks socks and sharp as --external so the bundle emits a valid server-node.mjs without inlining native modules. The runtime resolves them from node_modules normally.

Verification

  • bash browse/scripts/build-node-server.shSCRIPT_EXIT=0, emits browse/dist/server-node.mjs (≈498 KB)
  • bun run build (full, what ./setup runs) → BUILD_EXIT=0, all 6 artifacts compile
  • node --check browse/dist/server-node.mjsSYNTAX_OK

Notes

  • Only the build-script change is in this PR; no runtime behavior changed.
  • Pushed to the Spooks444/gstack fork per contributor's push policy (does not target upstream directly).

🤖 Generated with Hermes Agent

@trunk-io

trunk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@Spooks444

Copy link
Copy Markdown
Author

CI / verify note

This PR fixes a Windows bun build regression in browse/scripts/build-node-server.sh
and adds a missing-binary guard. Local verification performed by the author:

What was verified

  1. Build script (Windows/MSYS2): bash browse/scripts/build-node-server.sh runs
    clean (exit 0), bundles 53 modules, and writes browse/dist/server-node.mjs.
  2. Bundle syntax: node --check browse/dist/server-node.mjsSYNTAX_OK.
  3. Idempotent compat-header injection and the removed duplicate createRequire
    import are confirmed present (no _ftp/createRequire redeclaration).
  4. Guard happy path: with browse/dist/browse present, the Step-5 guard prints
    nothing and the script exits 0.
  5. Guard absent-binary path: with an empty browse/dist/, the guard fires and
    prints remediation (bash scripts/build.sh or direct bun build --compile).
  6. End-to-end navigation (loopback): against http://127.0.0.1:8099/ served
    locally, ./browse/dist/browse goto ... reports Navigated to http://127.0.0.1:8099/ (200)
    and exits 0 — proving server start, Chromium launch, and page render all work.

Not verifiable in the author's sandbox (environmental, not a defect)

  • External-URL navigation (goto https://example.com) fails with
    net::ERR_NAME_NOT_RESOLVED because the build sandbox has no outbound DNS.
    The loopback test above isolates this as a network-policy limitation, not a code issue.

How a maintainer can re-confirm on a networked runner

bash scripts/build.sh          # full build incl. compiled 'browse' binary
node --check browse/dist/server-node.mjs
./browse/dist/browse goto https://example.com   # needs outbound network

Bun's bundler refused to inline socks and sharp (both ship
.node native addons), so the Windows node-server build
failed with 'Could not resolve: socks / sharp'.

Added --external socks and --external sharp to
browse/scripts/build-node-server.sh, matching the script's
own documented rule for .node-addon deps. Verified:
bash build-node-server.sh -> exit 0, emits server-node.mjs;
bun run build -> exit 0 (all 6 artifacts compile).
- convert bun build paths to Windows-native via cygpath so the bundle
  actually lands on disk when MSYS_NO_PATHCONV is set
- make Step 3 compat-header injection idempotent (no re-duplicate on rebuild)
- drop the duplicate createRequire import (reuse bundle's own) to fix
  'Identifier createRequire has already been declared' on Node 25
build-node-server.sh only produces server-node.mjs (the Node.js server
bundle). The runnable 'browse' executable is a *compiled* artifact from
scripts/build.sh (bun build --compile) and is gitignored, so a bare
'rm -rf browse/dist' can leave a valid server bundle with no CLI binary.
Add a Step 5 that detects the missing binary and prints remediation
instead of failing later with a cryptic ENOENT.
@Spooks444

Copy link
Copy Markdown
Author

CI / verify note (updated 2026-07-15)

This PR fixes a Windows bun build regression in browse/scripts/build-node-server.sh
and adds a missing-binary guard.

What was verified

  1. Targeted build script (Windows/MSYS2): bash browse/scripts/build-node-server.sh
    runs clean (exit 0), bundles 53 modules, writes browse/dist/server-node.mjs.
  2. Bundle syntax: node --check browse/dist/server-node.mjsSYNTAX_OK.
  3. Full project build (bun run build, which runs bash scripts/build.sh) — OFFLINE,
    exit 0:
    vendor:xterm resolves from local node_modules, all 55 SKILL.md files
    regenerated, browse binary compiled (browse/dist/browse.exe), server-node.raw.mjs
    bundled (53 modules), final build-node-server.sh produced server-node.mjs.
    => The project-level build command works in an offline environment.
  4. Idempotent compat-header injection and the removed duplicate createRequire
    import are confirmed present (no _ftp/createRequire redeclaration).
  5. Guard happy path: with browse/dist/browse present, the Step-5 guard prints
    nothing and the script exits 0.
  6. Guard absent-binary path: with an empty browse/dist/, the guard fires and
    prints remediation (bash scripts/build.sh or direct bun build --compile).
  7. End-to-end navigation (loopback): against http://127.0.0.1:8099/ served
    locally, ./browse/dist/browse goto ... reports Navigated to http://127.0.0.1:8099/ (200)
    and exits 0 — proving server start, Chromium launch, and page render all work.

Not verifiable in the author's sandbox (environmental, not a defect)

  • External-URL navigation (goto https://example.com) fails with
    net::ERR_NAME_NOT_RESOLVED because the build sandbox has no outbound DNS.
    The loopback test above isolates this as a network-policy limitation, not a code issue.

Previous "network requirement" caveat is WITHDRAWN

An earlier note suggested scripts/build.sh might need outbound network. A full
offline bun run build now disproves that — vendor:xterm and gen:skill-docs both
succeed from local node_modules.

How a maintainer can re-confirm on a networked runner

bun run build                      # full build incl. compiled 'browse' binary
node --check browse/dist/server-node.mjs
./browse/dist/browse goto https://example.com   # needs outbound network

@Spooks444

Copy link
Copy Markdown
Author

Ready for review / merge. Branch is rebased onto current main (mergeable: MERGEABLE), all three commits verified (full offline bun run build exit 0; node-server bundle + browse binary produced). No review requests pending. @garrytan please run /trunk merge when convenient.

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