Skip to content

fix(storage): bound storage helper probes by the carrier's remaining budget - #437

Merged
roodboi merged 5 commits into
nextfrom
wf/g53-storage-probe-budget
Oct 24, 2026
Merged

roodboi merged 5 commits into
nextfrom
wf/g53-storage-probe-budget

Conversation

@roodboi

@roodboi roodboi commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Root cause

Every Docker probe and command inside a native storage carrier had a fixed timeout in src/lib/native-compose-storage-witness-docker.ts: 15 s for reads, target observation, volume create and helper create/start, and 10 s for helper removal and cleanup reads. This held even though #375 gave each carrier a 90 s HACK_NATIVE_STORAGE_TIMEOUT_MS budget. On loaded hosted runners, a slow docker start -ai of the helper hit the 15 s cap. It refused as E_COMPOSE_FAILED with stage storage-helper-start and reason probe-timeout, while most of the carrier budget was still unused. That failed hack exec/up in the native-config-compose and native-config-networks e2e scenarios. For exec, this could happen after the guest command had already printed its result, because the failing step was the closing storage proof.

Fix

A new function, nativeComposeStorageProbeTimeoutMs({ kind, deadline }), now sets every probe and command timeout in the carrier:

  • command (volume create, helper create/start): gets the carrier's whole remaining budget. A hung command still times out exactly when the carrier's budget ends.
  • read (reads, target observation, saved-recovery reads): gets the remaining budget, capped at 60 s. 60 s is the most createNativeComposeProbe accepts.
  • cleanup (post-exit reads, helper rm): has its own budget, the remaining budget clamped to 10–60 s. It is never clipped to the carrier's deadline, so cleanup still runs after the carrier expires, and a hung engine still cannot stall cleanup forever.

Every timeout is finite and named. A timeout that ends before the carrier's deadline still reports probe-timeout. One that ends at the deadline reports storage-budget, as before. No check, retention or recovery rule changed. Diagnostics code (compose-startup-budget resolution, effect diagnostics) is untouched; the only change in compose-startup-budget.ts is one doc-comment sentence that described the old fixed cap (to stay out of the way of G50/HACK-1361 and G51/HACK-1365).

Exec decision (documented). The closing storage proof after a saved exec stays fail-closed. If it refuses (including a probe timeout), exec fails with that refusal even though the guest command already ran, and the command's exit code is not reported. Ownership and saved state are unchanged, and the next admission proves storage again. This is now spelled out in docs/reference/native-compose-commands.md and in the runNativeComposeStorageVerifiedExec doc comment.

Docs updated: docs/reference/native-compose-storage-xattr-carrier.md (budget layering), docs/cli.md (storage budget section), docs/reference/native-compose-commands.md (exec contract).

Tests

New file: tests/native-compose-storage-probe-budget.test.ts. It runs the real createNativeComposeDockerStorageXattrCarrier through a full readonly root proof (target, helper create, policy, start, response, cleanup, closing observation) against a stub engine:

  • Engine reads go through a spied createNativeComposeProbe that answers in-process.
  • Daemon commands go through a spied shell.run that rewrites them into local /bin/sh children, so the real capture, process-group and settlement paths still run.
  • Elapsed engine time is a virtual clock (Date.now spy). There is no real 15 s wait.
  • The tests use a private HOME, HACK_HOME and DOCKER_CONFIG, plus HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1. DOCKER_HOST=tcp://… and DOCKER_API_VERSION stop the engine-ID fence from ever choosing a direct Unix socket. A tripwire docker on PATH must never run.

What the tests cover:

  1. A helper start that takes 20 s, longer than the old 15 s cap, completes within the default 90 s budget. Create and start are granted more than 20 s and at most the budget; rm gets 10–60 s; every probe gets 1–60 s.
  2. A start that outlasts a 12 s budget still refuses at the budget's end, with storage-helper-start / storage-budget.
  3. Policy unit checks for the three timeout kinds, including the cleanup floor after the deadline.
  4. A closing storage proof refusal fails saved exec after its command ran, in the order opening, command, closing.

Mutation proofs:

  • Restoring timeoutMs: 15_000 on the helper start makes test 1 fail with exactly the CI symptom { stage: "storage-helper-start", reason: "probe-timeout" }.
  • Restoring all former fixed caps fails tests 1 and 3.
  • Removing the cleanup floor fails test 3.
  • Making runNativeComposeStorageVerifiedExec swallow the closing refusal fails test 4.

Evidence

  • Bun 1.4.2 (mise x bun@1.4.2). bun install --frozen-lockfile; no lockfile changes.
  • Local (this Mac): new tests 4/4 pass. Neighbouring suites pass: native-compose-storage-witness-docker, -command-record, -witness-docker-io, -witness-dependency, -witness-xattr-owner, native-compose-effect-diagnostics, compose-startup-budget, agent-instruction-source (295 tests total).
  • bun run --cwd packages/cli typecheck: 0 errors. Root bun run check (includes privacy:check): 0 errors (existing complexity warnings only).
  • gitleaks 8.30.1 over origin/next..HEAD: no leaks.
  • Full suite on the M3 at the pushed commit: NOT RUN. My job queued on the shared M3 CI lock for about 90 minutes behind another unit's multi-hour loaded benchmark (G52) and three other queued suites. I cancelled it before it took the lock, so no M3 worktree was created. The full suite at 4e37716 is still outstanding.

