Skip to content

test(runtime-host): observe pre-factory drain over handshake, not registration file - #1660

Merged
Astro-Han merged 1 commit into
mainfrom
fix/runtime-host-drain-handshake
Jul 30, 2026
Merged

test(runtime-host): observe pre-factory drain over handshake, not registration file#1660
Astro-Han merged 1 commit into
mainfrom
fix/runtime-host-drain-handshake

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

The drain requested before factory completion begins drain before recovery exactly once test in packages/runtime-host was flaky and failed the test job on main after #1658 landed (run 30544963454), even though #1658 only touches packages/headless and the runtime-host code was identical between the PR and main runs. The flaky test was introduced in #1359.

The flake came from asserting on the registration file mid-flight. requestDrain() flips the in-memory #state to 'draining' synchronously, but persisting that to the registration file is the async writeHostRegistration I/O in #closeResources, and #start() had already persisted 'recovering' before entering the composition factory. The test's fixed sleep(50) bet that I/O landed within 50ms, which lost on loaded CI runners and read back stale 'recovering' (AssertionError: + 'recovering' - 'draining').

That assertion tested a non-contract. No production caller reads HostRegistration.state; clients learn draining from the handshake (#admitHandshake returns kind: 'draining' from #shutdownRequested), and connectResolvedRuntimeHost reads the registration only for rootId, hostEpoch, and the endpoint. The invariant the test name claims, "begins drain before recovery exactly once", is already proven by the final lifecycle array ['factory-return', 'begin-drain', 'recover', 'close'] with begin-drain counted once, and the in-memory draining state is covered elsewhere (candidate.host.state === 'draining').

Drop the sleep(50), the readHostRegistration call, and the registration.state === 'draining' assertion. The test still verifies, while the factory is suspended, that startup has not settled, that no lifecycle event has fired, and that the host still holds the interactive root owner lock, then checks the full lifecycle ordering after release.

A handshake-based replacement was tried first but also raced: #closeResources calls server.close() right after the registration write, so on loaded CI the socket stops accepting before connectRuntimeHost connects, returning unavailable instead of draining (the first push of this PR). Polling the registration file would fix the timing too, but it keeps testing a field no caller reads, so deleting the assertion is the smaller and more correct fix.

Refs #1658 (main CI failure, not a regression from that PR).

Verification

  • npm --workspace @maka/runtime-host run build then node --test dist/__tests__/host-kernel.test.js: 30/30 pass
  • All runtime-host tests in parallel, node --test --test-concurrency=4 "dist/__tests__/*.test.js": 364/364 pass
  • 40 consecutive runs of the targeted test: 40/40 pass (no sleep or I/O left in the assertion path)
  • Reproduced the original flake by injecting a 300ms writeHostRegistration delay: the old sleep(50) form fails with + 'recovering' - 'draining'; the deleted form has no file read to race
  • Reproduced the failed handshake approach by delaying the connect 200ms: returns unavailable, matching the first push's CI failure
  • npm run format:check, npm run lint, npm run typecheck: clean

Astro-Han added a commit that referenced this pull request Jul 30, 2026
requestDrain() flips in-memory #state to 'draining' synchronously, but
persisting that to the registration file is the async
writeHostRegistration I/O in #closeResources, while #start() had already
persisted 'recovering' before entering the factory. The fixed sleep(50)
bet that I/O landed in time, which lost on loaded CI runners and read
back stale 'recovering' (the #1658 main CI failure).
Poll the registration file for 'draining' instead of sleeping a fixed
gap. #closeResources writes 'draining' first, and the file stays
'draining' while the factory is suspended, because removeHostRegistration
runs only after the blocked compositionStartup await later in
#closeResources.
A handshake connection does not work here: #closeResources calls
server.close() right after the registration write, so on loaded CI the
socket stops accepting before connectRuntimeHost finishes its
resolveStorageRoot, mkdir, and registration read, returning 'unavailable'
instead of 'draining' (the #1660 CI failure).
Verified with a 300ms writeHostRegistration delay: the old sleep(50) form
fails with the same 'recovering' seen on main, while the polling form
passes.
@Astro-Han
Astro-Hanforce-pushed the fix/runtime-host-drain-handshake branch from 0583de2 to 8f3ab55CompareJuly 30, 2026 14:30
… on CI
The "drain requested before factory completion begins drain before
recovery exactly once" test failed the test job on main after #1658
landed (run 30544963454), even though #1658 only touches
packages/headless. The flaky test was introduced in #1359.
The flake came from asserting on the registration file mid-flight.
requestDrain() flips in-memory #state to 'draining' synchronously, but
persisting that to the registration file is the async writeHostRegistration
I/O in #closeResources, while #start() had already persisted 'recovering'
before entering the factory. The fixed sleep(50) bet that I/O landed in
time, which lost on loaded CI runners and read back stale 'recovering'.
That assertion tested a non-contract. No production caller reads
HostRegistration.state; clients learn draining from the handshake
(#admitHandshake returns kind:'draining' from #shutdownRequested), and
connectResolvedRuntimeHost reads the registration only for rootId,
hostEpoch, and the endpoint. The invariant the test name claims, "begins
drain before recovery exactly once", is already proven by the final
lifecycle array ['factory-return', 'begin-drain', 'recover', 'close'] with
begin-drain counted once. The in-memory draining state is covered
elsewhere (candidate.host.state === 'draining').
Drop the sleep, the readHostRegistration call, and the state assertion.
The test still verifies, while the factory is suspended, that startup has
not settled, that no lifecycle event has fired, and that the host still
holds the interactive root owner lock, then checks the full lifecycle
ordering after release. A handshake-based replacement was tried first but
also raced: #closeResources calls server.close() right after the
registration write, so on loaded CI the socket stops accepting before
connectRuntimeHost connects, returning 'unavailable' instead of 'draining'
(the first push of #1660).
Refs #1658 (main CI failure, not a regression from that PR).
@Astro-Han
Astro-Hanforce-pushed the fix/runtime-host-drain-handshake branch from 8f3ab55 to 72dac8eCompareJuly 30, 2026 14:53
@Astro-Han
Astro-Han merged commit 19a0093 into mainJul 30, 2026
3 checks passed
Sign up for freeto 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

@Astro-Han