Skip to content

test(native): read the probe child PID before cancelling and size binding tests for load (HACK-1452) - #495

Merged
roodboi merged 5 commits into
nextfrom
wf/g102-binding-probe-cancel-reap
Oct 25, 2026
Merged

roodboi merged 5 commits into
nextfrom
wf/g102-binding-probe-cancel-reap

Conversation

@roodboi

@roodboi roodboi commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups to #487 (HACK-1432). This changes tests only. The product's cancellation path already joins its probe child, so no product code changes.

Root cause

  1. "Cancelled probe child still alive" was a test race, not a leak. createNativeComposeProbe (src/lib/native-compose-ownership.ts) awaits child.exited on every path before it rethrows: the success path's Promise.all and the finally block's Promise.allSettled. Bun resolves exited only after it reaps the child. A local check found 0 zombies in 400 SIGKILLed detached children, measured right after await child.exited, at host load average 40–180. The binding acquisition and assertFresh call probes one after another, never in a race, so a refusal cannot get ahead of its child. The bad PID came from the test. "captured cancellation cannot be replaced by a later fresh signal" read the PID after the abort, from a started file that the fake Docker child creates and then writes. A child killed between those two steps leaves the file empty. Number("") is 0, and process.kill(0, 0) signals the test's own process group and succeeds. I reproduced this exactly: with the old test, a fake that leaves started empty fails with Received function did not throw / Received value: true.
  2. Start-up poll. "cancelled real child is reaped…" waited 100 × 5 ms for the child. Under load the acquisition takes several seconds before its first probe (14 s at load ~170 here), so the poll expired. The acquisition it never aborted later rejected between tests.
  3. Default probe deadline. About 60 acquisitions used the product's default 15 s probe deadline and refused with E_LEGACY_COMPOSE_BINDING_PROBE under load.
  4. Throw-site coverage. No test drove tryLegacyComposeAdoptedCommand's refusal or cancel throw site, so changing either message passed.

Fix

  • The fake Docker child now publishes its PID by rename, so the file is never seen empty. A named, bounded startedChild() wait (CHILD_START_WAIT_MS = 15 s, polling every 10 ms) returns the PID and rejects 0 and 1. Both cancellation tests read the PID before the abort.
  • observeDockerChildren() records each spawned fake Docker child and when it exits, without changing spawn or the product's timers. This logic was moved out of the timeout test, which now uses it too. expectReaped() asserts, at the moment the refusal resolves, that the published PID is an observed child, that the child already exited by SIGKILL, and that kill(pid, 0) throws. The assertion was not relaxed; it now holds at once and deterministically.
  • The cancellation test always aborts and awaits its acquisition in finally, then restores Bun.spawn. A failed start-up wait can no longer reject into a later test or leave a spy installed.
  • acquireFixtureBinding() gives the file's acquisitions a named FIXTURE_PROBE_DEADLINE_MS (45 s, 3 × the product default) unless a test names its own deadline. The caller-mutation test and the direct source-bind inspection use the same constant. FAKE_DOCKER_TEST_TIMEOUT_MS is scaled to keep its existing rule: a slow host ends in the product's closed refusal before Bun's timeout.
  • Deadline-bound cases. Under load the first two runs failed in two cases this unit had not touched. The moving job snapshot case's 20 s ordered operation refused with "did not become ready", and the 3 s shared-deadline case ended before its first bridge read. ORDERED_OPERATION_DEADLINE_MS is now 4 fixture probe deadlines. Both exhausted-deadline cases ("moving snapshot guard…" and "a persistent endpoint placeholder…") now use EXHAUSTED_PROBE_DEADLINE_MS (15 s) plus EXHAUSTED_REFUSAL_SLACK_MS (5 s). They still require more than two bridge scans and a refusal soon after the deadline.
  • New tests/native-compose-adoption-command-messages.test.ts drives the adopted command's refusal site (loadActive throws a canary) and its cancel site, in both the lease path (ps) and the mutation path (up). It cancels by calling the command's own SIGTERM listener directly; no signal is sent. It maps each result through the completed-job diagnostics and asserts refusal=adopted-refused or refusal=adopted-cancelled, with no canary in the output. The store and the marker selection are module spies, as in tests/native-compose-adoption-publication-diagnostics.test.ts.

Evidence