Gaps

  • The new test proves the storage-carrier step that both exec and up run, using a stub engine. It does not drive the hack exec/hack up CLI end to end: that needs a real Docker engine, and these tests may not use one. The hosted native-config-compose and native-config-networks e2e scenarios are still the end-to-end check, and only hosted CI on this PR shows whether those flakes are gone.
  • A hung non-cleanup read now waits up to 60 s, or the remaining budget, instead of 15 s before refusing. The carrier as a whole is still bounded by HACK_NATIVE_STORAGE_TIMEOUT_MS.
  • The compose-startup-budget.ts doc-comment edit may textually conflict with G50/G51 if they edit the same lines. The conflict would be trivial.

Round 2: recorded exec/up path (review follow-up)

The verifier found that the tests above missed the path exec and up actually take. prepareNativeComposeCommandStorage creates every carrier with originalCommandRecords: true, so their readonly verify proofs run each helper command through the recorded original-command branch of runNativeComposeStorageDockerCommand (publication handshake, boundaryDeadline, settlement). The first test used an unrecorded root request, so capping only the recorded child at 15 s (Math.min(15_000, boundaryDeadline - performance.now())) left every test green. The code fix already reached this branch, because both branches use the same computed timeout. But no test proved it.

Changes in this round (test and docs only, no product code change):

  • Merged next (5208ee5) into the branch. The merge was clean.
  • New test, "a recorded exec/up verify proof completes a helper start slower than the former 15 s cap". It builds the carrier the way exec/up do (originalCommandRecords: true, default 90 s budget from resolveNativeStorageTimeoutMs) and sends a readonly verify request. The helper start takes 20 s of virtual time. The test checks that the proof completes, that create and start were each granted more than 20 s and at most the budget, and that the carrier's commands.json record holds create, start and remove, each with a published child and a clean settlement. That last check proves the recorded branch really ran. The test journals the carrier intent the way the witness owner does (initializeNativeComposeStorageCarrierJournal and beginNativeComposeStorageCarrierIntent). The journal's recordCreated is the invocation's callback. After the proof, the test calls journal.complete() and passes the completion to finish(), which retires the invocation files and closes their held directories. The test asserts that the invocation directory is gone.
  • The file's afterEach now calls Bun.gc(true). If this file ever leaks a held directory again, the leak is reported against this file instead of a later, unrelated one.

The first push of this round, 16782e3, failed hosted test (run 40058406790) with 1 fail and 3 errors out of 8256 tests. My recorded case called finish() with a synthetic completion. The finalizer consumes the completion before its try/finally block, so the refusal left the three held invocation directories (the instance, storage-carriers and invocation directories) open. Garbage collection later closed them during native-compose-inputs.test.ts and raised ERR_INVALID_STATE, which Bun counted against the test that was running ("omitted native compiler context preserves Compose routing probing and branch omission"). 8d3d1f3 fixes this with the real journal completion. With the Bun.gc guard in place, removing the finish() call makes this file fail with exactly those three FileHandle errors, so the guard catches the leak.

  • Test engine fix: the daemon-side state change of create, start and rm now happens when the child exits, not when the command is issued. A recorded command checks admission again after its child is spawned, while the helper must still be created. Each recorded command's host identity reads (/bin/ps, plus sysctl on macOS) pass through to the host. They are read-only.
  • docs/reference/native-compose-storage-xattr-carrier.md now says that recorded original commands (the verify proofs that exec and up run) also share the remaining-budget bound, together with their publication handshake and identity reads.

Not done: driving prepareNativeComposeCommandStorage end to end through ports.carrier with the real Docker carrier. That path also needs an enrolled witness and the carrier journal. The new case uses the same carrier constructor and options that prepareNativeComposeCommandStorage passes, and the same request kind.

