Skip to content

fix(ios-runner): a caller deadline no longer kills a starting runner - #2982

Open
okwasniewski wants to merge 5 commits into
callstack:mainfrom
okwasniewski:oskar/ios-runner-start-survives-caller-deadline
Open

okwasniewski wants to merge 5 commits into
callstack:mainfrom
okwasniewski:oskar/ios-runner-start-survives-caller-deadline

Conversation

@okwasniewski

@okwasniewski okwasniewski commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #2894.

A wait poll bounds each attempt with an abort whose reason is a TimeoutError (runWithinWaitDeadline). ensureRunnerSession merged that signal with the request cancellation and handed the result to the detached xcodebuild test-without-building launch, where exec kills the process tree on abort. So a short-timeout wait that landed during runner start killed the runner, the lifecycle invalidated the half-started session, and the retry paid the whole start again: on a slow host it never caught up.

This is option (b) from the issue: the start is budgeted by startupTimeoutMs only.

  • runner-contract.ts owns the typed distinction (a separate module grew the Apple facade eager-import closure, which the budget test rejects): isCallerDeadlineAbortReason, callerDeadlineExpired (moved from runner-client.ts), and resolveRunnerStartupSignal, which forwards the registered request signal and every non-deadline abort but drops a caller deadline.
  • ensureRunnerSession opens the startup budget on that signal. A client disconnect still kills the xctestrun build and launch.
  • executeRunnerCommand no longer invalidates a starting session when the abort was the caller's deadline; the session stays starting and the next request joins it through the existing reuse path. The error still carries readinessPhase: 'runner-start'.
  • RunnerSession.launchDeadline opens when the runner process is launched (the request's startupTimeoutMs, else the 45 s default). Every joiner measures readiness from that one clock, and the reuse check retires a starting session past it (ios_runner_session_invalidated, reason runner_launch_budget_exhausted) so the same request relaunches. A hung runner is given up once, not once per joiner.
  • ensureRunnerSession runs the lock-held start detached and races it against the caller's own signal. A wait whose deadline lands mid cold build returns on time; the build is neither killed nor repeated, and the next request queues on the session lock and joins the session it registers. The HTTP progress sink skips a response that already ended, since a detached start can emit progress after its caller returned.

Touched: 4 source files, 2 test files (1 new).

Validation

Tested commit e593f9463 (3f2dac8 plus the wire-compat ledger ack for the progress sink guard and a test trim for the file-size ratchet).

  • pnpm check:affected --run: passed on this head. The first push failed Coverage on eager-closure-budgets.test.ts (new module in the facade closure) and Smoke on an xcrun --show-sdk-version ETIMEDOUT probe before the test ran (ci(apple): characterize bounded xcrun stalls in iOS preflight and fold-helper gate #2940).
  • New tests: a TimeoutError abort during launch leaves the launch signal unaborted, the session starting, the lease retained, and a second command reuses it with one xcodebuild launch total; a plain abort and the registered request signal still kill the start; six unit cases on the signal helper. Reverting the lifecycle change fails the liveness assertion; reverting the session change fails the launch-signal assertion.
  • Live, iPhone 17 Pro / iOS 26.5 simulator, fresh state dir, CLI at 3f2dac8, --json --debug: open com.apple.Preferences, killed the runner, removed the cached xctestrun, then three wait text General 4000 back to back. fix: skill should work, even if the npm package is not installed #1 returned in 4181 ms with wait_readiness_exhausted / readinessPhase: runner-start while its request log shows runner_xctestrun_cache: missing_xctestrun then built_new, ios_runner_startup_ensure_xctestrun, ios_runner_startup_launch_xcodebuild; the xcodebuild test-without-building process only appeared 14 s in, so the deadline fired inside the build. Update README with correct GitHub link #2 (t+5 s) joined the same session (ios_runner_session_reuse, ios_runner_session_startup_timings) and found the text in 4085 ms. bug: --session flag does not take effect #3 and a later bug: fill on search does not always work #4 succeeded in about 240 ms. Across the run: one ensure_xctestrun, one launch_xcodebuild, three session_reuse, zero runner_startup_request_canceled, zero runner_launch_budget_exhausted.
  • Tests: a second deadline-bounded request joining a start past its launch budget sees the invalidation, a second xcodebuild launch and a new session; a deadline firing mid ensure_xctestrun rejects the caller while the build is still pending, leaves the build signal unaborted and lets the next request join with one build and one launch total. Against the previous head the second test hangs for the full vitest timeout.
  • Known bound: the pre-launch phase (cold xcodebuild build) is bounded only by xcodebuild finishing, a request cancel while a caller is attached, and daemon shutdown, same as every non-wait command before this PR. A default build budget for non-prepare starts is a follow-up if wanted.

Review in cubic

Copilot AI lite review requested due to automatic review settings September 25, 2026 15:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/platform-apple/src/runner/runner-request-deadline.ts Outdated
A wait poll bounds each attempt with a TimeoutError abort. That signal was
merged with the request cancellation and handed to the xcodebuild launch,
so a short wait timeout during runner start killed the runner and the
retry paid the whole start again. The start now reacts only to request
cancellation; a caller deadline leaves the session starting so the next
request joins it. Closes callstack#2894.
Copilot AI review requested due to automatic review settings September 25, 2026 16:04
@okwasniewski
okwasniewski force-pushed the oskar/ios-runner-start-survives-caller-deadline branch from c9f0b9a to 56d63cb Compare September 25, 2026 16:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…teners

The caller signal is filtered through its own controller and the registered
request signal is composed with AbortSignal.any, so a request that polls
many times does not accumulate abort listeners on its cancellation signal.
Copilot AI review requested due to automatic review settings September 25, 2026 16:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@thymikee

Copy link
Copy Markdown
Member

At eeac883 this still has two gaps that can hang a wait loop.

A started runner can hang: xcodebuild stays alive but /health never answers. A caller-deadline cancel now leaves the session in 'starting' instead of killing it, and resolveReusableRunnerSession (https://github.com/callstack/agent-device/blob/eeac883/packages/platform-apple/src/runner/runner-session.ts#L416) reuses any live 'starting' session. Each joiner gets a fresh readiness budget from readRunnerStartupTimeoutMs (https://github.com/callstack/agent-device/blob/eeac883/packages/platform-apple/src/runner/runner-lifecycle.ts#L291), and nothing records when the session actually launched. So a loop of short wait polls joins the same hung start, and every poll's deadline cancel skips invalidation — before this PR each poll killed the start and let the next one relaunch, so the loop could recover, but now it just returns wait_readiness_exhausted forever. Should the launch Deadline be stored on RunnerSession (https://github.com/callstack/agent-device/blob/eeac883/packages/platform-apple/src/runner/runner-lifecycle.ts#L318) when it enters 'starting', so every joining request measures readiness from that fixed launch time and invalidates the session once that budget is spent, whatever the caller's abort reason was? A test where a second deadline-bounded request joins a start already past its own budget would cover this.

Only prepare sets startupTimeoutMs and buildTimeoutMs (https://github.com/callstack/agent-device/blob/eeac883/packages/platform-apple/src/runner/runner-lifecycle.ts#L509); a plain wait sets neither. Before this PR the caller deadline was the only bound on the pre-launch phase — reuse probes, adoption, ensure_booted, device readiness, and the cold xcodebuild build-for-testing (timeoutMs undefined at runner-artifact.ts:~503). Now that deadline is filtered out and runWithinWaitDeadline (https://github.com/callstack/agent-device/blob/eeac883/packages/platform-apple/src/runner/runner-session.ts#L147) just awaits quiescence, so a deadline that fires during a cold build blocks wait until the build finishes, with no bound at all — on a cold cache wait text X 4000 could return well past 4 s, up to the 90 s client envelope whose policy is reset-daemon. The linked issue asked for the start to continue in the background or run on its own budget, not for the caller to sit through it. Can the pre-launch phase run detached under the session lock, so the caller races it against its own signal and the next request just joins the same start, or should non-prepare starts get a finite default startup/build budget with the overshoot documented in the wait help? A test where the deadline fires mid ensure_xctestrun would cover this.

The posted live run used a cached xctestrun (https://github.com/callstack/agent-device/blob/eeac883/packages/platform-apple/src/runner/runner-session.ts#L264), so it only exercised the launch/readiness phase, not the build-phase route this PR changes. Nothing shows yet what wait returns, or when, if its deadline fires during a cold build. The missing run: clear the runner xctestrun/derived-data cache, run open, then run wait text <x> 4000 three times, and report each call's wall-clock return time and error code plus the daemon-log counts of ios_runner_startup_launch_xcodebuild, ios_runner_session_reuse, and runner_startup_request_canceled.

Smoke Tests was still running at review time. This diff changes ensureRunnerSession and executeRunnerCommand, which the iOS smoke lane exercises through the XCTest runner, so a failure there can't be treated as unrelated. The PR body attributes the earlier Smoke failure to an xcrun --show-sdk-version ETIMEDOUT preflight (#2940), which is a different route from this diff.

No conflicts.

I did not reproduce the author's live run or run tests myself, and I did not check Node's AbortSignal.any listener lifecycle, so the no-accumulation claim is unverified. The 90 s reset-daemon envelope for wait comes from DEFAULT_TIMEOUT_POLICY; I did not confirm wait's own descriptor policy, so the envelope interaction described above is a likely read, not a confirmed one.

This isn't ready to merge: the detached start needs to be bounded by its own launch deadline, the caller deadline needs to stop waiting through an unbounded cold build, and then the cold-cache live run above needs to be posted.

A caller deadline no longer waits through the start: ensureRunnerSession runs
the lock-held start detached and races it against the caller signal, so a
short wait poll returns on time during a cold xctestrun build and the next
request joins the same start.

The session records a launchDeadline when it enters starting. Joiners measure
readiness from it instead of opening a fresh budget each, and the reuse check
retires a starting session past that budget so a runner that never answers
cannot be joined forever.

The HTTP progress sink skips a response that already ended, since a detached
start can emit progress after its caller returned.
Copilot AI review requested due to automatic review settings September 26, 2026 16:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@okwasniewski

Copy link
Copy Markdown
Contributor Author

[claude-fable-5-1] responding on behalf of Oskar

Both gaps confirmed and closed at 3f2dac8.

Launch budget: RunnerSession now carries launchDeadline, opened when the runner process is launched (the request's startupTimeoutMs, else the 45 s default). readRunnerStartupTimeoutMs returns what is left of that clock, so every joiner measures readiness from the launch, not from its own arrival. isRunnerSessionServing refuses a starting session past that deadline: it emits ios_runner_session_invalidated with reason runner_launch_budget_exhausted, stops it, and the same request relaunches, whatever ended the earlier joiners' waits. Test: a second deadline-bounded request joining a start past its budget observes the invalidation, a second xcodebuild launch and a new session.

Detached start: ensureRunnerSession runs the lock-held start detached and races it against the caller's own signal. A wait whose deadline lands mid ensure_xctestrun returns on time with wait_readiness_exhausted / runner-start; the build is neither killed nor repeated; the next request queues on the session lock and joins the session it registers. A cancelled request still kills the start through its own startup signal, so the race decides nothing about survival. Test: deadline fires inside the xctestrun build, the caller rejects while the build is still pending, the build signal is not aborted, the follow-up request joins with one build and one launch total. Against eeac883 that test hangs for the full timeout.

Detached over a default budget: no non-prepare build bound exists to reuse, 45 s is too short for a cold build, and a budget alone still leaves the caller waiting. Consequence: the pre-launch phase is bounded only by xcodebuild finishing, a request cancel while a caller is attached, and daemon shutdown, same as every non-wait command before this PR. Happy to add a default build budget for non-prepare starts as a follow-up if you want that closed here.

Cold-cache live run, iPhone 17 Pro simulator (iOS 26.5), CLI at 3f2dac8, --json --debug. open com.apple.Preferences, kill the runner, remove the cached xctestrun, then wait text General 4000 three times back to back and once more after the runner was up:

call wall result request diagnostics
#1 (t+0 s) 4181 ms wait_readiness_exhausted, readinessPhase: runner-start runner_xctestrun_cache missing_xctestrun then built_new, ios_runner_startup_ensure_xctestrun, ios_runner_startup_launch_xcodebuild
#2 (t+5 s) 4085 ms success ios_runner_session_reuse, ios_runner_session_startup_timings
#3 (t+9 s) 239 ms success ios_runner_session_reuse
#4 (t+34 s) 233 ms success ios_runner_session_reuse

The xcodebuild test-without-building process first appeared at t+14 s, so call #1's deadline fired inside the build, and the call still returned at its 4 s. Counts across the run: ios_runner_startup_ensure_xctestrun 1, ios_runner_startup_launch_xcodebuild 1, ios_runner_session_reuse 3, runner_startup_request_canceled 0, runner_launch_budget_exhausted 0.

Also: the HTTP progress sink now skips a response that already ended, since a detached start can emit "Starting XCTest runner..." after its caller returned. Wait docs updated.

Copilot AI review requested due to automatic review settings September 26, 2026 16:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 26, 2026 16:46
@okwasniewski
okwasniewski force-pushed the oskar/ios-runner-start-survives-caller-deadline branch from a9a17a7 to 1df8c24 Compare September 26, 2026 16:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 26, 2026 17:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@thymikee

Copy link
Copy Markdown
Member

Reviewed e593f94, a follow-up on eeac883. The shared launch clock closes the first gap, but the pre-launch phase is still unbounded for a plain wait.

A plain wait opens the lock-held start with no startup or build deadline (createRunnerPhaseBudget(undefined, …) at https://github.com/callstack/agent-device/blob/e593f94/packages/platform-apple/src/runner/runner-session.ts#L147). The only signal is the wait request's own controller. When the caller deadline fires during the reuse probe, ensure_booted, device readiness, or the unbounded build in ensure_xctestrun, raceRunnerStartAgainstCaller returns and the wait ends. Then clearRequestAbortRegistration deletes that controller without aborting it. After that, nothing can stop the start, because launchDeadline only begins after launchRunnerProcess. If a build or boot step wedges, it holds withRunnerSessionLock forever. Every later command on that device queues behind it, including close, stopIosRunnerSession and invalidateRunnerSession, and only a daemon restart recovers. A start that can outlive its caller needs a deadline it owns, from the reuse check through the build to launch. Could ensureRunnerSession open startupBudget from options.startupTimeoutMs, with a finite default sized for a cold build, and pass that default into ensureXctestrunArtifact's build budget? A test where the mocked build never resolves and the request registration is cleared, then asserting that the next request or stopIosRunnerSession settles within the default, would cover it. I found this by reading the code; I did not run it.

Does the posted cold run show a deadline landing inside ensure_xctestrun? Call #2 reports ios_runner_session_reuse at about t+5 s, so a session existed by then, but the note says xcodebuild test-without-building first appeared at t+14 s, and call #1 also lists ios_runner_startup_launch_xcodebuild. As posted, the run does not show whether #1's deadline hit the build (the new route) or readiness after launch (already covered at eeac883). A rerun with derived data cleared, with #1's return time next to the ensure_xctestrun start and end and the launch_xcodebuild time from ios_runner_session_startup_timings, would settle it. The fix above changes the start budget, so this run is needed again anyway.

The commands.md line that says a runner start "keeps going after the deadline on its own startup budget" is not yet true for a plain wait. It becomes true once the whole start is bounded.

All checks are green, including the iOS smoke lane that exercises the changed ensureRunnerSession and isRunnerSessionServing.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A wait timeout during runner start stops the runner, so the retry pays the start again

3 participants