Skip to content

refactor(ios): runner error classification as data; recovery tested through the AppleRunnerProvider seam #1631

Description

@thymikee

From the 2026-08-06 architecture review (claims verified by independent adversarial passes; direction confirmed consistent with AGENTS.md's "Apple runner seams" guidance and the no-test-only-DI-seams gate).

Problem

Command-level runner lifecycle policy is already a clean declarative table (RUNNER_COMMAND_TRAIT_MANIFEST, runner-command-manifest.ts:10-45, satisfies-exhaustive, consumed via 4 named predicates). Error-level retry classification is not — it is four scattered implementations, all consulted from the single catch dispatch in executeRunnerCommand (runner-lifecycle.ts:296-336):

  1. isRetryableRunnerError (runner-contract.ts:132-144) — substring-match chain ('xcodebuild exited early', 'device is busy'+'connecting', 'fetch failed', 'econnrefused', 'socket hang up', …)
  2. shouldRetryRunnerConnectError (runner-contract.ts:162-171) — second, overlapping substring chain
  3. resolveRunnerFatalErrorReason (runner-session.ts:941-950) — separate AppError.code-keyed chain deciding session fatality
  4. an inline composite in runner-lifecycle.ts:296-302 (code === 'COMMAND_FAILED' && message.includes('Runner did not accept connection') && shouldRetryRunnerConnectError(...))

Testability is the sharper half:

  • runner-command-recovery.ts (437 LOC, the transport-error recovery policy) has no dedicated test file; its only importer is runner-lifecycle.ts.
  • runner-command-retry.test.ts (1,275 LOC) drives the facade but vi.mock-replaces ensureRunnerSession/executeRunnerCommandWithSession — one layer inside the module under test.
  • runner-session.test.ts (2,068 LOC) uses 5 separate vi.mock blocks (exec, host-process, tool-provider, runner-transport, runner-xctestrun) and simulates recycled-pid wedges (fix(ios): never signal a recycled runner pid from a stale lease #1621) by scripting readProcessStartTime mock returns.
  • A stable seam already exists — AppleRunnerProvider (runner-provider.ts:63, the feat(apple): injectable Apple runner transport seam for provider interactors #1389 injectable transport) with a recording fake in interactor-runner-provider.test.ts — but it is used only for routing verification. No recovery/retry/wedge test crosses it.
  • Three different stubbing strategies coexist (fetch mock, internal-function mocks, fake-binary-on-PATH).

A "what happens when a runner request times out" trace crosses 10–12 files.

Proposed deepening

  1. Error classification as data: one declarative table beside RUNNER_COMMAND_TRAIT_MANIFEST mapping error shapes → retryable / connect-retry / session-fatal(reason), replacing the four sites. AGENTS.md already directs: "For connect errors, retry policy, or command typing, start in runner-contract.ts."
  2. Recovery tested through the seam: a deterministic fake-runner adapter behind AppleRunnerProvider that scripts wedge/recycle/timeout/status-recovery sequences, so recovery suites drive the interface the daemon actually uses instead of mocking internals. Two adapters (HTTP/usbmux prod + deterministic fake) make the seam real.

Scope guards

  • ADR 0005 lifecycle semantics unchanged (readiness preflight skip rules, status-recovery before invalidation, dead-runner invalidation without graceful shutdown).
  • The trait manifest pattern is the template; don't invent a new registry shape.
  • Existing behavior of every classified error string/code preserved — this is a declaration move, not a policy change.

Acceptance criteria

  • One classification declaration; the four sites consume it (or are deleted)
  • runner-command-recovery.ts behavior covered by direct tests through AppleRunnerProvider with a scripted fake runner
  • Recycled-pid (fix(ios): never signal a recycled runner pid from a stale lease #1621) and status-recovery scenarios expressed as fake-runner scripts, not readProcessStartTime mock choreography
  • No new vi.mock of runner-internal functions in the new tests

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions