fix(ios): budget cold toolchain probes for the first-exec signature stall

[thymikee]

Cause

Cold Apple toolchain probes carried per-call budgets sized for a warm toolchain, below the ~18-19 s syspolicyd signature-verification stall that blocks the first xcodebuild/xcrun/large-binary exec after a fresh macOS host boots (the second exec of the same tool is instant):

• packages/platform-apple/src/snapshot-source/cache-identity.ts: toolOutput ran xcodebuild -version, sw_vers, uname, xcrun --show-sdk-version with timeoutMs: Math.min(10_000, remaining).
• packages/platform-apple/src/runner/runner-cache-metadata.ts: TOOLCHAIN_PROBE_TIMEOUT_MS = 5_000 for the runner cache key's xcodebuild -version / xcrun --show-sdk-version / xcrun --show-sdk-build-version probes.

Both tripped on cold CI runners even though the surrounding deadlines had room, producing a toolchain-probe timeout unrelated to the change under test (#2422).

Reviewing the first fix surfaced the real defect underneath: these probes block the calling request but were charged to nobody. A build or startup handed the probes a timeout and then handed itself the same number again, so a stalled probe spent the phase's budget twice.

Fix

One owner for the probe budget. COLD_TOOLCHAIN_PROBE_TIMEOUT_MS = 30_000 lives in packages/host-kit/src/internal/exec.ts, beside isCommandTimeoutError, and is exported through @agent-device/host-kit/command — it is a property of exec'ing an Apple tool, not of either prober. cache-identity.ts imports it directly. runner-cache-metadata.ts reads it through the Apple runner host port, like every other host-kit symbol that file uses: a static edge from that file to host-kit adds five modules to the packages/platform-apple/src/runner/index.ts eager closure, which scripts/__tests__/eager-closure-budgets.test.ts holds at 13.

One clock per runner phase. createRunnerPhaseDeadline opens a single Deadline when a build or a startup begins; the cache decision's toolchain probes and the step the phase exists for both read it, and requireRunnerPhaseRemainingMs throws runner_phase_budget_exhausted rather than starting a process it would have to kill at once. The toolchain fingerprint opens its own deadline at whichever ceiling is nearer — the phase's remainder, or TOOLCHAIN_FINGERPRINT_BUDGET_MS (45 s) for a caller with no phase clock — so the three probes and their retries share one budget instead of one each.

Retry exactly once, on the shared remainder. Both probers retry a timed-out attempt once, classified by the exec layer's structured details.timeoutMs rather than by message text, so a tool that failed on its own and merely said "timed out" is not retried.

Cancellation stays typed. A request canceled while a probe blocked surfaces its own cancellation, not the timeout that happened to be in flight: cache-identity.ts raises the snapshot-source cancelled / abort-signal error, runner-cache-metadata.ts raises createRequestCanceledError. runner-adoption.ts forwards the request signal into the fingerprint probes and rethrows a cancellation from resolveExpectedDerivedPath instead of turning it into skip('expected_derived_unresolved') and walking on past a client that is gone.

Budget exhaustion is not a broken toolchain. A probe reached with nothing left throws the same runner_phase_budget_exhausted error the build and startup steps throw, instead of reporting apple_toolchain_probe_unavailable with a "check xcode-select" hint for a toolchain that was never probed.

Tests

• packages/platform-apple/src/snapshot-source/cache-identity.test.ts (new file): cold-start recovery on retry with the retry charged the remainder; a probe that spends the whole deadline is not retried; a never-returning host retries exactly once; a self-reported "timed out" is not retried; and a request aborted while a probe blocked throws the typed cancelled/abort-signal error after exactly one exec.
• packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts: the same cases against resolveExpectedRunnerCacheMetadata, plus the shared-budget stop, a 4 s phase capping the single attempt, and cancellation both mid-probe and before the first exec. A fake clock advances only when a probe actually blocks for the timeout it was handed, so a case claiming the budget was spent had to spend it.
• packages/platform-apple/src/runner/__tests__/runner-artifact-phase-budget.test.ts (new file): the build phase's remaining time after the probes.
• packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts: an aborted request during the fingerprint probe fails adoption instead of skipping it.
• packages/host-kit/src/internal/exec.test.ts: isCommandTimeoutError accepts only the exec layer's own structured timeout.

Verification

• The touched test files, then pnpm check:affected --run: all green.
• pnpm typecheck, pnpm lint, pnpm check:layering: pass.
• scripts/__tests__/eager-closure-budgets.test.ts against the merge-base: no closure grows.
• No simulator/device runs; the change is budget, cancellation and error-shape only.

This unblocks the iOS smoke/preflight lane that was failing on cold runners for #2418, #2420, #2421.

The third site named on #2422 (the fixed 45 s clang compile budget in native-runtime.test.ts) is split out as #2439 and not covered here.

Closes #2422

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.53 MB	4.53 MB	+2.3 kB
Package (unpacked)	4.53 MB	4.53 MB	+2.3 kB
Package (download)	1.34 MB	1.34 MB	+900 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.1 ms	27.6 ms	-0.5 ms
CLI --help	76.6 ms	77.5 ms	+0.9 ms

[thymikee]

Fixed the eager-closure-budgets failure (666283b).

Root cause: runner-cache-metadata.ts sits in the eager closure of all seven
gated entries, so its new import of toolchain-probe-budget.ts added one
module to each. snapshot-source/cache-identity.ts, the other consumer, was
never actually reached by any of the seven closures — its own import didn't
matter for the gate.

Fix: deleted toolchain-probe-budget.ts. COLD_TOOLCHAIN_PROBE_TIMEOUT_MS
now lives inline in snapshot-source/cache-identity.ts (the file it was
already only reachable through). runner-cache-metadata.ts declares its own
copy of the same constant instead of importing it — no shared module means
no new module in any of the seven closures. A new unit test in
runner/__tests__/runner-cache-metadata.test.ts imports both constants and
asserts they're equal, so the two copies can't drift apart.

Verified locally against merge-base e7d97f7dfe (origin/main):

• eager-closure-budgets.test.ts: 577/577 pass, all seven entries back to
  their merge-base counts (120/61/101/60/47/100/13), no APPROVED_OVER_CEILING
  row added.
• check:layering: OK (220/220).
• typecheck and lint: clean.
• cache-identity.test.ts and runner-cache-metadata.test.ts (including the
  two cold-start-retry tests this PR added): all pass.

[thymikee]

The runner probes need to honor the remaining request budget at 666283b. Three synchronous probes can each run twice for 30 seconds, blocking for roughly 180 seconds without a deadline or cancellation check. Please pass the owning budget through, cap each attempt by its remaining time, and test exhausted and canceled requests; the current immediate-throw mocks do not prove the deadline.

Both retry classifiers also inspect error text. The subprocess timeout already carries structured timeout details. Please classify that signal instead, and test that a non-timeout error with similar wording is not retried.

[thymikee]

Both review points addressed at 6bf0809 (rebased on 9d7d60c5e0).

1. The probes now honor the owning request budget.

The budget threaded is the one the request already carries into the runner
preflight — there is no ambient deadline below it, so it is passed explicitly as
RunnerCacheProbeBudget { timeoutMs?, signal? }, a third argument to
resolveExpectedRunnerCacheMetadata:

• ensureXctestrunArtifact → options.buildTimeoutMs + options.signal. That
  is the prepareDeadline-derived phase timeout from fix(ios): honor the startup budget through a cold Simulator boot #2325
  (readPrepareDeadlinePhaseTimeouts sets buildTimeoutMs from
  prepareDeadline.remainingMs()), and the same abort signal the build itself
  gets.
• ensureRunnerSession's reuse path → options.startupTimeoutMs +
  resolveRunnerRequestSignal(options).
• tryAdoptRunnerSessionFromLease → options.startupTimeoutMs.

One clock is created per fingerprint read, so the three probes and their retries
share a single budget rather than each getting its own:

• every attempt runs at min(COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, remaining);
• no retry once the remaining time is 0 — the original timeout error propagates;
• a probe reached with nothing left returns a probe_error failure instead of
  starting another blocking spawnSync;
• an aborted signal throws the request-canceled error (checked before each probe
  and after each failed attempt), so cancellation surfaces instead of being
  folded into a toolchain failure. spawnSync cannot be interrupted mid-call, so
  cancellation is observed between attempts — the per-attempt cap is what bounds
  how long that takes; this is stated on the type.

Worst-case wall clock: 45 s, down from ~180 s. A caller with no budget of its
own is capped by TOOLCHAIN_FINGERPRINT_BUDGET_MS = 45_000 (one 30 s stall plus
its now-warm retry and the two remaining probes); a caller with less time is
capped by its own. Error codes, texts, retriable, and TOOLCHAIN_PROBE_HINT
are unchanged — an exhausted budget still fails as
COMMAND_FAILED / apple_toolchain_probe_unavailable with the hint.

2. Typed timeout classifier in both consumers.

isCommandTimeoutError(error) is now exported once from
packages/host-kit/src/internal/exec.ts via @agent-device/host-kit/command:
AppError with code COMMAND_FAILED and a numeric details.timeoutMs, the
detail both createTimeoutError and the runCmdSync ETIMEDOUT branch stamp.
snapshot-source/cache-identity.ts imports it directly. runner-cache-metadata.ts
reaches it through the Apple runner host port (runner/host.ts +
core/runner-host.ts) rather than a new import, so its eager closure is
unchanged. Both local regex helpers are deleted.

New tests. The immediate-throw mocks are replaced by a fake clock installed
through the test host's deadlineFromTimeoutMs, advanced only by a fake exec
that blocks for the whole timeout it was handed — a case claiming the budget was
spent has to spend it.

runner/__tests__/runner-cache-metadata.test.ts (describe('toolchain probe budget'),
each case starting from an empty fingerprint memo):

• cold-start recovery: attempt timeouts are exactly [30000, 15000] — the retry
  runs on what the shared budget has left, not a fresh ceiling;
• never-returning host: 2 execs total and clock.nowMs === 45_000; the two xcrun
  probes fail on the budget instead of blocking 30 s each;
• owning request with 4 s left: one 4 s attempt, no retry, clock.nowMs === 4_000;
• canceled while a probe blocked: 1 exec, request-canceled error surfaces;
• already-canceled request: 0 execs;
• COMMAND_FAILED saying timed out after 10ms with no timeoutMs detail: 1
  attempt, not retried.

snapshot-source/cache-identity.test.ts: the same non-timeout look-alike is not
retried (1 call), and the always-timing-out case now pins exactly 2 calls.
host-kit/src/internal/exec.test.ts: the predicate is true for real runCmd and
runCmdSync timeouts and false for the look-alike message, a plain Error, and
undefined.

Verified locally against merge-base 9d7d60c5e0: eager-closure-budgets.test.ts
581/581 (no growth on any of the seven entries), typecheck, lint,
check:layering (220/220), check:affected --run (715 files / 5351 tests), and
the apple-runner project suite (382 tests). No device runs.

[thymikee]

The typed timeout check and shared probe budget are improvements, but ensureXctestrunArtifact gives probing options.buildTimeoutMs and then gives the build that same full timeout again. A cold probe can add 30–45 s on top of the phase budget. Pass one remaining deadline through both and add a probe-plus-build regression. The snapshot-source timeout test also needs to advance its clock and prove the retry receives only the remainder. Finally, #2422 asks both consumers to read one timeout constant: move it to a small dependency-free owning module instead of keeping two copies plus an equality test.

[thymikee]

Addressing the three 15:13 points. Branch was rebased onto main (now including #2420, #2421, #2416) and force-pushed with lease; the fix itself is unchanged in substance, just re-based.

(a) One deadline across the probes and the step they precede

RunnerCacheProbeBudget now carries the phase's deadline instead of a raw timeoutMs number, and each caller builds exactly one:

• ensureXctestrunArtifact — the toolchain probes and xcodebuild read the same clock; a phase with nothing left fails before the spawn.
• ensureRunnerSession — the reuse probe spends from the startup clock; the new session gets what it left.
• tryAdoptRunnerSessionFromLease — the fingerprint probe spends from the caller's clock; the adopted session gets the remainder.

New regression test: packages/platform-apple/src/runner/__tests__/runner-artifact-phase-budget.test.ts (3 cases, using a fake clock that only advances when a probe actually blocks for the timeout it was given):

• "a warm toolchain leaves the build the whole phase budget" — with a 120 s phase, no probe blocks, xcodebuild is still handed the full 120,000 ms.
• "a cold-start probe stall comes out of the build budget instead of being added to it" — the first xcodebuild toolchain probe blocks for its whole 30 s ceiling and is retried; xcodebuild is then handed 90,000 ms (120,000 − 30,000), not a second full 120,000.
• "a probe that spends the whole phase fails before xcodebuild is spawned" — a probe consumes all of a 30 s phase; ensureXctestrunArtifact rejects with reason: 'runner_phase_budget_exhausted', phase: 'runner_xctestrun_build', and runCmdStreaming (the xcodebuild spawn) is called 0 times.

Net effect on worst-case wall clock: a cold-start probe stall now comes out of the phase budget instead of stacking on top of it, so the worst case for the phase is the phase budget itself, not phase-budget-plus-probe-stall.

(b) createSnapshotSourceDeadline takes an injectable clock

cache-identity.test.ts now drives a fake clock that only moves when a probe actually blocks for the timeout it was handed (so a case claiming the budget was spent had to spend it):

• "a cold-start toolchain probe recovers on retry, and the retry gets only what the stall left" — deadline built with 40,000 ms; the stalled first attempt is capped at the 30,000 ms per-probe ceiling, and the retry runs on the 10,000 ms the shared deadline has left: timeouts.slice(0, 2) = [30_000, 10_000], clock.nowMs === 30_000.
• "a probe that spends the whole deadline is not retried" — deadline built with 30,000 ms; the probe blocks for the full 30,000 ms, leaving nothing: timeouts = [30_000] (single attempt, no retry), clock.nowMs === 30_000.

(c) The constant has one owner

COLD_TOOLCHAIN_PROBE_TIMEOUT_MS now lives only in packages/platform-apple/src/core/config.ts. snapshot-source/cache-identity.ts imports it directly. runner/runner-cache-metadata.ts instead reads it through the Apple runner host port (AppleRunnerHost.coldToolchainProbeTimeoutMs()), because core/config.ts is missing from one of the seven eager closures that scripts/__tests__/eager-closure-budgets.test.ts evaluates for that file, and a direct import would grow it. The equality test that used to pin the two duplicated constants together (COLD_TOOLCHAIN_PROBE_TIMEOUT_MS matches the copy in snapshot-source/cache-identity.ts) is gone — there's only one definition left to test.

Gate results (from the worktree, post-rebase)

• pnpm typecheck — clean
• pnpm lint (oxlint --deny-warnings) — clean
• pnpm check:layering — 221/221 passed
• eager-closure-budgets.test.ts (against merge-base origin/main) — 581/581 passed; all seven Apple façade entries hold at merge-base counts
• Touched tests — runner-artifact-phase-budget.test.ts (3), runner-cache-metadata.test.ts (32), runner-client.test.ts (50), cache-identity.test.ts (4): 89/89 passed
• pnpm check:affected --run — 717 test files / 5401 tests passed, all runnable checks passed

Force-pushed claude/2422-cold-toolchain-probe with --force-with-lease after the rebase.

[thymikee]

The shared phase deadline, fake-clock tests and single timeout owner address the previous findings at b9bc957. One cancellation case remains: after a probe times out, toolchainProbeDeadlineHasRoom returns false for an aborted signal and rethrows the timeout. Preserve the typed cancellation instead, and add a timeout-then-abort regression proving there is no retry and the cancellation reason survives. CI is green.

Please also replace Closes #2422 with a reference, or split its remaining native-runtime compile-timeout case into a tracked follow-up before closing it; that third site is outside this fix.

[thymikee]

Round 3, rebased on origin/main.

The remaining cancellation case

runToolchainProbe in snapshot-source/cache-identity.ts asked toolchainProbeDeadlineHasRoom, which answered "no room" for an aborted signal and rethrew the exec timeout — the abort reached the caller wearing the wrong error. It now classifies in order: not a structured timeout, rethrow; signal aborted, throw this module's own snapshotSourceError('cancelled', 'abort-signal'), the same typed cancellation remainingSnapshotSourceMs raises; deadline spent, rethrow the timeout; otherwise retry. toolchainProbeDeadlineHasRoom is deleted.

New regression in cache-identity.test.ts: the fake host blocks for its whole timeout and aborts the signal during that block, with 90 s still on the deadline so only the cancellation can stop the retry. It asserts exactly one exec and a SnapshotSourceError of kind cancelled / code abort-signal with reason: 'request_canceled'. Planted red confirmed against the old predicate.

Runner side checked as asked: a request canceled while a probe blocked surfaces the cancellation instead of retrying already asserts isRequestCanceledError and one exec, so it is unchanged.

Cancellation swallowed in adoption

resolveExpectedDerivedPath wrapped the fingerprint probe in catch { return null }, so a cancellation became skip('expected_derived_unresolved') and startup walked on. It now rethrows on isRequestCanceledError (the kernel predicate, not message text). The signal could not actually reach those probes — adoption never accepted one — so it is now forwarded from ensureRunnerSession's existing request signal, the same one the reuse path already passes. New test in runner-adoption.test.ts; planted red confirmed.

Simplifications

• Second clock — createToolchainProbeClock opened a 45 s fingerprint deadline beside the phase deadline and took a three-way Math.min. Now one Deadline, opened at min(TOOLCHAIN_FINGERPRINT_BUDGET_MS, phase remainder); both are wall-clock, so the step after the probes still sees what they spent. The "Two clocks, because…" comment is gone.
• Exhausted-budget failure path — a budget spent before a probe started returned a probe failure, surfacing as apple_toolchain_probe_unavailable with a "check xcode-select" hint for a toolchain nothing had probed. It now throws the same runner_phase_budget_exhausted error the build and startup steps throw, from one shared constructor that requireRunnerPhaseRemainingMs also uses. TOOLCHAIN_PROBE_BUDGET_EXHAUSTED_DETAIL is deleted and the two budget tests assert the new shape.
• RunnerPhaseDeadline alias — deleted, along with its two barrel re-exports; Deadline is used directly in runner-artifact.ts, runner-adoption.ts, runner-session.ts and RunnerCacheProbeBudget. createRunnerPhaseDeadline and requireRunnerPhaseRemainingMs stay.
• core/config.ts copy of the constant — deleted. COLD_TOOLCHAIN_PROBE_TIMEOUT_MS now lives in packages/host-kit/src/internal/exec.ts beside isCommandTimeoutError, exported through @agent-device/host-kit/command, which is the honest owner: it is a property of exec'ing an Apple tool, not of either prober. cache-identity.ts imports it from there.

Net production diff: 421+/66- over 13 files → 431+/67- over 12 files. The deletions above are offset by the shared error constructor, the adoption signal plumbing, and the doc that moved with the constant.

One item I could not do: deleting the port method

You asked me to drop coldToolchainProbeTimeoutMs from the AppleRunnerHost port and let runner-cache-metadata.ts reach the constant the way it reaches Deadline and createTtlMemo. Two things in the tree do not match that plan:

1. runner/host.ts has no plain module re-exports at all. It imports only types; Deadline, createTtlMemo, isEnvTruthy and every other runtime symbol are port methods over requireHost(). There is no "plain re-export form" to copy.
2. I tried the direct import anyway and the eager-closure gate fails:

packages/platform-apple/src/runner/index.ts evaluates 18 modules on import;
the merge-base evaluated 13.
  packages/platform-apple/src/runner/index.ts
      -> packages/platform-apple/src/runner/runner-cache-metadata.ts
      -> packages/host-kit/src/command.ts
  (+4 more newly evaluated module(s))

runCmdSync reaches that file through the port, so host-kit's exec module is not in the runner entry's closure today; a static edge to it adds five. Per your instruction I stopped rather than forcing it.

What I shipped instead keeps the part that was actually wrong — two homes for one number — and drops the part that was not: host-kit owns the constant, core/config.ts no longer has a copy, and the composition root (which already imports @agent-device/host-kit/command) feeds it to the port. The port doc now names host-kit as the owner and cites the gate by name instead of the vaguer eager-closure paragraph. isCommandTimeoutError stays a port method for the same reason. Happy to revisit if you would rather approve the closure growth.

Split-out

Per your second note, the third site named on #2422 — the fixed 45 s clang compile budget in native-runtime.test.ts — is now #2439 and called out in the PR body as not covered here. Closes #2422 is kept, since both sites #2422 names as this fix are addressed.

Verification

Touched test files, pnpm typecheck, pnpm lint, pnpm check:layering, scripts/__tests__/eager-closure-budgets.test.ts against the merge-base (no closure grows). No device runs.

pnpm check:affected --run reports 1 failure: runner-client.test.ts :: ensureXctestrunArtifact aborts only the disconnected request build… times out at 5 s. It is pre-existing, not from this round — the identical run at the parent commit cf5fbc1331 fails the same single test at the same 5 s, and the test passes 5/5 in isolation on this branch. The slow-test gate already flags it at 5.1 s against a 2.5 s budget, so it tips over whenever the suite runs it under parallel load.

[thymikee]

The timeout-then-abort fix is in place at 2f4462e, but the warm-cache path still skips cancellation: requireRunnerToolchainFingerprint returns its memoized value before checking the signal. Adoption can then probe uptime and write the lease for an already-canceled request. Check cancellation before the cache return, and add a regression that warms the real fingerprint cache before passing an aborted signal; require typed cancellation with no uptime probe or lease adoption. The compile-timeout follow-up is now correctly tracked in #2439.

[thymikee]

Addressed in 47220f6 (fix(apple): check cancellation before the warm toolchain fingerprint cache, rebased on main):

• Where the check moved: requireRunnerToolchainFingerprint in runner-cache-metadata.ts now creates the ToolchainProbeClock and calls clock.throwIfCanceled() before the toolchainFingerprintCache().get(sdkName) lookup, then reuses that same clock for the cold-probe path. Per-probe checks inside runToolchainProbe are unchanged.
• runner-cache-metadata.test.ts: folded the new warm-cache case into the existing cold-cache test (now 'an already-canceled request runs no toolchain probe at all, cold or with the fingerprint cache warm') rather than adding a near-duplicate — it first asserts the cold path, then warms the real memo with an ordinary call and re-asserts the same typed-cancellation + zero-exec-calls outcome on the cache-hit path.
• runner-adoption.test.ts: extended the existing 'a request canceled during the fingerprint probe fails adoption instead of skipping it' test (renamed to add ", cold or with the fingerprint cache warm") with a second phase that bypasses the file's resolveExpectedRunnerCacheMetadata mock for one call, warms the real singleton fingerprint memo directly, then calls tryAdoptRunnerSessionFromLease again with an aborted signal — asserting typed cancellation, sendRunnerCommandOnce (uptime probe) never called, and the stale lease's ownerToken untouched (no adoption/write).
• Verified both new assertions fail without the fix (reproduced the bug) and pass with it.

Verification: pnpm typecheck, pnpm lint, pnpm check:layering all clean; pnpm check:affected --run is green except the known contention flake in runner-client.test.ts ("aborts only the disconnected request build", times out at 5s under load, confirmed passing alone).

[thymikee]

Pushed b6611b6cf6: folded the sync toolchain probe's three duplicated cancellation/budget guard sites (runToolchainProbe's pre-check, runToolchainProbeCommand's retry pre-check, execToolchainProbeCommand's timeout computation) into one — attemptToolchainProbe now checks cancellation and the remaining budget before every exec, first attempt and retry alike, via clock.throwIfCanceled() then a timeoutMs <= 0 check that throws runnerPhaseBudgetExhaustedError. runToolchainProbeCommand's retry just re-invokes that same guarded attempt. runToolchainProbe's catch now rethrows cancellation and a spent budget (checked via isRequestCanceledError and a new isRunnerPhaseBudgetExhaustedError reason check) instead of swallowing them into a probe failure — only genuine probe errors become one.

Net line delta: +25/-17 (net +8) in packages/platform-apple/src/runner/runner-cache-metadata.ts.

packages/platform-apple/src/snapshot-source/cache-identity.ts (the async twin) was left unchanged: its explicit retry pre-check rethrows the original per-attempt timeout error on budget exhaustion (not a synthesized error), which folding into the shared remainingSnapshotSourceMs guard would change — that guard throws a different snapshotSourceError('timeout', ...) and would break cache-identity.test.ts's "a probe that spends the whole deadline is not retried" assertion on the exact rethrown error/message.

Tests unchanged: all 55 cases across runner-cache-metadata.test.ts, runner-adoption.test.ts, runner-artifact-phase-budget.test.ts, and cache-identity.test.ts pass unmodified. pnpm typecheck, pnpm lint, pnpm check:layering all clean. pnpm check:affected --run is green except the known runner-client.test.ts 5s contention timeout, confirmed passing (50/50) in isolation.

[thymikee]

The guard cleanup at b6611b6 loses cancellation on a failed final probe. If xcrun --show-sdk-build-version aborts the request and throws a non-timeout error, the catch now converts it to apple_toolchain_probe_unavailable; there is no next attempt to check cancellation. Previously the catch checked the signal before classifying the failure. Preserve that check and add a regression where the final probe aborts and throws a non-timeout error, asserting the typed cancellation instead of a toolchain failure.

[thymikee]

Restored in c24d42c95e: runToolchainProbe's catch calls clock.throwIfCanceled() before classifying the failure, so a request that aborts while the last probe is in flight and then fails with a non-timeout error surfaces the typed cancellation instead of apple_toolchain_probe_unavailable.

Regression added: the fake exec answers the first two probes and, on xcrun --show-sdk-build-version, aborts the request and throws a plain COMMAND_FAILED without the structured timeout detail. Asserts isRequestCanceledError and exactly three execs. Red without the one-line fix (1 failed | 32 passed), green with it (33 passed). Typecheck and lint clean.

[thymikee]

The cancellation regression is fixed at c24d42c. The final-probe test now checks the non-timeout failure case, and the common catch preserves typed cancellation without changing the retry budget. No remaining code findings; current checks are green.

[thymikee]

Structural round on top of c24d42c — pushed as b21efb0. No behaviour change: the shared phase deadlines, the one timeout-only retry, the cancellation check before the fingerprint cache hit and the distinct budget-exhaustion error are all untouched, and no retry abstraction was added.

(A) The 30 s constant now lives in platform-apple

COLD_TOOLCHAIN_PROBE_TIMEOUT_MS moved to packages/platform-apple/src/runner/apple-runner-platform.ts — the module that already owns the Apple SDK names the probes are run against (resolveRunnerSdkName produces the --sdk argument of the very probe this bounds). Both probers now import it directly:

• runner/runner-cache-metadata.ts (already imported that module — +0 edges)
• snapshot-source/cache-identity.ts (new edge; cache-identity.ts is in no gated entry's eager closure, so also +0)

Deleted with it: COLD_TOOLCHAIN_PROBE_TIMEOUT_MS from @agent-device/host-kit/command and internal/exec.ts, and coldToolchainProbeTimeoutMs (port type + wrapper) from runner/host.ts and its binding in core/runner-host.ts. isCommandTimeoutError stays in host-kit, as asked — the exec layer stamps the detail it classifies on.

Why not a new tiny module: scripts/__tests__/eager-closure-budgets.test.ts enforces no growth vs the merge-base for every existing entry, and APPROVED_OVER_CEILING only covers first-introduced entries (see the header of scripts/__tests__/eager-closure-budgets.ts). Round 1's dependency-free toolchain-probe-budget.ts was +1 on all seven Apple entries that evaluate runner-cache-metadata.ts.

Evidence — intersection of the eager closures of those seven entries (app-lifecycle-facade, app-resolution-facade, doctor-facade, perf-facade, physical-device-facade, runner-operations-facade, runner/index.ts) is exactly runner/index.ts's own 13 modules. Its packages/platform-apple/src/ members are:

runner/apple-runner-platform.ts   <- chosen (leaf, owns Apple SDK identity)
runner/host.ts                    (leaf, but it IS the host port the review wants this out of)
runner/runner-source.ts           (leaf, but it owns the runner Swift source fingerprint)
runner/runner-provider.ts         (leaf, provider seam — unrelated)
runner/runner-sequence.ts         (leaf, tap/press sequences — unrelated)
runner/runner-cache-metadata.ts   (the consumer itself)
runner/index.ts                   (the entry barrel)

Nothing under core/ is in the intersection at all, so core/config.ts and its neighbours were never candidates. pnpm check:layering is green on the new snapshot-source/ -> runner/ edge, and the closure gate is back at merge-base counts (581/581).

(B) Splitting runner-cache-metadata.ts — measured, deferred

Not done here, because it is unlandable under the current gate. Measured by planting runner-phase-budget.ts + runner-toolchain-probe.ts (the phase-budget helpers and the toolchain probing, imported directly, no re-export barrel) and running the gate:

entry	merge-base	with the split	delta
packages/platform-apple/src/app-lifecycle-facade.ts	120	122	+2
packages/platform-apple/src/app-resolution-facade.ts	61	63	+2
packages/platform-apple/src/doctor-facade.ts	101	103	+2
packages/platform-apple/src/perf-facade.ts	60	62	+2
packages/platform-apple/src/physical-device-facade.ts	47	49	+2
packages/platform-apple/src/runner-operations-facade.ts	100	102	+2
packages/platform-apple/src/runner/index.ts	13	15	+2

Seven failures, +2 each — one per extracted module, since neither pulls an edge the closure does not already have. There is no approval row for growth of an existing entry, so this needs a gate follow-up (an approval path for a same-weight file split, or a way to say "these modules are one unit"). The plant was discarded; runner-cache-metadata.ts is unchanged in structure.

(C) Comments

Production diff (':!*.test.ts' ':!*__tests__*', vs merge-base 41e2633): 164 → 69 added comment lines, −95, with zero code change from the trim (the only non-comment lines this commit touches are the constant's move).

Kept, once each: the cold-start stall rationale (on the constant), "spawnSync cannot be interrupted once it has started, so cancellation is observed between attempts" (on ToolchainProbeClock), and one line at each phase-deadline creation site saying the probes and the step share it (runner-artifact.ts, runner-session.ts). Gone: the multi-paragraph docs on createRunnerPhaseDeadline, requireRunnerPhaseRemainingMs, runnerPhaseBudgetExhaustedError, RunnerCacheProbeBudget, runToolchainProbeCommand and its cache-identity.ts twin, the "arrives through the port" paragraph (deleted with the port method), the clock-construction narration and the catch-block narration.

Verified from the worktree

runner-cache-metadata.test.ts / runner-adoption.test.ts / runner-artifact-phase-budget.test.ts / cache-identity.test.ts (56 passed) · pnpm typecheck · pnpm lint · pnpm check:layering · eager-closure gate vs origin/main (581 passed) · pnpm check:affected --run (719 files, 5413 tests, no runner-client.test.ts timeout this run).

[thymikee]

No code findings in b21efb0. Moving the shared timeout into the Apple module removes the extra host-port plumbing without changing retry, cancellation or deadline behavior. The integration failure is an unrelated connection reset while fetching Chrome-for-Testing metadata; rerun that job before merge.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-10 14:34 UTC