Repository navigation
test(native): import adopted refusal messages and size binding test budgets for load (HACK-1432) - #487
Merged
Conversation
…udgets for load (HACK-1432) - Export the adopted command's fixed refused/cancelled messages as LEGACY_COMPOSE_ADOPTED_COMMAND_MESSAGES and import them in the completed-job diagnostics scenario instead of hand-copied literals; add one test per token (adopted-refused, adopted-cancelled). - Give the placeholder re-observation acquisitions an explicit, named probe deadline (PLACEHOLDER_RESCAN_PROBE_TIMEOUT_MS) and a test budget derived from their probe deadlines. - Cover the duplicate-key check in bridgeMembers() for a duplicate container ID and a duplicate endpoint placeholder. - Set a named file-wide per-test budget (FAKE_DOCKER_TEST_TIMEOUT_MS) sized from the product's probe deadline instead of Bun's 5 s default and ad hoc 10-30 s overrides. Refs HACK-1432 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…obe deadline (HACK-1432) Under host load the clean baseline acquisition in both placeholder re-observation tests refused at the 15 s default probe deadline before the placeholder acquisition ran. Pass PLACEHOLDER_RESCAN_PROBE_TIMEOUT_MS to both acquisitions and derive the test budget from four such probes. Refs HACK-1432 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
roodboi
marked this pull request as ready for review
October 24, 2026 18:15
This was referenced Oct 24, 2026
Merged
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
…not calendar boot time (#486) ## Root cause On macOS, `node::ownership::inspect` built the job identity's `boot` from `sysctl kern.boottime`. That value is calendar time. XNU moves it whenever the wall clock is stepped, and `timed` steps the clock with `settimeofday` many times a day on this host (today's log shows steps of −235 ms, −71 ms and +103 ms). If a step landed between the identity read at spawn and the read before the timeout signal, `verify()` saw a different `boot`. The supervisor then quarantined its own normal timeout with "Owned child boot/start/UID/group identity mismatch; no signal sent." (HACK-1429). Observed on this host without touching the clock: a passive sampler watched one live `sleep` across the 11:00:34 `timed` step of +0.1027 s. - `kern.boottime` moved from `…:330343` to `…:433042`. - The process's `pbi_start` (`…960113930`) and `kern.bootsessionuuid` did not change. So the old identity would have mismatched, and the new identity does not. The process start (`pbi_start_tvsec/usec`) also comes from calendar time, but XNU snapshots it once at fork (`p_start`) and never rewrites it. A step does not change a live process's start (the sampler confirms this), and only a new process, such as one on a reused PID, gets a new value. The start therefore stays in the identity unchanged, and PID-reuse protection is unchanged. ## Fix - On macOS, `boot` is now `kern.bootsessionuuid`, normalized to a canonical lowercase UUID. This is the Darwin equivalent of Linux `boot_id`, and the same source that `provider::graph::native::foreground::host_boot` already uses. If the session is unavailable or malformed, the read is refused with "Host boot identity unavailable.". There is no fallback to calendar time. The read goes through a small `host::sysctl` seam that tests can override per thread. - Linux is unchanged in substance. `boot_id` must now be a canonical UUID instead of only non- empty. `starttime` is already measured in clock ticks since boot. - **Old receipts:** `verify` first refuses any recorded `boot` that is not a canonical UUID: "Owned child identity uses a retired boot format; no signal sent.". - A Darwin receipt written before this change (`seconds:micros`) still deserializes, because the struct shape is unchanged, so journals stay readable. - Such a receipt never authorizes a signal, not even against an observer that still reports the old format. - Nothing migrates it, because nothing needs to. A supervisor that was in flight during an upgrade keeps running its own old binary with its in-memory identity. A new node never signals a recorded PID: it leaves a job whose lock is still held, and quarantines one whose supervisor is gone. A new test covers both cases. - **Second commit: a late stop signal now gets its settling window.** This was found while gathering the 20× load evidence. The first loop had 1 failure in 20 with "Child termination deadline exceeded.", quarantined. - Temporary instrumentation (not committed) showed SIGTERM at 1.5 s and the first SIGKILL at 4.4 s: one supervisor pass stalled for about 2.8 s. The deadline check ran in the same pass, nanoseconds after that SIGKILL. - Change: the deadline (timeout + 2 s from start) now also requires 2 s since the owner's first SIGTERM or SIGKILL. Each anchor is set once, so termination stays bounded. - A child that is never signalled, for example one that reads as Gone forever, keeps the original deadline. - **Third commit (test only):** `an_upgraded_node_never_signals_a_pre_upgrade_in_flight_job` failed 2 of 30 times under load. A sibling test thread's fork briefly shared the dropped job- lock file until its exec closed the CLOEXEC descriptor. The test now retries `schedule` for a bounded time and still requires quarantine and an unsignalled child. ## Tests (each mutation applied alone, then `cargo +1.97.1 test --lib node::`; all 12 caught at the final head) | # | Mutation | Caught by | |---|---|---| | M1 | boot taken from `kern.boottime` again (UUID- shaped, through the seam) | `a_wall_clock_step_keeps_the_owned_identity_and_a_new_boot_session_refuses` | | M2 | malformed session falls back to a constant | same | | M3 | retired `seconds:micros` boot accepted by `verify` | `an_identity_recorded_in_the_retired_calendar_boot_format_never_authorizes_a_signal` | | M4 | journal alias check reverted to check-then-open (`symlink_metadata` then `private_file`) | `a_rollback_journal_deleted_between_check_and_open_reads_as_absent` | | M5 | `run_child` timeout branch: `Exiting => break` | `run_child_defers_a_gone_live_child_at_timeout_and_keeps_its_own_exit`, `run_child_bounds_an_endless_gone_window_by_its_deadline_without_signalling` | | M6 | timeout branch: `Exiting` sends SIGTERM and records a stop reason | `run_child_defers_a_gone_live_child_at_timeout_and_keeps_its_own_exit` | | M7 | kill branch: `Exiting => break` | `run_child_never_kills_during_a_gone_window_and_keeps_its_stop_reason`, `a_stop_that_never_settles_is_still_bounded_by_its_grace` | | M8 | `Gone` on a live child authorizes a signal | 5 supervisor tests | | M9 | `Gone` on a live child quarantines (the HACK-1418 bug) | 5 supervisor tests | | M10 | recovery signals a recorded child when it quarantines | `an_upgraded_node_never_signals_a_pre_upgrade_in_flight_job` | | M11 | deadline ignores a fresh SIGTERM/SIGKILL (the previous rule) | `a_kill_check_delayed_past_the_deadline_still_settles_as_the_owners_timeout` (plus 2 more) | | M12 | stop grace unbounded (`settling.is_none()`) | `a_stop_that_never_settles_is_still_bounded_by_its_grace` | What each item is tested with: - **(1) Clock step:** a thread-local sysctl override moves `kern.boottime` to simulate `timed` steps of −73 ms, +235 ms and −5 s. The host clock is never touched. - The identity stays equal and `verify` passes. - A different boot session is refused, and six unavailable or malformed session values fail closed. - A last read without the override checks that the real session is canonical and stable. - **(2) #474 journal race:** a one-shot `before_open` hook in `open_private_file` deletes `journal.sqlite-journal` just before the open, the way a committing SQLite writer would. `Store::open` must succeed. The same hook swapping in a symlink at that point must still be refused by `O_NOFOLLOW`. - **(3) `Authority::Exiting` wiring:** `run_child` now takes its `inspect` function as a parameter, and production passes `ownership::inspect`. The tests drive a real TERM-ignoring group leader with injected answers: - `Gone` at the timeout sends nothing, and the child's own exit code 3 is kept. - `Gone` in the kill window sends no SIGKILL, keeps `timed_out`, and observes again. - An endless `Gone` reaches the deadline with the child still alive and never signalled. - A kill check delayed past the deadline still ends as `timed_out`. - A stop that never settles is still bounded. - **(4) Contract test:** `restart_does_not_own_or_duplicate_supervisor` now checks `state == "running"` on the same receipt that first shows `child_pid`. A second read can no longer land after the 1.2 s timeout. The assertion is unchanged. ## Evidence Run on macOS with cargo 1.97.1, private HOME, HACK_HOME, DOCKER_CONFIG and DOCKER_HOST, and `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`. The host was already at load average 40–70 from other work. - At the final head (`c98e3004`): - `bun run check:local` (fmt and clippy `-D warnings`, all targets) passed. - `bun run test:local` passed: lib 1063, main 43, `node_contract` 20, and every other test binary. - Root `bun run check` (includes `privacy:check`) and `bun run --cwd packages/cli typecheck` (0 errors) passed. - gitleaks 8.30.1 (`gitleaks dir` on the `git log -p origin/next..HEAD` export) found no leaks. - `node::` lib tests, 30 loops with 12 `yes` load processes: 30/30 passed after the third commit. Before it, 2 failures in 30, both in the recovery test, as described above. - Contract test `restart_does_not_own_or_duplicate_supervisor` with 12 `yes` load processes, stopped by an EXIT trap: | Head | Result | Failure | |---|---|---| | first commit | 19/20 | 1 deadline quarantine | | first commit, instrumented | 28/29 | 1 deadline quarantine (diagnosed above) | | final head, run 1 | **20/20** | none | | final head, run 2 | 19/20 | 1 quarantine with "Supervisor failure: database is locked" | The last failure is a different, pre-existing failure mode, covered under Gaps. No identity- mismatch quarantine was seen in any loop. - `~/Library/Logs/DiagnosticReports`: - One new `hack_runtime_core-5860c71c28018015-…-111458.ips` (SIGABRT in `provider::relay_owner ::lifecycle_intent::tests::empty_graph_requires_fresh_native_proof_and_preserves_unrelated_flo w`). It is not from this work. That process started at 11:10:58 running the whole lib suite. This work only ran `node::`-filtered lib tests then, and its `test:local` lib binary was built at 11:17:43. Cargo gives the same binary name across worktrees, so it is most likely another session on this shared host. - The other new files are system `SFA-*.diag` files. - No CLI surface or consumer agent guidance changed, so no regeneration was needed. ## Gaps - **Journal busy timeout under heavy load (pre-existing, not fixed here):** in 1 of the last 20 loaded runs, the supervisor quarantined with "database is locked". Its per-pass `store.get`/`store.update` waited longer than the 2 s SQLite busy timeout while the restarted node held the journal. The current rule is that every journal failure in `run_child` quarantines, and the outcome is fail-safe. Changing that rule (retry versus a longer timeout) is a separate decision. It is reported as a finding. - The Linux branch is checked only by hosted CI (Candidate core ubuntu). The macOS start-time analysis relies on XNU source behaviour and one observed natural clock step, not on a forced step. - A mutant that resets the SIGKILL anchor on every repeated SIGKILL cannot be observed: a real child always dies on SIGKILL, and then `waitid` breaks the loop. - `node` is part of the unsupported remote/node surface. It is fixed here only because its contract test runs in the required Candidate core job. - Hosted CI at `c98e3004` (run 40109306158) is all green: - Candidate core macOS (14m57s) and ubuntu (7m29s) - `test` shards 1–4, which make up the full suite - docker-e2e shards 1–4 - the config-compiler matrix, runtime state models, linux-process-lifetime, runtime-images and secret-scan One hosted pass does not measure the flake rate. ## Rework round: deterministic `run_child` fixture The verifier found that the new `run_child` tests were not deterministic. `Job::new` spawned `/bin/sh -c "trap '' TERM; read line; exit 3"` and returned at once. Under CPU load, the 50 ms timeout could send SIGTERM before the shell had run `trap`, so the default action killed it. Three tests then failed: `run_child_never_kills_during_a_gone_window_and_keeps_its_stop_reason`, `a_kill_check_delayed_past_the_deadline_still_settles_as_the_owners_timeout` and `a_stop_that_never_settles_is_still_bounded_by_its_grace` (2 of 40 loaded runs). The first and third are the tests that catch M7 (kill branch `Exiting => break`), so that wiring was covered only by chance. **Fix (test fixture only, `supervisor.rs`):** the shell now runs `trap '' TERM; echo ready; read line; exit 3`. `Job::new` reads exactly the 6-byte `ready\n` line from the child's stdout before it returns. `echo` runs only after `trap` is installed, so every test starts with SIGTERM already ignored. `run_child` still drains the rest of stdout. No production code changed in this round. **Evidence (macOS, cargo 1.97.1):** | Check | Result | |---|---| | Fixture with `/bin/sleep 0.3;` before `trap`, handshake kept | 7 of 7 supervisor tests pass | | Same delay, handshake removed (the old fixture) | the same 3 tests fail with the verifier's messages (`Some(15)` vs `Some(9)`, and so on) | | M7 (kill branch `Exiting => break`) with the 0.3 s delay and the handshake | caught by `run_child_never_kills_during_a_gone_window_and_keeps_its_stop_reason` and `a_stop_that_never_settles_is_still_bounded_by_its_grace` | | `--lib node::supervisor` under 18 busy loops (load average 70–109) | 40 of 40 | | `--lib node::` under the same load | 40 of 40 | | `node_contract` (whole binary) under the same load (load average 87–163) | 20 of 20 | | `restart_does_not_own_or_duplicate_supervisor` alone under load | 20 of 20 | | `fmt --check`; `clippy --all-targets -D warnings` | exit 0 | | full `cargo test` (the `test:local` equivalent) | all binaries ok; lib 1063 passed, 101 ignored | | root `bun run check` (includes `privacy:check`); `packages/cli` typecheck | exit 0; 0 errors | | gitleaks 8.30.1 `dir` over `git log -p origin/next..HEAD` | no leaks | The delay test proves the handshake closes the race without depending on timing: a 0.3 s delay is much longer than any delay seen under load, and the tests still pass. `origin/next` was merged in twice (up to `d09e1b73`). Next changed only TS, docs and CI files. None of them overlap this PR's files. **New gap seen this round:** a second full `cargo test` run, made while other sessions held the host's load average at 100–130, failed several `provider::lifecycle` isolated tests with `provider_timeout_uncertain`. One `--lib provider::` rerun under the same load failed the same way. The first full run passed. This PR does not touch `provider` code. These tests are timing sensitive under heavy host load, and this is reported as a separate finding. Hosted CI at `f0e5b27b` (run 40120129817) is all green, 20 of 20 checks: Candidate core macOS (14m20s) and ubuntu (7m45s), `test` shards 1–4 plus the `test` aggregate (the full suite), docker-e2e shards 1–4, the config-compiler matrix, runtime state models, linux-process-lifetime, runtime-images and secret-scan. One hosted pass does not measure the flake rate. After that run, next gained `78487f72` (#487, TS tests only). The branch is BEHIND again but `git merge-tree` against it is clean. Refs HACK-1429 🤖 Generated with [Claude Code](https://claude.com/claude-code)
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
…d skip comment lines in jobs (#489) Refs HACK-1441 Follow-ups from the #476 verifier on `scripts/ci-bun-crash-census.ts`. **Stacked on #476:** #476 is still open, so this branch merges its head (`e3814e87`) on top of `next` (`1281184b`). Until #476 lands, this PR's diff also shows #476's changes. Only commit `159df1e0` belongs to this PR. ## Root cause 1. **Cache key had no test.** If you change ``const cacheKey = `${sha}\n${job}` `` to `const cacheKey = sha`, all 16 existing tests still pass. With a wildcard selector, a sha-only key gives every job at that sha the version of whichever job resolves first. For example, the `test` fan-in (no Bun) and a shard (pinned Bun) would share one result. 2. **Column-0 comments ended the job scan.** `workflowJobBunVersion` stopped at the first non-indented line after `jobs:`. A column-0 `# comment` inside `jobs:` therefore hid every later job, and the function returned null for those jobs. 3. **One fetch per job name.** The only cache was keyed by (sha, job), so the same `ci.yml` was fetched once for each distinct job name at a commit. That is up to 4 GETs per sha for the shards, plus 1 for the fan-in. ## Fix - Add a per-sha cache of the decoded workflow text. Each sha now costs one contents GET, whatever the number of jobs, runs or concurrent workers. The cache stores the promise, so concurrent runs at the same sha share the in-flight request. - Keep the per-(sha, job) version memo on top of that cache. A comment explains why the key must never be the sha alone. - `workflowJobBunVersion` now skips blank lines and lines that start with `#` in column 0. Any other column-0 line is still the next top-level key and still ends the jobs. - The header doc now says the workflow is fetched once per commit. ## Tests (`tests/ci-bun-crash-census.test.ts`, new `workflow reads (HACK-1441)` block) - **Comment and blank lines:** a workflow with a column-0 comment, blank lines, a commented-out job and a top-level `env:` key after `jobs:`. Shard, lint, fan-in, commented-out-job and post-`jobs:` lookups each return the right value. - **Per-job version under `--job "test*"`:** three runs, two of them at the same sha. In one run the fan-in is listed first; in another a shard is listed first. Every shard gets `1.4.2 workflow`, the fan-in gets `unknown`, and the expected warning is `no_bun_setup 2/9`. - **Fetch count:** the same fixture at concurrency 1 and 3 makes exactly one contents GET per sha (2 in total). ## Evidence - Focused tests: `bun test tests/ci-bun-crash-census.test.ts` gives 19 pass, 0 fail (16 existing + 3 new), on Bun 1.4.2. - Mutation results. Each mutation was applied to the fixed script, tested, and then the file was restored: | Mutation | Result | |---|---| | M1: `cacheKey = sha` | killed by the per-job version test | | M2a: column-0 comment ends the scan again | killed by the comment test and the version test | | M2b: full revert to break-only behavior | killed | | M2c: skip every column-0 line (never end `jobs:`) | killed by the comment test (`late` lookup) | | M3: no per-sha text cache (the pre-fix behavior) | killed by the fetch-count test | - Gates: root `bun run check` passes (bun-version, privacy:check, turbo check; existing warnings only). `bun run --cwd packages/cli typecheck` has 0 errors and covers `tests/`, which imports the script. Biome is clean on both changed files. gitleaks 8.30.1 (`gitleaks dir` over a `git log -p origin/next..HEAD` export) found no leaks. - No CLI surface or consumer agent guidance changed, so I did not run `docs:cli-reference` or `generate:agent-plugins`. - Full suite: hosted CI `test` shards at this PR's exact head. ## Gaps - No live `gh` census was run against GitHub. The behavior is proven with the fake `GhReader` only. - If `#476` is squash-merged before this PR, `next` has to be merged into this branch again. Expect a possible conflict at the end of the test file, because both sides add lines after the same last block. ## Round 2: merge `next` after #476 landed #476 was squash-merged into `next` as `101d2638`. This branch still carried #476 as the merge commit `0482673b`, so the PR conflicted with `next` (`d09e1b73`) in `scripts/ci-bun-crash-census.ts` and `tests/ci-bun-crash-census.test.ts`. - **Fix:** merge commit `499e8019` merges `next` (`d09e1b73`) into the branch and keeps the PR side of both census files. Before resolving, I checked that #476's five files are byte- identical between `0482673b` and `d09e1b73` (`git diff` is empty), so the PR side differs from `next` only by the HACK-1441 changes. - **Diff now:** `git diff next HEAD` is only the two HACK-1441 files (+177/-20). The "Stacked on #476" note above no longer applies, because #476's changes are already in `next`. - **Focused tests at `499e8019`:** 19 pass, 0 fail, 87 expects (Bun 1.4.2). - **Mutations re-run at `499e8019`:** each was applied, tested and then restored. M1 (`cacheKey = sha`) was killed by the per-job version test. M2 (skip block removed) was killed by the comment test and the per-job test. M3 (per-sha text cache never hits) was killed by the fetch- count test. - **Gates at `499e8019`:** root `bun run check` exits 0 (privacy-check ok). `bun run --cwd packages/cli typecheck` exits 0 with 0 errors. Biome is clean on both files. gitleaks 8.30.1 (`gitleaks dir` over a `git log -p origin/next..HEAD` export plus the net diff) found no leaks. - **Hosted CI at exactly `499e8019`:** run 40120172542 passed all 20 checks, including `test` shards 1/4 to 4/4, `docker-e2e` shards 1/4 to 4/4, secret-scan, Candidate core, Config compiler (all platforms), Runtime state models, linux-process-lifetime and runtime-images. This replaces the earlier evidence from `159df1e0`. - **Remaining:** after CI started, `next` gained one more commit, `78487f72` (#487). `git merge- tree` reports a clean merge, and that commit touches no census files. GitHub shows `BEHIND`, not a conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
roodboi
added a commit
that referenced
this pull request
Oct 25, 2026
…ding tests for load (HACK-1452) (#495) 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](https://claude.com/claude-code)
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 from #477 (HACK-1421). Test and diagnostic hardening only: the adopted command's behavior and messages do not change.
Root cause
tests/e2e/scenarios/native-compose-adoption-job-diagnostics.tsmaps the adopted command's fixed messages to closed tokens by exact match, but theadopted-refusedandadopted-cancelledmessages were hand-copied literals. A wording change insrc/lib/native-compose-adoption-command.tswould silently turn either token intoother. No test coveredadopted-cancelled.createNativeComposeProbe). Under heavy load the probe refused withE_LEGACY_COMPOSE_BINDING_PROBE.bridgeMembers()a duplicate.Containerskey, so thenew Set(value).size === value.lengthcheck could be removed without any test failing.tests/native-compose-adoption-binding.test.tsran with Bun's 5 s default, and some had ad hoc 10-30 s overrides. Every fake Docker query is a cold Bun process, so under load those budgets ran out while the host was still starting processes, before the product logic finished. The 30 s snapshot case is worse: when it times out it leavessnapshotCaseUnconfirmedset, and every later test in the file then fails inbeforeEach.Fix
src/lib/native-compose-adoption-command.tsexportsLEGACY_COMPOSE_ADOPTED_COMMAND_MESSAGES(refused,cancelled). Both throw sites use it, and the scenario's token table imports it.tests/native-compose-adoption-job-diagnostics.test.ts: one test per token (adopted-refused,adopted-cancelled) replaces the copied literal.tests/native-compose-adoption-binding.test.ts:PROBE_DEADLINE_MS = 15_000records the product's default probe deadline.FAKE_DOCKER_TEST_TIMEOUT_MS = 8 * PROBE_DEADLINE_MSis set once withsetDefaultTimeout, replacing the 5 s default and the 10/20/30 s overrides. On a slow host, a test should end with the product's own closed refusal, not a Bun timeout.timeoutMs: PLACEHOLDER_RESCAN_PROBE_TIMEOUT_MS(3 x default) to the clean baseline acquisition and to the placeholder acquisition. Under load, the clean baseline also refused at the 15 s default. Their test budget,PLACEHOLDER_RESCAN_TEST_TIMEOUT_MS, is the sum of their four probe deadlines.test.each: a duplicate container ID and a duplicate endpoint placeholder each refuse withE_LEGACY_COMPOSE_BINDING_IDENTITYon the first bridge read, with no retry (exactly one network inspect).Tests and mutation proof
Mutation runs are scripted. Each one applies an exact-text change, runs the focused tests, and restores the file.
adopted-cancelledentryadopted-cancelledreverts to a drifted literal (canceled)adopted-refusedreverts to a drifted literal (kept)bridgeMembers()duplicate check replaced bytrue(M6)Focused runs:
tests/native-compose-adoption-job-diagnostics.test.ts: 10 pass, 0 fail.Load comparison (item 4)
This host has 18 cores and is shared with other agents. I ran
tests/native-compose-adoption-binding.test.tsfrom an extractedorigin/nexttree (1281184) and from this branch at the same time, so both saw the same load. Each run used a private HOME, HACK_HOME and DOCKER_CONFIG, with keychain fallback disabled.On next, a few tests hit Bun's 5 s timeout, and the 30 s snapshot case times out. That leaves
snapshotCaseUnconfirmedset, so every later test fails inbeforeEach. On this branch there are no Bun timeouts and no cascade. The failures that remain under this load are the product's own closed refusals, or timing assertions that are already in the file:E_LEGACY_COMPOSE_BINDING_PROBE, because one acquisition took longer than the product's 15 s default probe deadline. Those tests do not name a deadline.Gates
gitleaks dirover agit log -p origin/next..HEADexport): no leaks.bun run check(includesprivacy:check): passes. The 34 warnings are existing complexity warnings in untouched files.bun run --cwd packages/cli typecheck: 0 errors.HACK_E2E_DOCKERunset). The scenario module change is covered by the unit test that imports it, and hosted docker-e2e shards run it.Gaps
Findings (not fixed here)
E_LEGACY_COMPOSE_BINDING_PROBE. Possible follow-up: give those tests a named, wider probe deadline across the file.process.kill(pid, 0)did not throw) right after theE_LEGACY_COMPOSE_BINDING_CANCELLEDrefusal under load. Either cancellation can return before the child is reaped, or the assertion is racy. This needs investigation.Refs HACK-1432
🤖 Generated with Claude Code