node:cluster: let workers shut down gracefully on worker.disconnect()

[robobun]

Problem

• worker.disconnect() called from the primary does not shut a worker down cleanly. Three separate gaps, each reproduced by a test in test/js/node/cluster.test.ts:
  • A worker's node:http server keeps it alive forever. _http_server.ts binds its own socket in a worker instead of going through cluster._getServer, so the child's handle table (src/js/internal/cluster/child.ts, handles) never contains it and _disconnect() has nothing to close.
  • Worker.prototype.disconnect on the primary (src/js/internal/cluster/primary.ts) called this.process.disconnect() itself. The worker is supposed to close its servers and then disconnect from its side; closing the channel from the primary skips that, and if disconnect() is called before the worker has booted, the worker's online message is lost.
  • Calling disconnect() twice re-ran the worker's _disconnect(), which closed the servers again and tore down an already-closed channel. Node makes the repeat a no-op.
• The originally reported case (Cluster module freezes on worker disconnect when using unshared TCP servers #20642: a net.Server keeping the worker alive) is already fixed on current main by the kClusterOwner linkage between node:net and the cluster child, so that part of this PR was dropped during the rebase. Its test passes on main without this change.

Fix

• child.ts: add cluster._trackServer(server), which puts a server into the handle table the disconnect protocol closes, and removes it when the server closes on its own. _disconnect() returns early if it already ran.
• _http_server.ts: call cluster._trackServer in the worker listen path.
• primary.ts: stop calling this.process.disconnect(); this matches Node's Worker.prototype.disconnect, which only sends the disconnect message and drops the worker's handles.
• Verification, debug build on current main:
  • src/ reverted to main: the three new tests fail (http variant times out; the other two get no online / no events).
  • With the fix: all three pass; the rest of cluster.test.ts is unchanged.
  • All 80 test/js/node/test/parallel/test-cluster-*.js upstream tests pass.

Background

• In node:cluster, a worker registers each listening server with the primary via cluster._getServer; the child keeps a table of those handles. worker.disconnect() in the primary sends an internal disconnect message, and the worker's _disconnect() closes everything in that table, waits for the close callbacks, and then calls process.disconnect() to drop the IPC channel, at which point nothing keeps its event loop alive and it exits. The primary emits exit when the process is gone.
• Bun's node:http does not use _getServer in workers; it listens on its own socket, so it falls outside that table unless registered explicitly.

Earlier shape of this PR

The PR originally shared an owner_symbol between node:net and the cluster child so the faux handle returned by _getServer could find its net.Server, which is what #20642 reports. main has since reworked that area (kClusterOwner, kClusterFauxListen), so after rebasing onto 95cb6939fa the net.ts / shared.ts changes and the net.Server tests were removed as redundant. The three remaining fixes were squashed into one commit.

Fixes #20642

---

no test proof · iteration 20 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/cluster.test.ts

[robobun]

^{Updated 9:40 AM PT - Aug 15th, 2026}

❌ @robobun, your commit 7cd2886 has 1 failures in Build #97889 (All Failures):

• test/js/node/test/parallel/test-cluster-shared-leak.js - timeout on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 30548

That installs a local version of the PR into your bun-30548 executable, so you can run:

bun-30548 --bun

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: adbf9c80-62f6-4ad2-a536-bb1ecfc24532

📥 Commits

Reviewing files that changed from the base of the PR and between 8437683 and 7cd2886.

📒 Files selected for processing (4)

• src/js/internal/cluster/child.ts
• src/js/internal/cluster/primary.ts
• src/js/node/_http_server.ts
• test/js/node/cluster.test.ts

Walkthrough

This PR fixes a hang that occurs when Bun cluster workers call disconnect() while holding an active unshared TCP server. The fix introduces a shared owner_symbol to track server ownership across modules, uses it to link faux handles to their owning servers in the cluster path, and adds regression test coverage to verify workers exit cleanly.

Changes

Cluster worker disconnect with TCP server

Layer / File(s)	Summary
Shared symbol definition and module imports src/js/internal/shared.ts, src/js/internal/cluster/child.ts, src/js/node/net.ts	owner_symbol is added to the shared symbols collection and imported in both cluster/child.ts and net.ts, replacing local symbol declarations to ensure identity alignment across modules.
Handle-server association in listenInCluster src/js/node/net.ts	In the cluster _getServer callback, the faux handle returned by the primary is explicitly linked to the server via handle[owner_symbol] = server, and a close hook ensures the faux handle is closed when the server closes, enabling proper cleanup during worker disconnect.
Test fixture and regression test for worker disconnect test/js/node/cluster.test.ts, test/js/node/cluster/worker-disconnect-with-tcp-server-fixture.ts	A new test fixture reproduces the worker disconnect scenario with active TCP servers, and a regression test in cluster.test.ts verifies the process exits successfully within a timeout with expected lifecycle logs.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All code changes directly address issue #20642: unifying owner_symbol across modules [1], linking faux handles to servers [2], ensuring close callbacks execute [3], and adding test verification [4] that reproduces and confirms the fix.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the worker disconnect hang: symbol sharing in internal/shared, imports in child.ts and net.ts, and test fixtures verifying the fix—no extraneous modifications.
Title check	✅ Passed	The title clearly describes the primary change: graceful worker shutdown when worker.disconnect() is called.
Description check	✅ Passed	The description clearly explains the problem, fixes, background, and verification, although it uses different headings from the template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: ready for review at 7cd2886 (rebuilt on main 95cb6939fa). Latest bot review is an LGTM; the one remaining thread is answered and will be marked resolved shortly.

The case #20642 reports (a net.Server keeping the worker alive after worker.disconnect()) is already fixed on main by the kClusterOwner wiring; this PR's test for it passes on main unmodified, so that part was dropped. What remains are three disconnect bugs that still reproduce on main, each pinned by a test in test/js/node/cluster.test.ts:

1. A worker's node:http server is never closed by disconnect() (it does not go through _getServer), so the worker never exits.
2. The primary tore the IPC channel down itself instead of letting the worker do it after closing its servers; calling disconnect() before the worker booted also lost its online event.
3. A second disconnect() re-ran the worker's shutdown and tore down an already-closed channel; Node treats it as a no-op.

Verification (debug build): with src/ reverted the three tests fail; with the fix they pass, the rest of cluster.test.ts is unaffected, and all 80 upstream test-cluster-*.js tests pass.

About the red test-cluster-shared-leak.js on build 97889 (Windows 2019 x64): that test exercises primary-side disconnect(), so I checked it rather than assuming flake. It already times out intermittently on main's Windows lanes (flagged flaky in 18 of the last 30 main builds, same timeout signature). A/B on a Windows box, 40 runs each against the same build with only this PR's three source files varied: 5 hangs without this change, 2 with it. So the hang is pre-existing and this PR does not make it worse; it has been reported for main separately. Build 97889 has finished and is otherwise green: 178 jobs passed, and that Windows lane is the only failure.

The darwin-14 aarch64 shards that expired waiting for an agent were retried and passed (that lane targets the small release-tier=previous arm64 pool, so expirations there are queue backlog).

[claude]

Fix looks correct and well-tested, but it rewires faux-handle ↔ net.Server lifecycle in listenInCluster and activates the previously-dead maxConnections check in cluster's onconnection — subtle enough that someone familiar with Bun's cluster/net internals should give it a look.

Extended reasoning...

Overview

This PR fixes #20642 (cluster worker hangs on disconnect() when holding a net.Server). It touches three runtime files: src/js/internal/shared.ts (adds a shared owner_symbol), src/js/internal/cluster/child.ts (imports the shared symbol instead of a private one), and src/js/node/net.ts (imports the shared symbol and, in listenOnPrimaryHandle, sets handle[owner_symbol] = server plus server.once('close', () => handle.close())). It also adds a regression test + fixture. My earlier nit about the explicit 30_000 test timeout was addressed in a23fee2.

Security risks

None. This is internal handle-lifecycle bookkeeping between node:cluster child and node:net; no auth, crypto, user input parsing, or privilege boundaries are involved.

Level of scrutiny

Moderate-to-high. The diff is small (~15 runtime lines), the root-cause analysis is convincing, and the faux handle's close() is idempotent (key === undefined guard) so the new server.once('close', …) hook can't double-free. However, this is core node:net + node:cluster plumbing: it changes which object Worker#_disconnect closes (now the net.Server rather than the faux handle directly), it activates the previously-dead maxConnections branch in onconnection(), and the new owner_symbol/close-listener wiring in listenOnPrimaryHandle applies to both the round-robin and shared-handle code paths. These are exactly the kind of lifecycle interactions that benefit from review by someone with context on Bun's cluster implementation.

Other factors

No CODEOWNERS cover these files. The PR claims all test-cluster-* Node parallel tests still pass, and the new regression test exercises the exact reported hang. No bugs were flagged by the bug-hunting system. Given the subtlety of the area rather than any specific concern, I'm deferring rather than approving.

[claude]

Thanks for dropping the timeout. I didn't find any further issues — the symbol unification and faux-handle linkage look correct and the faux close() is idempotent — but since this changes teardown/ref-counting behavior in the core node:net cluster-worker listen path (and incidentally activates the previously-dead maxConnections check in onconnection), it's worth a quick human pass before merge.

Extended reasoning...

Overview

This PR fixes a hang when a cluster worker calls disconnect() while holding a net.Server. It touches:

• src/js/internal/shared.ts: adds a shared owner_symbol.
• src/js/internal/cluster/child.ts: imports the shared symbol instead of declaring a private one.
• src/js/node/net.ts: imports the shared symbol instead of declaring a private one, and in listenInCluster's listenOnPrimaryHandle callback sets handle[owner_symbol] = server plus server.once("close", () => handle.close()) so the faux round-robin handle is torn down when the server closes.
• A new fixture and regression test in test/js/node/cluster.test.ts.

The root-cause analysis is solid: the two modules previously had distinct private Symbol("owner_symbol") instances, so Worker#_disconnect could never find the owning server, fell back to the faux close() (which ignores its callback), and waitingCount never hit zero.

Security risks

None identified. No user-controlled input handling, auth, or privilege boundaries are affected; this is internal IPC/lifecycle wiring between cluster worker and primary.

Level of scrutiny

Moderate. The diff is small (~15 runtime lines) and gated to the cluster-worker branch of listenInCluster (!cluster.isPrimary && !exclusive), so the blast radius for non-cluster net users is nil. However:

• It changes teardown ordering and ref-counting (setInterval unref via faux handle.close()) in a path with subtle Node-compat semantics.
• Sharing owner_symbol between net.ts and cluster/child.ts also activates previously-dead code: the maxConnections guard in onconnection() (child.ts) now actually resolves to the net.Server and can reject connections. This matches Node.js, but it is a behavioral change beyond the headline fix.
• server.once("close", () => handle.close()) plus the user's own server.close() and _disconnect's server.close(checkWaitingCount) means handle.close() may be reached after handles.clear() has already run; I traced this and the faux close() is idempotent (if (key === undefined) return), so it's safe.

Other factors

• My one prior nit (explicit 30_000 test timeout) was addressed in a23fee2 / 2118a4b.
• The bug-hunting system found no issues.
• CI is green for all test-cluster-* and the new test across lanes; remaining failures are documented as unrelated flakes.
• No CODEOWNERS cover these paths.

Given it's correct as far as I can tell but alters lifecycle semantics in core node:net/node:cluster, deferring to a human for a quick look rather than auto-approving.

[claude]

Fix looks correct and the timeout nit was addressed — deferring to a human since this rewires net/cluster handle lifecycle (and as a side effect activates the previously-dead maxConnections check in onconnection), which is worth a maintainer's eyes.

Extended reasoning...

Overview

This PR fixes #20642: cluster workers hang forever after worker.disconnect() when they own a net.Server. The root cause is that internal/cluster/child.ts and node/net.ts each declared their own private Symbol("owner_symbol"), so Worker#_disconnect could never find the owning server on the faux round-robin handle, fell back to handle.close(cb) (which ignores the callback), and waitingCount never reached zero.

The fix:

• Hoists owner_symbol into internal/shared.ts and imports it in both modules so they agree on identity.
• In listenInCluster's listenOnPrimaryHandle callback (src/js/node/net.ts), sets handle[owner_symbol] = server and adds server.once("close", () => handle.close()) so closing the server also tears down the faux handle (unref the keep-alive interval + notify primary).
• Adds a regression test + fixture spawning two workers that each listen, connect to themselves, then disconnect() + server.close().

My earlier nit (drop the 30_000 per-test timeout) was addressed in a23fee2 and the PR was rebased onto current main at 24e6306.

Security risks

None. This is internal lifecycle plumbing between node:net and node:cluster in worker processes; no auth, crypto, parsing, or external input handling is touched.

Level of scrutiny

Medium-high. While the diff is small (~15 source lines + tests), it touches:

• src/js/node/net.ts — a core, very widely-used module, though the change is confined to the !cluster.isPrimary branch of listenInCluster, so blast radius is limited to cluster workers.
• Handle-close ordering between net.Server, the faux RR handle, and the shared-handle path. The faux close() is idempotent (if (key === undefined) return), so the new server.once("close", ...) chain doesn't risk double-close — but Bun's architecture here diverges from Node's (Bun keeps the real Bun.listen() handle as server._handle rather than swapping in the faux handle), so this is Bun-specific plumbing rather than a straight Node port.
• A side-effect behavioral change: unifying owner_symbol activates the previously-dead maxConnections check in child.ts's onconnection() (lines ~213-220). Cluster workers will now actually reject connections when server.maxConnections is exceeded. The PR description calls this out, but it's a second behavior change riding on the same symbol fix and isn't directly covered by the new test.

Other factors

• The regression test gates the fix (author verified it hangs without the src/ change and passes with it).
• Author reports all existing test-cluster-* Node parallel tests still pass after rebase.
• No CODEOWNERS cover these paths.
• No bugs were found by the automated review.

Given the subtle net/cluster lifecycle interaction and the secondary maxConnections activation, a maintainer familiar with Bun's cluster internals should sign off rather than auto-approving.

[claude]

All prior nits are addressed and I found no further issues, but I'm leaving this for a maintainer since it rewires the net↔cluster faux-handle linkage (and incidentally activates the previously-dead maxConnections check in onconnection()).

Extended reasoning...

Overview

This PR fixes a hang where cluster workers never exit after worker.disconnect() when they own a net.Server. The root cause was that internal/cluster/child.ts and node/net.ts each declared a private Symbol("owner_symbol"), so Worker#_disconnect's handle[owner_symbol] lookup on the round-robin faux handle always missed, the faux close() ignored the callback, and waitingCount never reached zero. The fix moves owner_symbol into internal/shared.ts, imports it from both modules, and in listenInCluster's callback explicitly sets handle[owner_symbol] = server plus server.once("close", () => handle.close()) so the faux handle's keep-alive interval is unref'd and the primary is notified. A new fixture + regression test in test/js/node/cluster.test.ts covers the scenario.

Security risks

None identified. No input parsing, auth, crypto, or filesystem-path handling is touched. The change is internal symbol plumbing and a close-chain hook between two already-coupled modules.

Level of scrutiny

Moderate-to-high. The diff is small (~15 source lines) and the reasoning is sound, but it sits in a production-critical Node-compat path (node:net × node:cluster) where Bun deliberately diverges from Node.js's design (Bun keeps the real Bun.listen() handle as server._handle rather than the faux handle). Two non-obvious behavioral side-effects warrant a maintainer's eye:

• Unifying owner_symbol activates the previously-dead maxConnections check in child.ts:onconnection(), so workers can now reject round-robin connections they previously accepted. The PR description notes this as an incidental fix, but it is a behavior change beyond the headline bug.
• The new server.once("close", () => handle.close()) hook adds a close chain; the faux close() is idempotent (key === undefined guard) so double-close from _disconnect → server.close → handle.close is safe, but someone familiar with Bun's cluster handle lifecycle should confirm there is no ordering hazard with handles.clear() / IPC teardown.

Other factors

All three of my earlier style nits (explicit test timeout, comment length, standalone expect(stderr).toBe("")) have been addressed in f08fbb6 / a64fd57 / 6b545d0, and the bug-hunting pass found nothing this round. CI on the prior runs was green except for an unrelated darwin artifact-download infra failure. No CODEOWNERS cover these paths. Given the cross-module coupling and the secondary behavior change, I'm not auto-approving — but I have nothing further to request.

[claude]

All prior nits are addressed and no bugs were found; deferring to a human for the substantive review since this changes cluster's IPC disconnect ordering across four src files (drops the primary-side process.disconnect(), adds a Bun-specific _trackServer hook into the http listen path, and shares owner_symbol which also activates the previously-dead maxConnections check in onconnection).

Extended reasoning...

Overview

The PR fixes worker.disconnect() never closing a cluster worker's servers. It touches four source files — internal/cluster/child.ts (import shared owner_symbol, add cluster._trackServer), internal/cluster/primary.ts (remove this.process.disconnect() so the worker owns channel teardown), internal/shared.ts (export a single shared owner_symbol), node/net.ts (link the cluster faux handle to its server and tear it down on server close), and node/_http_server.ts (call cluster._trackServer(server) in the worker listen path) — plus three new regression tests and a fixture. The latest commit (7d9ca45) addressed my last open nit by inserting a blank line between the section header and the 3-line rationale in primary.ts.

Security risks

None apparent. No auth, crypto, permissions, or untrusted-input parsing is touched. The change is internal lifecycle wiring between Bun's cluster, net, and http modules.

Level of scrutiny

Medium-high. This is process-lifecycle / IPC code in core Node-compat modules that real cluster managers depend on for rolling restarts. The individual edits are small, but together they change disconnect ordering: the primary no longer tears the IPC channel down itself, the worker now closes http servers it previously didn't track, and sharing owner_symbol activates the previously-dead maxConnections branch in child.ts's onconnection. _trackServer is also a new Bun-specific internal API (a deliberate deviation from Node's _getServer flow). These are reasonable design calls but worth a human look rather than a bot rubber-stamp.

Other factors

The bug-hunting system found nothing this run. All five of my prior inline nits (per-test timeouts, comment-length caps, combined-object stderr assertion) have been addressed; the last open thread on primary.ts is fixed by 7d9ca45. Test coverage is solid: three new regression tests (worker-initiated net, primary-initiated http, primary-initiated net) plus the author reports all 56 test-cluster-* Node parallel tests still pass and one previously-failing in-worker assertion now passes. CI reds on the branch have been unrelated flakes per the author's status comment. Given the cross-module behavioral change to disconnect semantics, deferring rather than approving.

[robobun]

Pushed c5a17e0 onto this branch: the primary-side change here is necessary but not sufficient for a double worker.disconnect(), which is the normal race in a cluster manager (a rolling restart and an error path both retiring the same worker). Node makes the second call a no-op.

const cluster = require("node:cluster");
if (cluster.isPrimary) {
  const worker = cluster.fork();
  worker.on("online", () => {
    worker.disconnect();
    worker.disconnect();
  });
  worker.on("exit", (code, signal) => console.log("worker-exit", code, signal));
}

	primary	worker
main	dies: uncaught ERR_IPC_DISCONNECTED from Worker.prototype.disconnect	dies: uncaught ERR_IPC_DISCONNECTED
this branch before c5a17e0	survives	dies: worker-exit 1 null
this branch with c5a17e0	survives	worker-exit 0 null (same as Node)

With the primary no longer closing the channel, the second disconnect() sends a second internal {act: "disconnect"} message (Node's sendHelper only bails once the channel is actually gone). The worker ran _disconnect() a second time and called process.disconnect() on a channel it had already torn down:

error: IPC channel is already disconnected
      at checkWaitingCount (internal:cluster/child:210:27)
      at onmessage (internal:cluster/child:63:25)

The guard is a three-line early return in Worker.prototype._disconnect. It also closes a second divergence on this branch: a worker disconnected before its online handshake now emits online on the primary, where main swallowed it (the primary had already closed its read end).

Two tests added to test/js/node/cluster.test.ts: the double-disconnect case fails on this branch without the guard, the online case fails on main. All 59 test/js/node/test/parallel/test-cluster-*.js still pass.

For the record, the buildkite/bun failure on the previous build (66558) is unrelated flake: dev-and-prod.test.ts, watch-many-dirs.test.ts, inspect.test.ts, spawn-pipe-leak.test.ts, napi.test.ts and friends, nothing cluster-related.

[robobun]

Independent report of the same root cause, with a user-visible symptom this PR's description doesn't call out: the primary never emits online for a worker that is disconnected before its handshake lands.

const cluster = require("node:cluster");

if (cluster.isPrimary) {
  const events = [];
  const worker = cluster.fork();
  worker.disconnect(); // before the worker has reported itself online
  worker.on("online", () => events.push("worker:online"));
  cluster.on("online", () => events.push("cluster:online"));
  worker.on("disconnect", () => events.push("worker:disconnect"));
  worker.on("exit", () => {
    events.push("worker:exit");
    console.log(JSON.stringify(events));
  });
}

node v26:  ["worker:online","cluster:online","worker:disconnect","worker:exit"]
bun 1.4.0: ["worker:disconnect"]                     (and the worker never exits)

It is the this.process.disconnect() line in Worker.prototype.disconnect (root cause 3 here): the primary tears the channel down before the worker has booted far enough to send {act:"online"}, so that message is never read. It matters because readiness gates ("wait until all N workers are online, then start accepting") are the standard primary-side pattern, and an early disconnect() from a crash-loop guard or a rolling restart silently drops the count, hanging the manager.

Verified this branch at bd5a2c4ce1 (rebased on d37f52067b) on linux-x64 debug:

• the repro above prints the same four events in the same order as node
• all 54 test/js/node/test/parallel/test-cluster-*.js pass
• all 10 tests in test/js/node/cluster.test.ts pass

The only red lane on build #68658 is darwin 26 aarch64 - test-bun, which carries no test annotation; every failing test annotation on that build (test-tls-reuse-host-from-socket.js, shell/leak.test.ts, http/serve.test.ts, net-mongodb-pattern-leak.test.ts, symlink-path-traversal.test.ts, init.test.ts) is a retry-and-pass warning on an unrelated lane.

I had an independent fix on farm/5f94f0c9/cluster-online-before-disconnect (drop this.process.disconnect(), link the faux round-robin handle to its net.Server in rr(), plus two tests), but it is a strict subset of this PR, so no competing PR. The only thing it asserts that the test here doesn't is the exact ordering including cluster.on("online"); happy to port that over if it's wanted.

[robobun]

Another face of the primary-side this.process.disconnect() call this PR removes: it silently drops messages that worker.send() already accepted.

for (let i = 0; i < 50; i++) worker.send({ i, payload: Buffer.alloc(64*1024, 'z').toString() });
worker.disconnect();
// node: worker receives 50/50, then 'disconnect'
// bun:  worker receives ~3/50, 'disconnect' never fires, send() callbacks report success

Opened #33594 with a narrower subset of this PR's src/ changes (primary.ts + the owner_symbol share and listenInCluster linkage, without the _trackServer/http hook) plus a message-delivery test, so the data-loss fix can land independently while this PR's broader server-shutdown coverage is reviewed.

[robobun]

Triage note: #20642 was closed by #31155, and the fixture from that issue (worker-initiated disconnect() with a net.Server) now passes on main. Three of the five tests added here still fail on current main (f426a8e, which includes #31829), and Node passes all three:

• primary-initiated worker.disconnect() while the worker runs an http server: the worker never exits
• calling worker.disconnect() twice throws ERR_IPC_DISCONNECTED (a no-op in Node)
• worker.disconnect() before the worker is online: 'online' is never emitted

Leaving this open.

[claude]

LGTM — the last nit (per-test timeouts) is addressed in 7cd2886 with the file-level setDefaultTimeout(40_000), and the three new tests no longer carry trailing timeout args.

What was reviewed:

• primary.ts: dropping this.process.disconnect() matches Node's Worker.prototype.disconnect; the worker now owns channel teardown after closing its servers. Checked that removeWorker/removeHandlesForWorker are still safe on a second call.
• child.ts: _trackServer keys handles by the server object, so it can't collide with the string keys from _getServer/onconnection; _disconnect falls through to handle.close(cb) for it. The exitedAfterDisconnect early return only affects the primary-initiated path (worker-initiated already guards on state).
• _http_server.ts: the _trackServer call is inside the !isPrimary branch, and Server.prototype.close(cb) does invoke the callback via setCloseCallback, so checkWaitingCount fires.

Extended reasoning...

Overview

Three small src/ changes (~10 lines) to node:cluster's disconnect lifecycle plus one call site in _http_server.ts, with three new subprocess-spawning tests in test/js/node/cluster.test.ts:

• src/js/internal/cluster/primary.ts: remove this.process.disconnect() from Worker.prototype.disconnect so the worker (not the primary) tears the IPC channel down after closing its servers. This is what Node's reference implementation does.
• src/js/internal/cluster/child.ts: add cluster._trackServer(server) so servers that don't go through _getServer (Bun's node:http binds its own SO_REUSEPORT socket) still land in the handles table that _disconnect() iterates; add an exitedAfterDisconnect early return so a second primary-sent {act:"disconnect"} is a no-op.
• src/js/node/_http_server.ts: call cluster._trackServer(server) in the worker listen branch.

Security risks

None. This is Node-compat lifecycle plumbing between a cluster primary and its forked workers over the existing IPC channel; no new external input, no auth/crypto/permissions surface.

Level of scrutiny

Moderate. node:cluster disconnect is a subtle lifecycle path (channel teardown ordering, event-loop keepalive), but the changes are minimal, each aligns with Node's reference implementation, and each is pinned by a dedicated test. The author reports all 80 upstream test/js/node/test/parallel/test-cluster-*.js tests pass, and the PR description verifies each new test fails on main and passes with the fix.

Other factors

• This PR has been through six-plus review rounds since May; every prior thread (comment length, per-test timeouts, stderr assertions, comment-cop) is resolved. The last open item — my per-test-timeout nit at line 1356 — was addressed in 7cd28869 exactly as suggested (file-level setDefaultTimeout, per-test args dropped from all three new tests).
• I checked the interactions I was unsure about: _trackServer uses the server object as the map key so it cannot collide with onconnection's string-keyed lookups; handles.clear() after the forEach makes the later once("close") deletion a harmless no-op; Server.prototype.close in _http_server.ts does register the callback so checkWaitingCount decrements; the _trackServer call is gated on !isPrimary; and Worker.prototype.destroy already sets exitedAfterDisconnect before exiting, so the new guard doesn't break that path.
• The bug-hunting system found nothing this run.