Repository navigation
test(native): read the probe child PID before cancelling and size binding tests for load (HACK-1452) - #495
Merged
Conversation
…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>
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
marked this pull request as ready for review
October 25, 2026 01:37
roodboi
added a commit
that referenced
this pull request
Nov 3, 2026
…s for load (HACK-1319) (#551) Closes HACK-1319. This changes tests only: no product timeout or product code changes. ## Findings **"moving job snapshot" fails locally (environmental, not a product defect).** When the ticket was filed, this case's ordered start had a fixed 20 s deadline and a 30 s test limit. On the pre-#487 test at load average ~100, it timed out after 30 s; at ~58 it finished in 18 s of its 20 s deadline. Each fake Docker query is a cold Bun process, so the start was budgeting host process start-up, not product behavior. #495 (HACK-1452) already sizes that deadline from the counted freshness checks (`ORDERED_START_FRESHNESS_CHECKS`). On this branch's base, the case passed in every loaded full-file run below. This PR does not change it. **"real child" tests used wall-clock waits.** - The cancellation tests waited at most 15 s for the hanging fake Docker child to publish its PID. Under load the acquisition alone took ~14 s before its first probe, so the wait expired. - The timeout test armed a 150 ms probe deadline. The probe reads `Date.now()` when it is created and spawns later, so a stall between the two could use up the whole deadline before any child existed (`children.length > 0` failed). **Generation v5/v7 (and static7) cases outgrew their 20/30/60 s limits.** With every limit lifted at load average ~90 (20 cores), the slowest cases took 27 s (plain saved adoption), 44 s (retained operations) and 60 s (public static7 up/down). The public adopted command gives each operation a 90 s deadline (`DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS`), but those tests allowed 60 s in total. A Bun timeout leaves the owner running, so the file's settlement guard makes every later case refuse: one timeout produced 118 failures. ## Changes `tests/native-compose-adoption-binding.test.ts` - `startedChild(pending)` is a handshake: it returns when the child publishes its PID, and fails with a distinct message if the operation settles first. It has no clock of its own. Both cancellation tests give their probe `CANCELLED_CHILD_PROBE_DEADLINE_MS` (60 s, the largest deadline the product admits), so only the abort ends it. - The timeout test pins `Date.now()` (`setSystemTime`) while it acquires. The probe then arms its whole `EXPIRING_PROBE_DEADLINE_MS` (150 ms) as a real timer after the spawn, so there is always a child to reap. The clock is restored in `finally`, and the reaping assertions are unchanged. `tests/native-compose-adoption-generation.test.ts` - Named budgets replace the literals: - `ADOPTION_CASE_TIMEOUT_MS`: 120 s, for saved-adoption and CLI cases (~4× the measured worst). - `RETAINED_OPERATION_DEADLINE_MS`: the public command's 90 s deadline, replacing the 16 `Date.now() + 20_000` caller deadlines. - `retainedOperationCaseTimeout(n)`: adoption budget plus one deadline per operation, so a slow host ends in the product's own refusal before Bun's timeout. - Expiry cases name `EXPIRING_OPERATION_DEADLINE_MS` (15 s, was 5 s, so the pre-callback probes fit) and `DEADLINE_REFUSAL_SLACK_MS` (5 s, was 1.5 s). The cancelled-effect bound is named `CANCELLED_EFFECT_REFUSAL_BOUND_MS` (10 s, was 4 s). It stays far below the 60 s fake child and the deadline, so only cancellation can satisfy it. - Hanging fake children publish PIDs by rename (the HACK-1452 empty-file race, now fixed in this file too). The cancelled-effect start-up wait is a handshake instead of a 4 s poll. ## Verification - Linux host, Bun 1.4.2, synthetic CPU load (50 busy loops on 20 cores, plus ambient load): both files 5 consecutive times. In progress; this line will be updated with the results. - Biome and `tsc --noEmit`: clean for both files. - No macOS-specific behavior is involved. These fakes run on any host, so no M5 check is needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- codesmith:footer --> --- <a href="https://backend.blacksmith.sh/track/run-proof?expires=1796248950&installation_model_id=17053&pr_number=551&ref=codesmith_pr_footer&repository=hack-dance%2Fhack&return_to=https%3A%2F%2Fgithub.com%2Fhack-dance%2Fhack%2Fpull%2F551&variant=see_it_in_action&signature=20d7d93b1d1cb2a697dcd7b47f9ceadf1b220dc343449bab4fe68ebe8212e328"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-light.svg"><img alt="See it in action" src="https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-dark-3x.png" width="132" height="28"></picture></a> <sup><a href="https://app.blacksmith.sh/hack-dance/codesmith/hack/pr/551?autoLogin=true&ref=codesmith_pr_footer">Open in [code]smith</a> · Need help on this PR? Tag <code>@codesmith-bot</code> with what you need.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> Co-authored-by: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
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
createNativeComposeProbe(src/lib/native-compose-ownership.ts) awaitschild.exitedon every path before it rethrows: the success path'sPromise.alland thefinallyblock'sPromise.allSettled. Bun resolvesexitedonly after it reaps the child. A local check found 0 zombies in 400 SIGKILLed detached children, measured right afterawait child.exited, at host load average 40–180. The binding acquisition andassertFreshcall 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 astartedfile that the fake Docker child creates and then writes. A child killed between those two steps leaves the file empty.Number("")is0, andprocess.kill(0, 0)signals the test's own process group and succeeds. I reproduced this exactly: with the old test, a fake that leavesstartedempty fails withReceived function did not throw / Received value: true.E_LEGACY_COMPOSE_BINDING_PROBEunder load.tryLegacyComposeAdoptedCommand's refusal or cancel throw site, so changing either message passed.Fix
rename, so the file is never seen empty. A named, boundedstartedChild()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 thatkill(pid, 0)throws. The assertion was not relaxed; it now holds at once and deterministically.finally, then restoresBun.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 namedFIXTURE_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_MSis scaled to keep its existing rule: a slow host ends in the product's closed refusal before Bun's timeout.ORDERED_OPERATION_DEADLINE_MSis now 4 fixture probe deadlines. Both exhausted-deadline cases ("moving snapshot guard…" and "a persistent endpoint placeholder…") now useEXHAUSTED_PROBE_DEADLINE_MS(15 s) plusEXHAUSTED_REFUSAL_SLACK_MS(5 s). They still require more than two bridge scans and a refusal soon after the deadline.tests/native-compose-adoption-command-messages.test.tsdrives the adopted command's refusal site (loadActivethrows 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 assertsrefusal=adopted-refusedorrefusal=adopted-cancelled, with no canary in the output. The store and the marker selection are module spies, as intests/native-compose-adoption-publication-diagnostics.test.ts.Evidence
Each new test was proved by mutation:
Promise.racewith the abort,exiteddropped fromfinally): both cancellation tests fail atexpect(exited).toBe(true).startedempty: fails withReceived value: true, the symptom from the issue.finallyabort and await in place, only the forced test fails. With them removed,# Unhandled error between testsappears.other. Refusal site given a different literal: the refusal test fails with tokenother.Load runs at
db1037dc: the whole binding file ran 10 times (2 parallel lanes × 5 runs) with 6 addedyesCPU 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 at391af280failed 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: thetestjob (4/4 shards) and every other check.Gates:
bun run checkpassed (34 existing warnings, none in changed files).bun run --cwd packages/cli typecheckpassed with 0 errors; it includestests/. gitleaks 8.30.1 (gitleaks diron thegit log -p origin/next..HEADexport) found no leaks. The CLI surface and agent guidance did not change.Gaps
exitedsemantics, checked by experiment on macOS. A Bun change that resolvedexitedbefore reaping would failexpectReaped, 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_MSwas 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
assertFreshclosure in a scratch copy. One ordered start makes exactly 20, which matches the V7 job executor (executeLegacyComposeRetainedJobs): 1 on entry;db3; theseedjob 7 (its first completion poll still sees the job running);web5; 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_MSis 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.freshnessChecks <= ORDERED_START_FRESHNESS_CHECKS. If the executor ever makes more checks, the case fails at any load, not only on a slow host.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.FAKE_DOCKER_TEST_TIMEOUT_MSwas false. I countedcreateNativeComposeProbecalls 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.Evidence:
ORDERED_START_FRESHNESS_CHECKS = 19, the case fails withExpected: <= 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.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 addedyesworkers 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.07e1597f(run 40141039934) succeeded: thetestjob (4/4 shards),docker-e2e(4/4), and every other check.bun run checkpassed, includingprivacy:check, with Biome clean on the changed file.bun run --cwd packages/cli typecheckpassed with 0 errors. gitleaks 8.30.1 (gitleaks diron thegit log -p origin/next..HEADexport) found no leaks. Thecommand-messagestest passes 3/3. The CLI surface and agent guidance did not change.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_MSis 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