Skip to content

refactor(apple-runner): own close retention behind one runner operation #2615

Description

@thymikee

refactor(apple-runner): own close retention behind one runner operation

Purpose and evidence

PR #2605 (44f78a986d51aeb0aab3e031700a96c2302e299b) fixes reuse of a busy runner. Application lifecycle now combines stopRunnerSessionIfBusy, scheduleRunnerIdleStop and stopRunnerSession, although the runner owns occupancy and disposal state. Adding the busy primitive also changes shared contracts, forwarding modules and four replay fixtures.

Evidence: application close orchestration, runner state owner, host contract.

Required behavior

Expose one awaited close-finalization operation through the existing runner host seam, conceptually finalizeRunnerClose(deviceId, { retainForReuse: boolean }): Promise<void>.

The application lifecycle supplies reuse intent; runner-session.ts chooses using its own occupancy state. Preserve this truth table:

Caller intent / state Required result
Retain + busy runner Await disposal and lease release; never schedule reuse
Retain + idle runner Schedule the existing idle stop; retain healthy warm reuse
Do not retain Await the existing unconditional stop
Retain + no in-memory runner Preserve existing idle-stop scheduling/cancellation behavior; do not create a runner
Do not retain + no in-memory runner Still await the existing stop path, including idle-timer cancellation and owned-lease cleanup
Daemon shutdown Preserve the separate shutdown/alert-cleanup path; do not introduce the ordinary close-finalization call

Preserve alert-dismissal order, request behavior, error propagation, idle-stop timing and all occupancy update semantics from #2605. Keep lifecycle's shutdown decision outside this operation. Do not return early merely because the session map has no runner: unconditional stop also owns durable lease cleanup under its existing locks.

Scope and exclusions

Runner session implementation, Apple lifecycle, current client/facade/host bindings, AppleApplicationTools, root composition and directly affected fixtures/tests. Retire the new busy-only public plumbing once unused. Keep unconditional stop wherever other callers still need it. Preserve ADR 0005 and ADR 0019's injected host and lazy loader; no direct lifecycle import of runner implementation, new hook bag, Swift protocol change or recovery-policy redesign. #2524 and #2475 own different recovery/readiness behavior.

Completion and validation

  • Characterize the truth table and call ordering on the fix(ios-runner): dispose a busy runner on close so open boots clean (#2552) #2605 result before changing the API, including owned-lease cleanup without an in-memory session, disposal rejection before alert dismissal, and a close/open sequence that cannot reuse a busy runner. Exercise an idle-retain decision with a queued occupancy/drain transition around the existing awaited busy check; preserve observable ordering without pinning arbitrary microtask counts or introducing broader locking/concurrency policy.
  • Drive tests through the real lifecycle-to-runner boundary where practical; retain independent runner tests for occupancy stamps, transport failures and disposal. Do not reduce proof to a mocked method being called.
  • The lifecycle supplies intent through one operation instead of orchestrating runner state checks. The busy-only operation and unused forwarding exports are removed; other consumers retain genuinely required primitives.
  • Demonstrate a temporary additional runner-local reuse condition needs no extra contract method, forwarding adapter or replay fixture edit. Remove it before publication.
  • pnpm check:affected --run passes; preserve existing eager-closure constraints and follow docs/agents/device-verification.md for a live close/open check on a verified Apple target. Report native/CI obligations separately; a nondeterministic live wedge is not required proof of this behavior-preserving API refactor.

Dependencies and readiness

Blocked by: #2605 landing. Re-audit the merged result and all primitive consumers first. Do not implement against pre-#2605 main. Stop if the proposed seam cannot preserve shutdown or lease ordering without moving application policy into the runner. This is a follow-up, not a blocker for #2605.

Effort: M. Risk: medium (shutdown, lease disposal, warm reuse). No existing issue found for this ownership change.

Activity

  1. thymikee commented on Sep 15, 2026

    @thymikee
    MemberAuthor

    Re-audited the #2605 head and implemented the remaining obligation in #2623 (stacked on #2605's branch, since this issue is blocked by it).

    Audit: the seam this issue asks for already landed on that head in 029c9e97 — finalizeApplicationClose now issues one awaited releaseRunnerOnClose(deviceId, { retain }), runner-session.ts decides using its own runnerMainThreadBusy, and the busy-only operation plus its forwarding exports are gone. So the finalizeRunnerClose(deviceId, { retainForReuse }) name is the only thing the merged shape differs on, and the issue calls that conceptual. What was still owed is the completion evidence.

    Added packages/platform-apple/src/runner/__tests__/runner-close-finalization.test.ts (8 cases): the real finalizeApplicationClose driving the real runner module, composed as the root's lazy tools compose it. Row -> case: idle retain keeps the same session and launches nothing; busy retain disposes, releases the lease and the reopen boots a second process; non-retain stops and cancels the idle stop a retain armed (proven by the absence of ios_runner_idle_stop under a 40 ms window); non-retain with no runner in memory still releases an owned lease; retain with no runner in memory starts and claims nothing; daemon shutdown never issues the release; disposal rejection propagates with dismissCloseAlerts unreached; and a drain that lands after close is issued (runner exit held open, occupancy cleared by a served reply) cannot return the runner to reuse. Busy is produced by a real RUNNER_BUSY refusal and cleared by a real stamped reply. Four dead scheduleIosRunnerIdleStop mock entries went with it.

    Discrimination (temporary edits, all reverted): busy check removed -> 3 fail; release no longer awaited -> 6; cleanupOwnedIosRunnerLease removed -> the lease case; cancelIosRunnerIdleStop removed -> the cancellation case.

    Runner-local extension proof: a temporary ready !== false reuse condition plus its own case touched exactly runner/runner-session.ts and the runner's test — no contract method, no forwarding adapter, no replay-fixture edit, typecheck and the close/replay/shutdown suites green unchanged. Removed before publishing.

    Gates: pnpm check:affected --run green on 455f9ec5c3 (the earlier failure was 8 unrelated 5 s timeouts at host load 60+ from sibling worktrees' gates; those files pass in isolation and the tier went green on re-run). No live close/open run: this diff changes no production line, so the wedge scenario and device lanes stay with #2605.

  2. thymikee commented on Sep 16, 2026

    @thymikee
    MemberAuthor

    Closing: every obligation is on main at 17eabc8fd1.

    Not needed as a separate PR: the refactor was absorbed into #2605 before merge, which was the better outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions