Repository navigation
fix(apple-runner): inline status probes must not clear outstanding command charges #2965
Description
Problem
Deletion-first acceptance
This is part of #2803's simplify-and-shrink initiative. Before production simplification, name the production mechanism, duplicated decision, state, fallback, forwarding layer, or public/internal interface this change removes. Compare deletion and inlining against extraction. Moving the same state into another file is not a deletion case.
Every PR reports baseline/final SHAs; added, deleted and net production lines; test/fixture/documentation changes separately; and the exact mechanisms removed. Use a rename-aware diff (git diff --numstat -M BASE...HEAD) and account for all affected production paths, including destination packages. Moving tests out of production files, generated output, formatting churn, and moving code elsewhere are not production-code deletion. Preserve readable code; no compressed formatting to hit a number.
Default for simplification work: net production reduction. A required behavior-preserving size split may be line-neutral. Growth needs an explicit, quantified explanation of the correctness or type guarantee it buys, why deletion/inlining cannot achieve it, and what old mechanism is removed; it is not automatically acceptable because ownership looks cleaner. If the only benefit is relocating fields or adding a wrapper, defer the change. Required regression tests and a minimal correctness fix are allowed to grow without inventing unrelated deletion to offset them.
Priority and scope gate
Priority: first correctness work. The accounting-model decision is still required; code reduction must not weaken command-specific, exactly-once settlement. Resolve the transition table first, then implement the smallest owning fix. Implementation scheduling is blocked by #2963 merging (or an explicitly declared stack on its exact published head), because both edit runner-session state and lifecycle tests; the design table can be completed now. #2911 is already merged. Only the minimal accounting extraction needed by the >1,000-line rule belongs in this fix; the broader split remains #2967.
A successful inline runner probe can clear the host's outstanding-command accounting while the command it probes is still executing. Graceful shutdown can then consider that runner eligible for handoff.
Verified by source inspection against main at 1206495222385424d96b11bd9c9be25eb267455e; no live-device reproduction yet. The trigger is a retryable post-send transport failure while the request signal still permits a status probe. An already-aborted request generally makes the probe fail and follows existing invalidation instead.
The path is:
packages/platform-apple/src/runner/runner-command-recovery.tssendsstatusthroughexecuteRunnerCommandWithSession.runner-session.tscallssettleRunnerCommandAnsweredfor a decoded answer, settling both this exchange and one previous abandoned charge.- Swift
RunnerTests+Transport.swiftservesstatusanduptimeinline, outside the serial command queue.RunnerTests+MainThreadWork.swiftreports busy only for watchdog-abandoned work, solifecycleState: 'started'withrunnerMainThreadBusy: falseis legitimate. runner-session-types.tsthen allows handoff because the outstanding count, abandoned flag, and busy flag all read idle.
Current accounting for one lost mutation response:
| Step | inFlightCommands | hasAbandonedCommands |
|---|---|---|
| Mutation sent | 1 | false |
| Transport fails | 1 | true |
| Status sent | 2 | true |
Status answers started, busy=false |
0 | false |
The last row must retain the original outstanding charge. runner-session-lifecycle.test.ts currently asserts that any answered exchange forgives it; recovery-only fixtures start with zero charges and do not catch this composition.
Required change
- Extract the command-charge accounting from the 1,353-line
runner-session.tsinto a cohesive module under the same runner directory, with its matching test. Preserve behavior in the extraction, then apply the fix there. This satisfies the repository's split-before-behavior rule without requiring the full session/exchange redesign. The existing session remains the sole state owner; do not add a parallel registry. - Settle an inline probe's own exchange without forgiving earlier abandoned work. Use the existing
readinessProbetrait forstatusanduptime, and pin its correspondence with Swift inline routing in a contract test; do not add another command-name allowlist. - In status recovery, terminal evidence (
completedorfailed) for the affected command may settle its abandoned charge exactly once.acceptedandstartedmust retain it. Unknown/missing status and probe failures keep their current invalidation policy. Repeated evidence must not discharge another command's charge. - Preserve ordinary queued-answer settlement: the serial queue makes a later queued answer evidence that earlier queued handling finished. Preserve separate native busy accounting; terminal journal status alone must never bypass
runnerMainThreadBusyor other handoff refusals.
Do not change the wire format, Swift execution/occupancy semantics, mutation replay policy, startup/adoption/disposal, or device-specific handoff eligibility. Do not broaden this into the full exchange extraction, target-state refactor, or a new command-tracking framework unless the regression demonstrates the existing representation cannot meet this contract.
Acceptance and validation
Extend the existing fake-runner coverage in runner-recovery-wiring.test.ts, entering through runAppleRunnerCommand with real command-ID assignment, send, parse, and recovery. Mock process creation as the harness already does. Assert the resulting production handoff decision; helper-only tests are insufficient.
- Lost mutation response → status
acceptedorstarted, busy=false: original charge remains and handoff refuses withcommand_in_flight; mutation was sent once. - Lost mutation response → status
completedorfailedfor that command: charge settles once; normal response/error semantics remain, and handoff is permitted only when all other conditions allow it. -
uptimenever forgives an abandoned command. Repeated terminal evidence cannot consume another outstanding charge. - Terminal status with busy=true still refuses handoff. A later ordinary queued answer retains the existing conservative settlement behavior.
- Cancellation/probe failure and existing healthy simulator/physical handoff cases retain their behavior.
- Observe the new wiring regression fail on the extracted pre-fix behavior and pass with the fix. Update the existing overly broad lifecycle assertion, retaining ordinary queued-answer coverage.
For a fresh worktree, first run pnpm install --frozen-lockfile && pnpm build. Run focused accounting, recovery-wiring, recovery, session-lifecycle, and command-trait tests with pnpm exec vitest run --project apple-runner <test-paths>, then pnpm check:affected --run on the final diff. Preserve selected CI/device obligations and report fake-runner evidence separately from any live-device verification. No new test lane is needed.
Implementation-readiness gate
The primary failure is established, but this issue is not yet AFK-ready. Before implementation, specify the identity and settlement model inside the existing session; the current inFlightCommands counter and hasAbandonedCommands boolean cannot enforce the command-specific, exactly-once requirement above.
Resolve these cases in a transition table:
- Abandon command A, abandon command B, then queued command C answers. Current conservative behavior forgives one abandoned charge: which command's charge remains?
- Terminal status for A arrives after that sequence, then arrives again. Each observation must have one unambiguous effect and must not consume B's charge.
- Multiple exchange attempts share one logical
commandId(read-only retries reuse it). State whether terminal evidence settles one attempt or all abandoned attempts for that command; live waits must remain charged until their own exchange settles.
A small identity-bearing record owned by the existing session is permitted if needed; a second registry or generic tracking framework is not. Choose and independently check these observable outcomes before restoring ready-for-agent. The full session/exchange extraction is not a prerequisite.
Dependencies: the accounting design gate remains open; implementation scheduling also follows #2963 as specified above. The broader session/resource versus command-exchange split is a separate follow-up.
Architecture follow-through
Parent: #2803. The broader command-exchange extraction is downstream and must start from this issue's exact merged implementation. For every row in the accounting design gate, record the resulting outstanding exchange identities/charges and the production handoff verdict. Independently check that table before restoring ready-for-agent; no production implementation is authorized by treating unspecified identity semantics as an implementation detail.
Activity
thymikee commentedon Sep 28, 2026
Fixed by #2994: an inline status probe no longer clears an outstanding command charge.