Repository navigation
build: pin Bun 1.4.2 across mise, CI, release and runtime images - #108
Merged
Merged
Conversation
roodboi
marked this pull request as draft
September 30, 2026 13:29
roodboi
force-pushed
the
claude/hack-1211-bun-1-4-2
branch
from
September 30, 2026 13:48
fe78bd5 to
fb44c73
Compare
roodboi
marked this pull request as ready for review
September 30, 2026 15:33
roodboi
added a commit
that referenced
this pull request
Sep 30, 2026
…emoves a replacement (#109) ## Summary Compatibility prerequisites for moving past Bun 1.3.9 ([#108](#108), HACK-1211). Both changes are correct on 1.3.9 as well, so this PR is safe to merge on its own; #108 stacks on it. ### 1. Unix-socket endpoints can no longer be removed by a runtime close (product) From Bun 1.3.14, closing a `node:net` Unix-socket server unlinks its bound path **by name**, whatever that path holds by then. Bun also replaces an existing file at the bound path on `listen`. A standalone probe keeps a replacement file on 1.3.9 and loses it on 1.3.14 and 1.4.2. Three servers bound their public endpoint directly, so a close could remove a foreign replacement that their identity-checked cleanup exists to preserve: - `src/mcp/socket-backend.ts`: `mcp.sock`. Its test `idle cleanup preserves replacement endpoint and claim identities` failed on 1.4.2. - `src/backends/native-https-owner-server.ts`: `control.sock`. Its pre-close path check narrowed the window but did not close it, and `retire()` closed without a fresh check. - `src/backends/native-project-https.ts`: the owner challenge `owner.sock`. `stopOwnerChallenge` closed first and checked identity afterward. **Fix:** the new `listenPublishedUnixSocket` (`src/lib/unix-socket-publish.ts`), the smallest supported change: 1. Bind a fresh staging name in the same private directory, never longer than the endpoint's own name so existing path-length budgets hold, under an owner-only umask. 2. Verify it is this user's socket. 3. Publish it with `link(2)`. This refuses an existing endpoint (`EEXIST`, surfaced as `UnixSocketEndpointExists`) instead of replacing it. 4. Verify the published inode, then remove the staging name. A runtime close-time unlink then finds nothing, and each endpoint is removed only by its caller's existing identity-checked cleanup. On any failure after listening, the helper closes the server, removes the staging name, and removes the endpoint only while it still holds this socket. **Public endpoint contract: unchanged.** The same path, a socket inode, mode `0600`, and the same recorded device/inode (MCP receipt, owner `endpoint.json`); clients connect exactly as before. **Residual:** a hidden staging name exists in the private directory only between `listen` and publication. A crash inside that window could leave one such file behind. It is never bound again and blocks nothing. Per-caller changes are minimal: - **MCP:** keeps its pre-check and maps a publication collision to its existing "already exists" error. - **HTTPS owner:** keeps its identity checks around `chmod`. - **Owner challenge:** records the published identity immediately, so a failure after publication (for example the injected `chmod` failure) removes its own endpoint by identity instead of leaving it behind. ### 2. Tests delete unset environment variables (test-only) Bun 1.4, like Node, stores the string `"undefined"` when a `process.env` key is assigned `undefined`. Tests that unset that way, or restored a captured value that was never set, left `"undefined"` behind, and it leaked into later files in the same `bun test` process. That caused most of #108's 51 failures. - 49 literal unsets now use `Reflect.deleteProperty`. - 63 restores of captured values use a shared `tests/helpers/env.ts` `restoreEnv`, which deletes a missing value. - `config-paths.test.ts`'s local helper is fixed the same way. - No product code assigns `undefined` to `process.env`. ## Verification - **New `tests/unix-socket-publish.test.ts`**, 6 controls, passing on **Bun 1.3.9 and 1.4.2**: - the published endpoint serves clients, survives close, and is removed only by identity; the staging name stays in the same directory, is no longer than the endpoint name, and is gone after publication; - a replacement survives normal and forced close; - an existing endpoint refuses startup and is never replaced; - of two concurrent owners, exactly one publishes and the other leaves it intact; - a restart after owned cleanup publishes a fresh endpoint; - a startup that cannot bind creates nothing. - **Mutation check** on both versions: leaving the staging name, publishing with clobbering `rename`, and binding the endpoint directly (the previous design) each make controls fail. The previous design loses the replacement on 1.4.2, and replaces an existing endpoint at `listen` even on 1.3.9. - **Server test files on 1.3.9 and 1.4.2:** `mcp-socket-backend` 12/12, `native-https-owner` 12/12, `native-project-https` 24/24. - **All 49 affected test files in one `bun test` process** (mirroring CI's shared environment): 410/410 on 1.4.2 and 410/410 on 1.3.9. - Typecheck and `bun run check` pass uncached; the 47 changed test files pass `ultracite check`; privacy ok. Refs HACK-1211. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/hack-dance/codesmith/hack/pr/109?autoLogin=true&ref=codesmith_pr_footer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1793368061&installation_model_id=17053&pr_number=109&ref=codesmith_pr_footer&repository=hack-dance%2Fhack&return_to=https%3A%2F%2Fgithub.com%2Fhack-dance%2Fhack%2Fpull%2F109&signature=48ce6d15d860f958f4b6fe36d7376c8e5c2b2f2c1b8f633f2a03e9879b997622"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith-bot</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> --------- Co-authored-by: hack-cli-tests <tests@hack>
roodboi
force-pushed
the
claude/hack-1211-bun-1-4-2
branch
from
September 30, 2026 15:52
fb44c73 to
a52dcec
Compare
roodboi
marked this pull request as draft
September 30, 2026 15:54
added 6 commits
September 30, 2026 12:39
A review of #109 found two startup ownership gaps. The native HTTPS owner discarded the identity listenPublishedUnixSocket returned and only recorded one after lstat, chmod and lstat of the path. A failure after publication then skipped listener and endpoint cleanup, and a replacement in that window could be chmodded or recorded as the endpoint. The owner now keeps the published identity at once, compares its first observation against it, and never changes the endpoint by path. Failure cleanup always closes the listener, which can no longer unlink anything but the retired staging name, and then removes the endpoint only while it is this listener's inode; a replacement is kept. The helper awaited listen() outside its cleanup, so a listen that bound the staging name and then failed could leak that socket and listener. Listening is now inside the cleanup, and a failed start removes only a staging socket this user created. The helper also sets the endpoint mode (0600) on the staging inode before publication, so the MCP backend, the HTTPS owner and the owner challenge no longer chmod the public path. Controls: bind-then-fail, mode-preparation failure, an endpoint at the AF_UNIX path limit (and one byte over: refused cleanly on Bun 1.3.9, bound in full on 1.4.2, never truncated), and an owner fault or replacement between publication and first observation, on Bun 1.3.9 and 1.4.2.
The private-bind fixture hooked chmod on the public mcp.sock to prove the socket is private before its mode is set and that the creation mask is restored. Since the endpoint's mode is now set on its staging socket before publication, the hook never fired and the check failed. It now observes that staging chmod with the same privacy and mask checks, requires mode 0600, and fails if the published endpoint is ever chmodded by path.
Stand-in invocations ran concurrently (a foreground `up` while the driver polls `ps`) and each did an unlocked read-modify-write of state.json. A `ps` that loaded the state before the second `up` saved its foreground token then saved its stale copy over it, so `up` saw its token gone and exited 0 before readiness; lost call records failed the acceptance test as well. That failed 26 of 40 local runs, and Runtime state models on CI. Each invocation now holds an exclusive flock across its read-modify-write and releases it only before its long waits (the foreground loop and the ps-hang fault). 40 of 40 local runs pass.
…a socket A second review of the Unix-socket publication found three staging-path windows where an entry this attempt could not prove was its own could be changed or removed: - After an ambiguous partial bind (no recorded identity), cleanup unlinked any same-uid socket at the staging name. - The mode was set by path after an awaited lstat, so a replacement in that gap could be chmodded before the postcheck refused it. - On success the staging name was unlinked unconditionally after the awaited link and endpoint lstat. The socket is now created with exactly its mode (the umask during the synchronous bind), so nothing is ever chmodded, and its identity is recorded in the same tick. Publication links only while the staging name is still that socket, and retirement removes it only while it is (check and removal back to back). An entry that is not this socket, or cannot be proven to be, is never removed: on failure it is moved aside while the server closes, so the runtime's close-time unlink by name cannot reach it, and then put back with the same inode. The owner challenge's chmod dependency becomes an afterOwnerSocketPublish test seam, and the private-bind fixture now requires the socket to be created private with mode 0600 and no socket name to be chmodded.
…ng entry The failure close moved an unproven staging entry to one random holding name: if that name was occupied the close went ahead unprotected, and a holding entry created between the check and the rename was overwritten. The entry is now moved with link(2), which never replaces a holding entry, over a bounded list of fresh holding names, and the staging name is dropped only while it is still the linked entry. An entry that cannot be moved aside (every holding name occupied, or not hard-linkable) leaves the close to proceed, now documented as a residual. The helper's documentation now states its scope: accidental and concurrent entries in a private directory, not an adversarial process of the same user, with the remaining path-based and post-return residuals.
Bun 1.3.9 intermittently segfaulted inside its own socket poll dispatch while running tests/native-https-owner.test.ts on the macOS test job (2 of 60 recent test jobs, HACK-1211). Bun 1.4 carries upstream fixes to that usockets dispatch path. Every repository pin moves together: mise.toml, .tool-versions, packageManager, the CI and release workflows, the node and slim runtime images, and the native candidate build's version check. The lockfile is unchanged; a frozen install succeeds.
roodboi
force-pushed
the
claude/hack-1211-bun-1-4-2
branch
from
September 30, 2026 18:24
a52dcec to
a459762
Compare
roodboi
marked this pull request as ready for review
October 1, 2026 17:33
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current integration status — 2026-10-01
PR #112 is merged into
nextatf00a6478, including the acceptance stand-in fix; duplicate #116 is superseded. The pin branch includes that exactnexthistory at heade80171bewithout rewriting its qualified commits. Its source tree is byte-identical toa459762a(5a4150bd); the current diff againstnextis only the 11 Bun-pin files. All eight fresh exact-head CI jobs pass (run 36896025750).Native upgrade and rollback qualification passed on the M3. The signed candidate at
e582353dcombines #122's recovery sourcea330a73awith this unchanged pin. Both versions use the same recovery source; the only difference is the 11 Bun-pin files. The actual compiled CLI revisions were 1.3.9 → 1.4.2 → 1.3.9. All six new bundle manifest entries verified. At both restore points, Event Agent reported 14 services and nine healthy services; a uniquely owned Redis-volume marker survived and was removed after rollback. Verified native HTTPS health/sign-in and all 17 sign-in assets returned 200, and Chrome rendered the sign-in route after upgrade and again after rollback. Frontend and runtime owner paths/hashes matched each selected bundle; wrong-runtime selections were rejected without changing owner configuration. CA bytes/inode, VM birth/disk identities, sibling receipt and all four volume names remained unchanged.Normal Bun 1.4.2
downexited zero, retired the owned lifecycle and native frontend, released dependency/HTTPS listeners, and retained four volumes. An earlier baseline stop used a different tmux server with a same-named foreign session and correctly refused host lifecycle cleanup after native cleanup; the matching app-server context resolved that harness selection error. No foreign session or runtime state was manually removed.Compatibility review, signed build, retained-app upgrade/rollback and exact-head CI gates are complete. This qualification does not establish a crash fix, performance improvement, normal port-443 OAuth/search/image acceptance, or release readiness. Those remain separate work; no release publication is requested. The squash message must describe a compatibility-qualified upgrade and retain the unproven crash-cause/effect caveat.
The sections below preserve historical qualification and review context. Their old branch/dependency/gate status is superseded by this section.
Summary
Moves the repository's Bun toolchain from 1.3.9 to 1.4.2, managed through mise.
Why: Bun 1.3.9 intermittently crashes (
panic(main thread): Segmentation fault, "Bun has crashed") while runningtests/native-https-owner.test.tson the macOStestjob. Occurrences:testjobs when HACK-1211 was filed;Only one of those crashes has a decoded trace. It ends in usockets
us_internal_dispatch_ready_poll.Bun 1.4.0 includes upstream changes to usockets peer-reset and poll-error handling (oven-sh/bun#39600, #39621, #39860). None is shown to fix this segfault, and its cause remains unproven.
This pin is a compatibility-qualified toolchain upgrade, not a proven crash fix. The hosted crashes may or may not stop on 1.4.2, and any crash on 1.4.2 should be recorded per run. The bounded local loop did not reproduce the crash on 1.3.9 either (see below).
Commit message caveat: the pin commit
a459762asays "Bun 1.4 carries upstream fixes to that usockets dispatch path". Its content copies (c1e4f568, and root'sc906cb5c) say the same. That sentence overstates the evidence. Those commits are left unrewritten so that their exact qualified SHAs stay valid. The squash commit message should use the wording above instead.Every pin moves together:
mise.toml,.tool-versions,package.jsonpackageManager;.github/workflows/ci.yml(5 jobs),release.yml(3),release-prepare.yml,release-node-runtime-image.yml;docker/node-runtime/Dockerfileanddocker/slim-runtime/Dockerfile(oven/bun:1.4.2);scripts/build-native-candidate.sh's version check anddocs/guides/native-candidate.md.Unchanged on purpose:
bun.lock: a frozen install succeeds without changing it.docs/cli.mdand a test fixture's image string, which are not repository pins.Release note: release binaries and the node/slim runtime images will be built with Bun 1.4.2 from the next release on. Release qualification stays with the release workflow.
Verification
mise install bun@1.4.2, thenbun --versionreports 1.4.2.bun install --frozen-lockfile: lockfile unchanged.a459762aon 1.4.2, uncached (TURBO_FORCE=1, 0 cached):bun run typecheckandbun run checkpass.bun run privacy:checkata459762a: ok.a459762a(this head, the pin on top of fix: keep a published Unix socket's identity through startup failure #112): 1,774 tests across 250 files, 1,707 pass, 67 skip, 0 fail, no Bun crash markers, and no new host crash reports during the run. Run sequentially, with the host otherwise idle.1.3.9pin remains in workflows, Dockerfiles,mise.toml,.tool-versionsorpackageManager. The remaining mentions are the intentional ones listed above, one AF_UNIX test comment about 1.3.9 behavior, and an unrelated fixture version string.CI at exact head
a459762a(run 36758349825,pull_request, attempt 1)All 8 checks pass:
test, docker-e2e, Runtime state models, linux-process-lifetime, runtime-images, secret-scan;Candidate core on macOS and on Ubuntu.
Bun actually used: every Bun job installed
1.4.2+744846f84(bun --revision). The runtime images build fromoven/bun:1.4.2. The@types/bun@1.3.5in install logs is the type package, not the runtime.test(blacksmith macOS 15): 1,774 tests across 250 files, 1,676 pass, 98 skip, 0 fail (102.7 s).dist/hackwas compiled by 1.4.2 (bun build index.ts --compile).Candidate core jobs install only Zig through mise. They exercise the Rust candidate, not Bun.
Local and CI skip counts differ (67 vs 98) because of live-gated tests that depend on the environment.
Limit: one green CI run cannot show that a crash of about 3% frequency is gone. HACK-1211 defines a bounded reproduction and comparison experiment for that. It has not been dispatched.
Refs HACK-1211.
Integration review against the current candidate (
codex/event-agent-https-acceptanceat2fe54aa9, read-only)83571a61..a459762a) applies to2fe54aa9with a clean merge (git merge-tree). The candidate's copies of the fix: keep a published Unix socket's identity through startup failure #112 files are identical to83571a61.process.envset toundefined;Its other changes are Rust (
packages/runtime-core), models and a Python acceptance test, none of which run on Bun.scripts/build-native-candidate.shrefuses any Bun other than exactly 1.4.2 (exit 69)..hacktoolchain container takes Bun frommise.toml, so it follows this pin on rebuild. Itsbuildtask and the 1.3.9 cross-device staging workaround comments (.hack/toolchain/run.sh,.hack/README.md) are unexercised on 1.4.2; keeping the workaround is harmless.@types/bunislatest(resolved 1.3.5) and lags the runtime; typecheck passes.Combined-tree qualification with #120 (local, Bun 1.4.2)
Source:
f88c4227f2c7776fb8d6f99fb1a2148c57d40105(codex/stopped-retained-restart, whose fix: keep a published Unix socket's identity through startup failure #112 files are identical to83571a61).a459762a, applied by content (cherry-pick, same +/- lines).c1e4f5687ab294ca5b288fa7ccb5690c2a38de33(tree2de1cea518384a1eda7a46c60362a5c7b7acda9b), preserved and pushed asclaude/hack-1211-bun-1-4-2-combined. The combination needed no code changes, and no cumulative PR was opened.1.4.2+744846f84via mise; macOS host; stages run sequentially.Gates (all exit 0):
bun install --frozen-lockfile: no tracked changes.TURBO_FORCE=1, 0 cached:typecheckandcheck(which includesprivacy:check) pass.test, 0 cached: 1,804 tests across 251 files, 1,737 pass, 67 skip, 0 fail, 7,936expect()calls (127.8 s), no crash markers. The pass count equals fix(native): align stopped recovery preflight with retained startup #120's local 1.3.9 gate (1,737).Compiled CLI:
bun run buildbundled 588 modules intodist/hack, which embeds runtime 1.4.2 (BUN_BE_BUN=1). The smoke used an isolatedHACK_HOMEandHOME, with no Docker, daemon, native runtime, ports or routing. It passed 12 of 12 checks:--version(hack v4.2.1),--help,help --all;projects --json(valid JSON);The first smoke run had one FAIL: my check expected a bare
4.2.1. I corrected it and reran the whole smoke. The doctor, agent-docs-sync, tmux-session and Docker tiers were not run.Crash-path regression. The hosted crash site is
tests/native-https-owner.test.ts. #120's run 36776561885, attempt 1, shows Bun 1.3.9panic(main thread): Segmentation fault at address 0x80, exit 133, about 11.6 s into a fullbun testprocess; attempt 2 passed. The loop ran that file 50 times per version (50 of 50 iterations attempted, 274 s), alternating a Bun 1.3.9 control:The SIGKILL was in iteration 32: the run exited 137 about 2 s in, after printing only its header.
Interpretation:
tool(SIGKILL, code signature invalid), whose parent is ahack_runtime_coretest binary from another session. No Cargo ran here.Not run:
pushruns for branches), and no cumulative PR was opened, so there is none.Carry-over to #121 (
4a9ac29dc24a98b235ac380f6f3cd3077069c80f): #121 adds one commit overf88c4227, touching only Rust and the README inpackages/runtime-core. The pin applies to it cleanly (merge-tree tree50c54c3b). Everything Bun executes is byte-identical to the testedc1e4f568, so this qualification covers #121 plus the pin. It is branch qualification only; it is not native or browser acceptance.Dependency and status (draft until results are reviewed)
Stacked on hack-dance/hack#112 at
83571a61(startup ownership fixes; all required checks pass there). Until #112 lands, this PR's diff includes #112's commits. Review only the top commit,a459762a(the pin). After #112 lands, this branch is rebased ontonextso that only the pin remains. It is the same patch as the earliera52dcec9andfb44c730, with only hunk offsets inci.ymlmoved.History: the first CI run at
fe78bd53(pin only) failedtestwith 51 of 1,760 tests and no Bun crash. Two Bun behavior changes caused it, both fixed by hack-dance/hack#109 (merged,98c2694b):link(2)from a staging name. fix: keep a published Unix socket's identity through startup failure #112 then closes the startup ownership windows that review found.undefinedto aprocess.envkey stores"undefined". fix: publish Unix socket endpoints by link so a runtime close never removes a replacement #109 deletes such variables in tests.Gate (explicit):
a459762ais green, and the local combined-tree qualification with fix(native): align stopped recovery preflight with retained startup #120f88c4227passes (above).nextafter fix: keep a published Unix socket's identity through startup failure #112 lands, and must not be merged or released from here.