fix(ios-runner): dispose a busy runner on close so open boots clean (#2552)

[thymikee]

Summary

close then open no longer hands a wedged iOS runner back to the caller (#2552). A runner still finishing watchdog-abandoned main-thread work refuses every command until it drains or escalates to RUNNER_WEDGED, but plain close retained it and the next open reused the same stalled runner, so close recovered nothing.

The runner now stamps its live main-thread occupancy (runnerMainThreadBusy) on every successful response and tags the execution-watchdog timeout with the typed MAIN_THREAD_TIMEOUT code, so the stalling command itself reports the occupancy — not only a later RUNNER_BUSY refusal. The daemon mirrors it on the RunnerSession: set on RUNNER_BUSY or MAIN_THREAD_TIMEOUT, cleared by any other served reply (reaching the main thread proves the drain), and left intact by a transport failure or an unstamped recovered response. At close, finalizeAppleApplicationClose hands a single intent, releaseRunnerOnClose(deviceId, { retain }), to the runner module that owns the occupancy bit: an idle retained runner keeps warm reuse under the idle timer, while a runner still draining is disposed (like the RUNNER_WEDGED restart) so the next open boots a clean runner. The decision lives with the state it reads, scheduleIosRunnerIdleStop stays pure scheduling, and the close route is awaited so the lease is released before the next request.

Closes #2552.

Validation

Tested commit 57ab7ad (test-only on top of 029c9e9; production code unchanged).

• pnpm check:affected --run: everything passed except mutation-model, which fails the same way on a clean origin/main (scripts/mutation/ownership.test.ts, kernel ownership).
• runner-session-close.test.ts: the release test now trips a real MAIN_THREAD_TIMEOUT through executeRunnerCommandWithSession instead of setting the flag; deleting the catch-side busy marking makes it fail. test(apple-runner): pin the close release truth table at the lifecycle seam #2623 (stacked) pins the full close truth table at the lifecycle seam.
• Daemon close tests assert releaseIosRunnerOnClose(id, { retain }) arguments; the mocks no longer copy the retain policy.
• Live, fresh iPhone 17 simulator (iOS 26.2), Settings app, repo CLI with --debug:
  • Idle: snapshot, close logs ios_runner_idle_stop_scheduled; reopen + snapshot logs ios_runner_session_reuse; same xcodebuild pid.
  • Busy: longpress 200 130 45000 returns MAIN_THREAD_TIMEOUT at 30 s with the runner still alive; close logs ios_runner_retain_skipped_busy and the runner exits; reopen logs ios_runner_session_startup with a new pid.

Earlier at 029c9e9: check:xctest-selection, check:packaged-runner-swift, and macOS build-for-testing green.

[github-actions]

Size Report

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

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.4 ms	27.9 ms	-0.5 ms
CLI --help	81.1 ms	79.9 ms	-1.2 ms

[thymikee]

At c1c17da idle runners keep warm reuse, and only this session's runner is disposed. But the direct repro from #2552 is not covered yet.

The command that stalls never marks the runner busy. When the watchdog gives up, the runner answers with an error (mainThreadExecutionTimedOut), not RUNNER_BUSY. Error responses carry no occupancy stamp, and the catch in runner-session.ts only sets the flag for RUNNER_BUSY. So a stalled snapshot, then close, then open still reuses the busy runner, unless another command got RUNNER_BUSY in between. Please set runnerMainThreadBusy for the watchdog-timeout error too, ideally from a typed runner code.

The flag can also stay set after the runner drains. Only a successful stamped response clears it, so if every reply after the drain is an error (for example element not found), close kills a healthy runner and the next open pays a cold boot. Clearing it on any runner reply that is not RUNNER_BUSY would fix that.

scheduleIosRunnerIdleStop now starts a disposal it does not wait for, so close returns before the runner is gone. A smaller-design question: finalizeAppleApplicationClose already has an awaited stop branch, so could close ask whether the runner is busy and take that branch? That would also remove the second disposal path from a function named for scheduling.

The close test sets the flag by hand and calls the scheduler directly. A test that goes through close and then ensureRunnerSession and checks that a new runner boots would cover the production route.

There is no live wedge repro yet, Smoke Tests are still running, and there are no conflicts.

[thymikee]

Pushed 44f78a986d. All four points are addressed:

1. Stalled command now marks the runner busy from a typed code. The runner maps mainThreadExecutionTimedOut to the wire code MAIN_THREAD_TIMEOUT in commandFailedResponse(from:), so the error response carries the typed signal. The host catch sets runnerMainThreadBusy via isRunnerMainThreadOccupiedError, which matches RUNNER_BUSY or MAIN_THREAD_TIMEOUT. A stalled snapshot → close → open now recovers without needing an intervening RUNNER_BUSY.

2. Flag clears once the runner drains, even on an error. The catch clears runnerMainThreadBusy for any served reply that is not occupied (e.g. element-not-found), because reaching the main thread proves the drain. Transport-shaped failures and unstamped recovered responses leave the prior state intact — only a served runner reply mutates it. close no longer kills an already-drained runner.

3. scheduleIosRunnerIdleStop is pure scheduling again. It no longer starts a disposal it does not wait for. finalizeAppleApplicationClose now asks the new stopRunnerSessionIfBusy(deviceId) seam (exposed through the runner operations facade and the appleApplications bridge) and takes its existing awaited stop branch when occupied, before the retain-vs-stop decision. One disposal path, awaited, lease released before the next request.

4. Production close route is covered. lifecycle.test.ts drives finalizeApplicationClose: retain+busy → stopped and not retained; retain+idle → schedules the idle stop; non-retain → stops directly. runner-session-close.test.ts also asserts that after stopIosRunnerSessionIfBusy disposes a busy runner, the next ensureRunnerSession returns a new session id.

Still no live 2–3 min wedge repro on demand; deterministic host + Swift tests carry the coverage. Validation: full test:unit green (10071 passed, 2 fs-slow flakes pass in isolation), check:affected gates green, check:xctest-selection + check:packaged-runner-swift green, macOS runner host lane TEST BUILD SUCCEEDED (executes on the CI host lane).

[thymikee]

Verified at 44f78a986d, all four points:

1. Occupancy is now keyed on typed codes, not on one code: RUNNER_BUSY and the watchdog's MAIN_THREAD_TIMEOUT both mark the session busy (runner-session.ts:781-788), so the command that stalls and gets tagged by the watchdog is the same evidence the refusal gives.
2. The flag drains on evidence instead of on hope: a served reply stamped not-busy clears it, and an unstamped reply leaves a busy report intact rather than inventing a drain it did not observe.
3. close decides atomically. stopIosRunnerSessionIfBusy returns whether it stopped the runner and short-circuits before touching anything when the session is idle, so the retain-vs-stop choice has no re-check window against scheduleIosRunnerIdleStop.
4. The behaviour is pinned through the production route: runner-session-close.test.ts covers the watchdog tag, the drain-clear, and ensureRunnerSession → busy stop → reopened session booting fresh, rather than asserting on the flag directly.

[thymikee]

Reviewed at 44f78a9. The runner now sends a typed MAIN_THREAD_TIMEOUT on the command that stalls, the host marks the session busy from it, an unstamped reply no longer clears the flag, and close stops a busy runner in one awaited step. That covers the points from the last review.

The tests do not yet prove that the steps chain. The close-finalization tests in lifecycle.test.ts#L247 cast the host with as unknown as PlatformRuntimeHost, repeat the same override three times, and only assert that mocks were called. The MAIN_THREAD_TIMEOUT test stops at the flag, and the stopIosRunnerSessionIfBusy test sets the flag by hand. If close stopped calling the probe on the simulator route, the suite would stay green. Could one test drive a command to MAIN_THREAD_TIMEOUT, run the real close finalization, then call ensureRunnerSession and assert a new session id? A local factory for the host override would remove the casts.

This change is device-facing, and the PR reports unit and build checks only. Please add a live run of the #2552 route on 44f78a9: a snapshot that trips the watchdog, then close, then open, with the next snapshot served by a fresh runner.

CI is green on 44f78a9, and there are no conflicts. The next step is the chained test and the live run.

[thymikee]

Follow-up refactor 029c9e97ed — same behavior, smaller design. The retain-vs-stop decision now lives in the runner module that owns the occupancy bit:

• close calls one intent, releaseRunnerOnClose(deviceId, { retain }), instead of the stopRunnerSessionIfBusy → boolean → re-branch dance in lifecycle. The busy fact never crosses the package boundary and the boolean protocol is gone.
• scheduleIosRunnerIdleStop stays pure scheduling; the close route is awaited so the lease is released before the next request.
• Net −33 lines. Fallow resolves releaseRunnerOnClose through the existing runner-operations facade exemption; the two dead facet methods (stopRunnerSessionIfBusy, scheduleRunnerIdleStop) and their plumbing are removed.

vitest-related 2912 green (420 files), fallow clean, format/lint/typecheck green, xctest-selection/packaged-runner-swift green.

[thymikee]

Reviewed at 029c9e9, as a follow-up to 44f78a9. The releaseIosRunnerOnClose refactor is a good, smaller shape, but the core fix still has no end-to-end test and no live run.

runner-session-close.test.ts#L290: the MAIN_THREAD_TIMEOUT test at L238 stops at the busy flag, and the release test sets runnerMainThreadBusy by hand. If the catch-side marking stopped reaching the release path, the suite would stay green. Can one test drive executeRunnerCommandWithSession to a real MAIN_THREAD_TIMEOUT error, call releaseIosRunnerOnClose(id, { retain: true }) without touching the flag, and assert that ensureRunnerSession returns a new session id? lifecycle.test.ts#L247-L293 has the same gap: it casts the host with as unknown as PlatformRuntimeHost and only checks that releaseRunnerOnClose was called.

The daemon test mocks copy the retain policy in three places (session-test-harness.ts#L52, session-relaunch-close.test.ts#L42, session-close-shutdown.fixtures.ts#L585), and the copies leave out the busy branch. If the real policy changes, those tests keep passing against stale behavior. Could they check only the call arguments, as session-relaunch-close.test.ts#L567 already does, or share one mock factory?

session-test-harness.ts#L474 also un-exports about nine unrelated mocks (mockPrewarmIosRunnerSession, mockPrepareIosRunner, mockResolveIosApp and others). Does this delta need that for fallow, or can it move to its own change so a sibling branch that imports them does not break?

Close finalization now waits for the runner module to stop a busy runner (runner-session.ts#L591), and the PR reports only unit, build and fallow checks. This device-facing route needs one live simulator run of #2552 on the current head: a snapshot that trips MAIN_THREAD_TIMEOUT, then close, then open, with the next snapshot served by a new runner session, plus an idle close and open that still reuses the warm runner.

Smoke Tests was still running at review time, not failed; it runs open and close flows, so its result applies to this change. There are no conflicts. The next step is the chained timeout-to-fresh-session test and the live #2552 run.

[thymikee]

All points from the review at 029c9e9 are addressed in 57ab7ad (test-only):

• The release test now trips a real MAIN_THREAD_TIMEOUT through executeRunnerCommandWithSession and never touches runnerMainThreadBusy. Deleting the catch-side busy marking makes it fail. The lifecycle gap is covered by test(apple-runner): pin the close release truth table at the lifecycle seam #2623, which drives the real finalizeApplicationClose over the real runner module.
• The three daemon mocks no longer copy the retain policy. The close tests assert releaseIosRunnerOnClose(id, { retain }) arguments instead.
• The unrelated session-test-harness.ts un-exports are reverted; they were not needed.
• Live run on a fresh iOS 26.2 simulator: an idle close keeps the same runner process (ios_runner_idle_stop_scheduled, then ios_runner_session_reuse). A 45 s longpress returns MAIN_THREAD_TIMEOUT at 30 s with the runner still alive, close logs ios_runner_retain_skipped_busy and the runner exits, and the reopen boots a new runner (ios_runner_session_startup, new pid).

Details are in the PR body.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-15 11:12 UTC