Each new test was proved by mutation:

  • Probe changed to refuse on abort without joining the child (Promise.race with the abort, exited dropped from finally): both cancellation tests fail at expect(exited).toBe(true).
  • Old captured test with a fake that leaves started empty: fails with Received value: true, the symptom from the issue.
  • Start-up wait forced to 1 ms: with the finally abort and await in place, only the forced test fails. With them removed, # Unhandled error between tests appears.
  • Adopted cancel site given a different literal message: both cancel tests fail with token other. Refusal site given a different literal: the refusal test fails with token other.

Load runs at db1037dc: the whole binding file ran 10 times (2 parallel lanes × 5 runs) with 6 added yes CPU workers, on top of other agents' load, all stopped in a trap. 10/10 passed, 107 pass and 0 fail each. Runs took 547–1180 s, at 1-minute load averages of 22–76 on 18 cores. Before the deadline commit, the same harness at 391af280 failed only the two deadline-bound cases (2/107 in both lanes); that is why the second commit exists. The new message test passes 3/3.

Hosted CI at the exact head db1037dc (run 40126377421) succeeded: the test job (4/4 shards) and every other check.

Gates: bun run check passed (34 existing warnings, none in changed files). bun run --cwd packages/cli typecheck passed with 0 errors; it includes tests/. gitleaks 8.30.1 (gitleaks dir on the git log -p origin/next..HEAD export) found no leaks. The CLI surface and agent guidance did not change.

Gaps

  • The full suite ran only in hosted CI. Locally I ran only the focused files, and no Docker e2e.
  • The reap claim rests on Bun 1.4.2's exited semantics, checked by experiment on macOS. A Bun change that resolved exited before reaping would fail expectReaped, which is intended.

Round 2: moving job deadline sized from the real count

A verifier found that the moving job snapshot case still failed above a load average of about 115. It refused with "Legacy adoption ordered operation refused or did not become ready" in 3 of 4 full-file runs at loads of 112–188. The cause was a wrong count. ORDERED_OPERATION_DEADLINE_MS was 4 fixture probe deadlines (180 s), and its comment said the ordered start makes "about four freshness checks". It makes 20.

Count. I counted the calls to the case's assertFresh closure in a scratch copy. One ordered start makes exactly 20, which matches the V7 job executor (executeLegacyComposeRetainedJobs): 1 on entry; db 3; the seed job 7 (its first completion poll still sees the job running); web 5; then 4 for the final wait, the final read and the exit. The count does not depend on timing: every readiness poll reads fixture files, and the job's exit is triggered by a probe, not by a clock. At load 103–137 each check took 7–17 s, and the 180 s deadline expired at the 20th check (184.6 s).

Fix (tests/native-compose-adoption-binding.test.ts, test only):

  • ORDERED_START_FRESHNESS_CHECKS = 20, with the breakdown in its comment. ORDERED_OPERATION_DEADLINE_MS is now 20 × FIXTURE_PROBE_DEADLINE_MS (900 s), so the start can refuse for its own deadline only after one of its probes could have refused for its own.
  • The case asserts freshnessChecks <= ORDERED_START_FRESHNESS_CHECKS. If the executor ever makes more checks, the case fails at any load, not only on a slow host.
  • The case has its own budget, MOVING_JOB_TEST_TIMEOUT_MS: the ordered deadline, plus 4 probe deadlines for the acquisition, the first resolve, the closing check and the closing resolve, plus 1 for file reads and poll sleeps (1125 s). A slow host still ends in the closed refusal before Bun's timeout. The case owns 24 probes, not about 16.
  • The comment on FAKE_DOCKER_TEST_TIMEOUT_MS was false. I counted createNativeComposeProbe calls per test with temporary instrumentation (since reverted) over one full-file run. Apart from this case, no test creates more than 7 probes, and none uses a probe deadline above 45 s. The comment now says that, and that this case has its own budget.
  • Biome re-indented the case body because it now takes a third argument. With whitespace ignored, the diff is about 55 lines.