Mutation results on the new test file (run at 16782e3; the first two were run again at 8d3d1f3 with the same result):

  • Recorded child capped at 15 s (the verifier's surviving mutation): 1 test fails (the new recorded test). Before this round it survived.
  • Helper start call site restored to Math.min(15_000, remaining): 3 tests fail.
  • Command branch capped at 15 s (Math.min(15_000, opts.timeoutMs, …)): 3 tests fail.
  • Still surviving, as the verifier reported: volume-create, read and target-observation call sites restored to 15 s/10 s, and rm fixed at 10 s. These steps are not slow in the CI failure, and no test pins their exact bound.

Evidence for this round:

  • Bun 1.4.2, bun install --frozen-lockfile, with no bun.lock change.
  • New test file at 8d3d1f3: 6/6 pass in 3 consecutive runs (20–39 s each, while the host's load average was about 30). Locally the recorded case takes about 8 s, against its 30 s timeout; in hosted CI it took 0.7 s. At 16782e3 the file also passed once while the shared host's load average was about 250.
  • Focused set at 8d3d1f3: 167/167 pass across probe-budget, witness-docker, witness-docker-io, command-record, carrier-journal-completion, effect-diagnostics and compose-startup-budget.
  • bun run --cwd packages/cli typecheck: 0 errors.
  • Root bun run check: exit 0, privacy-check: ok, the same 34 existing warnings. Biome on the changed test file: clean.
  • gitleaks 8.30.1 dir on the git log -p origin/next..HEAD export: no leaks.
  • Safety: the test sets a private HOME, HACK_HOME and DOCKER_CONFIG, HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1 and DOCKER_HOST=tcp://127.0.0.1:9. The tripwire docker never ran.
  • Local caveat: native-compose-storage-witness-xattr-owner.test.ts had several of its 5 s-default tests time out locally while the shared host's load average was 75–280. One example is "command storage carriers use the unset named storage budget". Run alone with --timeout 120000, that test passed in 16 s, so it is slow on this host, not hung. That file uses a fake carrier transport and never reaches the Docker carrier this PR changes, and the file itself is unchanged from next. It passed in hosted CI at d15116e. Hosted CI at this head is the full-suite evidence.

Hosted CI at 8d3d1f3 (pull_request run 40062046367): every check passed. See the next section.

Round 3: independent recheck at 8d3d1f3

No code change in this round. The PR head is still 8d3d1f3, which already contains next (5208ee5). This round rechecked the Round 2 fix from a fresh worktree:

  • The new test file passes 6/6 (Bun 1.4.2, bun install --frozen-lockfile, private HACK_HOME and DOCKER_CONFIG, HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1, DOCKER_HOST=tcp://127.0.0.1:9).
  • The verifier's surviving mutation is now killed. With the recorded child capped at 15 s (timeoutMs: record ? Math.min(15_000, boundaryDeadline - performance.now()) : timeoutMs), 1 test fails: "a recorded exec/up verify proof completes a helper start slower than the former 15 s cap". After the source was restored, the tree was clean.
  • bun run --cwd packages/cli typecheck: 0 errors. Root bun run check: exit 0, privacy-check: ok, the same 34 existing warnings. gitleaks 8.30.1 dir on the git log -p next..HEAD export: no leaks.
  • Hosted CI at 8d3d1f3 (run 40062046367): every check passed. This includes the test job (38m12s), docker-e2e shards 1–4, Candidate core on macOS and Ubuntu, Config compiler ×4, runtime state models, linux-process-lifetime, runtime-images and secret-scan. In the e2e shards, native-config-networks passed (1901.4s) and native-config-compose passed (514.9s).

The remaining gaps are unchanged from Round 2. No test pins the exact bound of the volume-create, read, target-observation and rm call sites. Nothing drives prepareNativeComposeCommandStorage end to end with the real Docker carrier.

Refs HACK-1369

🤖 Generated with Claude Code

…budget

Native storage helper probes kept fixed 15 s (reads, target observation,
helper create/start) and 10 s (cleanup) caps although each carrier has a
90 s HACK_NATIVE_STORAGE_TIMEOUT_MS budget. On loaded hosted runners a slow
`docker start -ai` of the helper refused at 15 s as storage-helper-start
probe-timeout and failed `hack exec`/`up`.

Each probe and command now takes its timeout from
nativeComposeStorageProbeTimeoutMs: daemon commands get the remaining carrier
budget, reads the remaining budget up to the probe's 60 s ceiling, and
cleanup keeps its own 10-60 s bound that is never clipped to the deadline.
A timeout before the deadline still reports probe-timeout; one at the
deadline reports storage-budget.

Document the fail-closed saved exec contract: a closing storage proof
refusal fails exec after its command ran.

Refs HACK-1369

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

linear-code Bot commented Oct 23, 2026

Copy link
Copy Markdown

HACK-1369

roodboi and others added 3 commits October 23, 2026 12:03
… helper start

The probe-budget test drove only the unrecorded root proof, so a 15 s cap on
the recorded original-command child (the path exec and up storage proofs take)
survived. Add a case that builds the carrier with originalCommandRecords and
sends a readonly verify request with a 20 s virtual helper start, asserts every
helper command was published and settled as an original command, and apply the
helper's daemon-side state change when its child exits so recorded admission
re-checks see the created helper. Document that recorded commands share the
remaining-budget bound.

Refs HACK-1369

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@blacksmith-sh

This comment has been minimized.

…rnal completion

The recorded exec/up case called finish() with a synthetic completion. The
finalizer consumes the completion before its try/finally, so the refusal left
the held invocation directories open; hosted CI's full run then garbage
collected them and failed the file with ERR_INVALID_STATE. Journal the intent
like the witness owner does, complete it, and let finish() retire the
invocation files. Collect garbage after each test so any future leak is
reported against this file.

Refs HACK-1369

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@roodboi
roodboi marked this pull request as ready for review October 24, 2026 21:07
@roodboi
roodboi merged commit f24c7c8 into next Oct 24, 2026
16 checks passed
@roodboi
roodboi deleted the wf/g53-storage-probe-budget branch October 24, 2026 21:07
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