callstack / callstack/agent-device

fix(apple-runner): recovery decisions read typed rules; a connection deadline gets zero recovery today

Open
#2,524 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
TypeScript
Stars
4.6k
Forks
299
Avg merge
10h 14m
Merged PRs (30d)
537

Description

## Defects in `packages/platform-apple/src/runner/` (verified at `eefe37b51e`)

`runner-contract.ts:166-172` states that recovery decisions come from `RUNNER_ERROR_RULES`. Two private message-substring chains bypass it, both from `ad7b386444d` (2026-06-04): `isPrepareHealthTimeout` (`runner-lifecycle.ts:512-517`) and `isRunnerReadinessPreflightTimeout` (`:618-621`).

- **Zero recovery on a real deadline.** `Runner connection deadline exceeded` (`runner-startup-transport.ts:165/314/361`, wrapped with `runnerReadinessPreflightFailed`) matches neither `'timeout'`/`'timed out'` at `:618-620` nor any rule, so `executeRunnerCommand` (`:295-337`) rethrows without attempting a restart.
- **Explicit `retryable: false` is defeated.** `shouldRetryPrepareRunnerHealthFailure` (`:247-254`) ORs in `shouldRetryRunnerConnectError`, whose `connectRetry ?? true` default (`runner-contract.ts:297`) overrides rules such as `device_busy_connecting`.
- **Cache wipe fires on nearly any cached-artifact failure.** `shouldRecoverBadCachedRunnerArtifact` (`:501-510`) → `cleanRunnerDerivedArtifacts` → forced `xcodebuild` rebuild; the cost is a clean build, not the ~25 s the comment at `:269` suggests.
- **One timeout path is untyped.** `fetchWithTimeout` (`runner-transport.ts:82-91`) lets `AbortSignal.timeout` surface as a bare `DOMException`; `asAppError` wraps it with `details: undefined`, so `isCommandTimeoutError` can never match it. The substring chains are the only thing catching it today.

## Fix, in this order

1. In `fetchWithTimeout`, wrap the timeout rejection as `AppError('COMMAND_FAILED', …, { timeoutMs })` so it is typed. No production code matches on `DOMException`.
2. Add a typed timeout rule to `RUNNER_ERROR_RULES`; widen `RunnerErrorMatch['details']` from the closed union to a predicate so `runnerReadinessPreflightFailed` is expressible.
3. Delete both substring chains.
4. Replace `?? true`: add explicit `connectRetry: true` to the rules that mean it (`flagged_retriable`, `runner_connect_refused`, `fetch_failed`, `econnrefused`, `socket_hang_up`, `device_busy_connecting` stays false) and keep a documented `true` default for unmatched errors, because unmatched errors are the common case while the runner boots.
5. Narrow the cache-wipe predicate to the rule ids that indicate a bad artifact.

Keep the table in `runner-contract.ts`. Do **not** create a new module under `runner/`: `runner-lifecycle.ts` is in the static closure of seven Apple façades and `eager-closure-budgets` is strict no-growth. State-dependent predicates (cached-artifact presence, request-canceled) stay in `runner-lifecycle.ts` as composition over the table.

## Done when

Characterization tests first, on unmodified code: deadline-exceeded → today rethrows; `device_busy_connecting` → today retried. After the change both flip and every existing recovery test still passes. One live iOS simulator run of `open --relaunch` plus a forced connection deadline.

Contributor guide

Open the contributing guide

Research direction

Start with the characterization tests for runner-lifecycle.ts and runner-transport.ts, then read runner-contract.ts and the cited timeout and recovery entry points. Verify the current deadline and device_busy_connecting behavior first; done means both behaviors flip as specified, existing recovery tests pass, and an iOS simulator open --relaunch run survives a forced connection deadline.

Written by the indexing model from the issue text.

Assessment

Tech stack
ios, typescript
Domain
mobile, testing, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.