Evidence:

  • Mutation: with ORDERED_START_FRESHNESS_CHECKS = 19, the case fails with Expected: <= 19 / Received: 20. That run was at a 1-minute load of 192–222. The ordered start itself completed in about 262 s under the new deadline; only the mutated assertion failed.
  • Before the fix, the old 180 s deadline also failed this case in a full-file run here at load ~140 (106 pass, 1 fail).
  • Load runs at ffb4735c (the final head differs only by the probe-count comment): the whole binding file ran 10 times, in 5 parallel lanes × 2 runs, with 2 added yes workers on top of other agents' load. A trap stopped every worker afterwards, and I confirmed none were left. 10/10 passed, 107 pass and 0 fail each. Each run took 2449–2607 s. The 1-minute load average was 163 at the start, 110–116 between rounds, and 63–67 at the end, on 18 cores.
  • Hosted CI at the exact head 07e1597f (run 40141039934) succeeded: the test job (4/4 shards), docker-e2e (4/4), and every other check.
  • Gates at the head: bun run check passed, including privacy:check, with Biome clean on the changed file. bun run --cwd packages/cli typecheck passed with 0 errors. gitleaks 8.30.1 (gitleaks dir on the git log -p origin/next..HEAD export) found no leaks. The command-messages test passes 3/3. The CLI surface and agent guidance did not change.
  • This round first merged origin/next (191382d8) into the branch. The merge had no conflicts.

Gap: a pass at load ~115 takes about 4 minutes for this case. The 900 s deadline is an upper bound, not the expected duration.

Round 3 (bf3dab7): spare probe deadline

The final verifier counted the probes in the moving job case: 3 before the ordered start (the acquisition makes 2, the first resolve 1), 20 during it and 2 after it, 25 in all. The old budget, the ordered deadline plus 5 probe deadlines, was exactly 25 probe deadlines, so the spare the comment described did not exist. MOVING_JOB_TEST_TIMEOUT_MS is now the ordered deadline plus 6 probe deadlines (1170 s), and the comment gives the real count. Only that test timeout changes; hosted CI at bf3dab7 is the evidence.

Refs HACK-1452

🤖 Generated with Claude Code

roodboi and others added 2 commits October 24, 2026 15:16
…nding probe deadlines (HACK-1452)

The cancellation refusal already joins its probe child: createNativeComposeProbe
awaits child.exited on every path before it rethrows, and Bun resolves exited
only after reaping. The leaked-child report came from the test: it read the PID
after the abort, from a "started" file the fake Docker child created and was
killed before writing. Number("") is 0, and process.kill(0, 0) signals the
test's own process group and succeeds.

- The fake Docker child publishes its PID by rename, and tests read it before
  the abort with a bounded, named start-up wait that rejects PIDs 0 and 1.
- Both cancellation tests now observe the spawned child and assert at refusal
  time that it already exited, was SIGKILLed and is gone.
- The cancellation test always aborts and awaits its acquisition in finally, so
  a failed start-up wait cannot reject into a later test.
- The file's acquisitions use a named 45 s fixture probe deadline through
  acquireFixtureBinding, unless a test names its own deadline.
- New tests drive the adopted command's refusal and cancel throw sites and
  assert the diagnostics' closed tokens.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…HACK-1452)

Under heavy load the moving job snapshot case's 20 s ordered operation refused
with "did not become ready", and the 3 s shared-deadline case ended before its
first bridge read. Name an ordered operation deadline of four fixture probe
deadlines, and give both exhausted-deadline cases a 15 s probe deadline with a
named 5 s refusal slack. They still require repeated scans and a refusal soon
after the deadline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 24, 2026

Copy link
Copy Markdown

HACK-1452

roodboi and others added 3 commits October 24, 2026 18:17
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed freshness checks (HACK-1452)

The moving job snapshot case's ordered start makes 20 assertFresh calls, not
about four, so its 180 s aggregate deadline refused with "did not become
ready" at a load average of about 115 and above. Count the checks, size the
deadline at one fixture probe deadline each, assert the start makes no more,
and give the case its own test budget above that deadline. Correct the file
budget's comment, which named this case's probes as the heaviest test's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…get (HACK-1452)

The case makes 3 probes before its ordered start and 2 after it, so the
old 5-probe allowance left no room for file reads and poll sleeps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@roodboi
roodboi marked this pull request as ready for review October 25, 2026 01:37
@roodboi
roodboi merged commit d6eed4d into next Oct 25, 2026
21 checks passed
@roodboi
roodboi deleted the wf/g102-binding-probe-cancel-reap branch October 25, 2026 01:38
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