Repository navigation
refactor(ios-runner): one response decoder, a session state enum, and a verdict on the curl-through-simctl transport #2662
Description
Activity
(1) landed in #2666 (
c57649357b). Three things the next cut needs from it:The one decoder lives in
runner-contract.ts, notrunner-session.ts.runner-adoption.tscannot importrunner-session.ts(session imports adoption, and the layering rule rejects value cycles), and a separate module was refused byeager-closure-budgets:runner-session.tssits in the eager closure of the app-lifecycle, doctor and runner-operations facades, so one more module takes those closures past the merge-base count.runner-contract.tsis already evaluated by all three facades and already imported by all three readers — it is the only cycle-free home that keeps that gate green, and the envelope is the response half of the contract that file already owns.parseRunnerResponsestays the single place that decides success.The literal grep still shows five
JSON.parsesites inrunner/*.ts.runner-contract.ts:492is the only one decoding a runner response body; the rest read lease files (twice), cache metadata, and tool stdout — the #2598 cluster this issue puts out of scope.okacceptance is now=== true(the SwiftBool). Every body a real runner can send behaves exactly as before; a body reading"ok":"true"stops counting as a completed command's own retained result, which is the intended tightening.For (3), the archaeology branch is empty:
git log -Sputs the curl-through-simctl path in the initial commit4da4745a6c(2026-01-30), with no introducing commit and no failure it worked around anywhere in history. So the verdict is only available from theios_runner_startup_transportinstrumentation read over the lanes, or from deciding to delete it.One hole found while doing (1), deliberately left because (1) promised no behavior change: JSON that parses but is not an object (
42,"x") decodes to{}atrunner-contract.ts:492,buildRunnerResponseErrorthen stampsrunner: {}, andisStructuredRunnerFailure(runner-session.ts:820) reports true — so a body that is not an envelope at all is read as "the runner served an answer" and clearsrunnerMainThreadBusy(runner-session.ts:800). A truncated body is handled correctly: transport-shaped, stamp kept (#2552). The candidate fix is one line — a non-object decode throws the sameInvalid runner responseAppErroras malformed text — but it changes what counts as an answer, so it belongs in (2), where the session stops reconstructing "did the runner answer" from booleans.Follow-up on sub-task (2), for after the session state enum lands.
RunnerSessionalready hasstate: 'starting'atpackages/platform-apple/src/runner/runner-session.ts:283andadvanceRunnerSessionStateis in use, so the enum half of (2) is onmainsince33d48a14. Worth checking before someone re-does it.The half I actually want is the one that escapes the runner directory. The post-dispatch outcome is already decided internally, and decided correctly:
- Mutations are sent exactly once. Only read-only commands take the retrying
waitForRunnerconnect loop (runner-session.ts:903); a mutating command goes through one send after the readiness preflight. - When the response is lost after dispatch,
runner-command-recovery.ts:237-272asks the runner'sstatusfor a retained response and then distinguishescompleted_with_retained_response(payload recovered) fromcompleted_without_retained_response(the command ran, the answer is gone), surfacing the latter as aCOMMAND_FAILEDwhosedetailscarrylifecycleState: 'completed'andrecovery.
So we have the three states — ran, never dispatched, outcome unknown — but only two of them are names a caller can depend on. What reaches an SDK consumer for "the tap probably landed and I cannot prove it" is an error envelope with
details.lifecycleState, i.e. exactly the stringly-typed sniffing this issue exists to remove one level down.Ask: once (2) is in, give the outcome a name at the contract boundary. In
packages/contracts/src, a documented result field (something likedispatchOutcome: 'completed' | 'not-dispatched' | 'unknown') derived from the session state plus the recovery reason, with the mapping in one place.details.lifecycleStateanddetails.recoverycan stay as diagnostics.Why this belongs to this issue rather than its own: the producer is the state enum from (2) and the decoder from (1), and the acceptance bar is the same — one source of truth, no second reader allowed. There is external demand for it; a consumer of ours is reportedly reimplementing single-attempt mutation policy on top of our error text because it cannot tell these three cases apart from the public response. That report is secondhand, but the reason to publish the field does not depend on it.
- Mutations are sent exactly once. Only read-only commands take the retrying
Disposition at main
e4716e6f514273f060b4f1fb8a523d3c51a9c17d: close the remaining investigation as not planned, retaining the simulator curl transport. This is a prioritization decision, not a claim that task 3 was implemented or transferred.The response decoder and session state have already landed. The residual
postCommandViaSimulatorpath is still reachable from startup connection attempts and from the final simulator fallback inrunner-startup-transport.ts. The function is about 50 lines, and there is no demonstrated correctness defect or measured deletion benefit sufficient to justify a separate instrumentation campaign. We have not established that it is redundant. Retain it, its encoder, scoped simulator-set routing, and timeout/cancellation behavior. A concrete transport incident or a measured removal case can reopen this work.The public consumer ask now has an owning contract: #3071 and #3082, followed by #3099, expose typed
error.details.dispatched: "no" | "unknown", driven bycontracts/fixtures/dispatch-disclosure.jsonand documented in ADR 0011. Apple transport and status recovery classify pre-dispatch refusals and lost replies there. A recovered result succeeds; a failed mutation whose reply is lost isunknown, including completed-without-retained-reply. We deliberately do not add a seconddispatchOutcomefield or promisecompletedon a failure. This is the retry-safety decision consumers need without reading lifecycle/recovery strings.#2803 is being reconciled to remove the outstanding curl investigation. No transport code is deleted by this disposition.
Remaining scope in the deletion-first initiative
#2803 now includes task 3 (the curl-through-simctl verdict) in its measurement workstream. The decoder/state-enum work has already landed; do not repeat it from the historical descriptions below.
Before deleting
postCommandViaSimulator, establish that the control route is reachable and reproduce or explicitly account for the original failure condition. A week with no fallback observations is insufficient if the relevant simulator-set/toolchain/network condition was never exercised. Record baseline SHA, route selection, successful primary/fallback controls, timeout/cancellation outcomes, and a retain/delete verdict. Preserve #2963's scoped simulator-set behavior; coordinate overlapping edits with that PR. If retained, name the actual condition and evidence. If deleted, report all removed production paths/encoders and the net production-line delta, with tests separate. Temporary instrumentation is removed or justified as an owning diagnostic.The public dispatch-outcome discussion remains outside this cleanup scope. Historical task descriptions follow.
Why
The largest maintenance surface on the Apple platform is not geometry, it is the daemon-side cluster that babysits the XCTest runner process:
packages/platform-apple/src/runner/is 38 production files, about 10k lines. A survey onmainat91652a8fc5found the following (file:line refer to that head):parseRunnerResponsePayloadinrunner-session.ts:973(the canonical one),parseLifecycleResponsePayloadinrunner-command-recovery.ts:353(thestatusrecovery probe), and an inlineJSON.parseinrunner-adoption.ts:140(theuptimeprobe).runner-transport.ts:48(sendRunnerCommandOnce, host TCP / usbmux),runner-usbmux.ts:52(raw HTTP framing over the usbmux socket),runner-startup-transport.ts:371(tryRunnerEndpoints, the startup connect probe) andrunner-startup-transport.ts:418(postCommandViaSimulator:simctl spawn <udid> /usr/bin/curl …, a shell-out inside the Simulator, still reachable from lines 119 and 400).isRunnerProcessAlive/isRunnerProcessTreeAlive/runnerSessionsStillAliveinrunner-disposal.ts:274/269/177;hasLiveIosRunnerSessioninrunner-client.ts:192readinggetRunnerSessionSnapshotinrunner-session.ts:442;probeRunnerAnswersUptimeinrunner-adoption.ts:127(wire-level); andcanSkipRunnerReadinessPreflightAfterHealthyMutationinrunner-command-traits.ts:56, a separate "healthy enough to skip the preflight" axis.RunnerSession(runner-session-types.ts) has nostatefield, only booleans (ready, computedalive,lastHealthyMutation); the readiness-preflight decision inrunner-session.ts:87–101and the cache decision inrunner-cache.ts:450–462are string-literal unions that stand in for one.runner-startup-transport.ts:36–40,runner-disposal.ts:26–33,runner-session.ts:78–81,runner-lease.ts:23–25,runner-device-set.ts:18–20,runner-sequence.ts:24–35, and single constants in six more files).None of this is a bug. It is where the next incident will take longest to diagnose, and it is the code a contributor has to read to touch anything about runner startup.
Task
Three bounded cuts, each its own PR, in this order. Do not attempt a rewrite.
parseRunnerResponse(runner-session.ts) the only place a runner response body is decoded; have the recoverystatusprobe and the adoptionuptimeprobe call it (or a narrower function it exports) and delete the two private decoders. Tests:runner-command-recoveryandrunner-adoptionsuites keep their current expectations; add one test per probe proving a malformed body is rejected the same way the main path rejects it.state: 'starting' | 'ready' | 'draining' | 'stopped'(adjust names to what the code actually distinguishes; derive from the existing booleans and the readiness-preflight reasons, do not invent states) toRunnerSession, with one transition function. Replace the six "alive" checks with two: process liveness (OS fact,host.ts) and session state.hasLiveIosRunnerSessionbecomes a state read. The readiness-preflight decision keeps its reason codes (they are diagnostics), but the decision readsstatepluslastHealthyMutation. AnySessionState-like field added to a persisted record follows the R7/R10 rule inCONTEXT.md(owner entry plus schema bump).postCommandViaSimulatorif it is dead. Establish first whether the curl-through-simctl path ever answers where the host TCP and usbmux paths do not, after usbmux became primary (Explore usbmux as the primary physical iOS runner transport #1403). Instrument with a diagnostic (ios_runner_startup_transportphase, which transport answered) and read it over the iOS simulator lanes and the nightly (.github/workflows/xctest-nightly.yml) for a week, or find the commit that introduced it and the failure it worked around. If no run needs it, delete it together with its encode path; if one does, document the condition next to the function and close this sub-task.Acceptance criteria
grep -rn 'JSON.parse' packages/platform-apple/src/runner/*.ts(production files) shows exactly one site;pnpm check:affected --rungreen.RunnerSessionhas astatefield;grep -rn 'alive' packages/platform-apple/src/runner/*.tsshows only the process-liveness primitive and reads ofstate; the daemondevice statusoutput for a running iOS session is unchanged (compare JSON againstmain); the iOS simulator integration lane and the macOS host lane green; one recorded startup, one idle-stop and one recycle each transition through the enum, asserted inrunner-sessiontests.pnpm check:layering) andpnpm check:production-exportsstay green; no new exports frompackages/platform-apple/src/runner/index.ts.Non-goals
Merging or renaming files for their own sake. Changing lease, cache or xctestrun-artifact logic (
runner-lease.ts,runner-cache*.ts,runner-artifact*.ts): those are a different cluster with their own invariants (#2598). Reducing the number of timeout constants.Related: #1403 (usbmux primary), #2324 / #2325 (startup budget), #2598 (process lock), ADR 0005 (runner interaction lifecycle), ADR 0019 (request-bound platform runtime).