fix(ios-runner): a caller deadline no longer kills a starting runner - #2982
okwasniewski wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
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.
c9f0b9a to
56d63cb
Compare
…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.
|
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 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 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 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 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.
|
[claude-fable-5-1] responding on behalf of Oskar Both gaps confirmed and closed at 3f2dac8. Launch budget: Detached start: 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,
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. |
a9a17a7 to
1df8c24
Compare
|
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 A plain Does the posted cold run show a deadline landing inside 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 All checks are green, including the iOS smoke lane that exercises the changed |
Summary
Closes #2894.
A
waitpoll bounds each attempt with an abort whose reason is aTimeoutError(runWithinWaitDeadline).ensureRunnerSessionmerged that signal with the request cancellation and handed the result to the detachedxcodebuild test-without-buildinglaunch, whereexeckills the process tree on abort. So a short-timeoutwaitthat 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
startupTimeoutMsonly.runner-contract.tsowns the typed distinction (a separate module grew the Apple facade eager-import closure, which the budget test rejects):isCallerDeadlineAbortReason,callerDeadlineExpired(moved fromrunner-client.ts), andresolveRunnerStartupSignal, which forwards the registered request signal and every non-deadline abort but drops a caller deadline.ensureRunnerSessionopens the startup budget on that signal. A client disconnect still kills the xctestrun build and launch.executeRunnerCommandno longer invalidates a starting session when the abort was the caller's deadline; the session staysstartingand the next request joins it through the existing reuse path. The error still carriesreadinessPhase: 'runner-start'.RunnerSession.launchDeadlineopens when the runner process is launched (the request'sstartupTimeoutMs, else the 45 s default). Every joiner measures readiness from that one clock, and the reuse check retires astartingsession past it (ios_runner_session_invalidated, reasonrunner_launch_budget_exhausted) so the same request relaunches. A hung runner is given up once, not once per joiner.ensureRunnerSessionruns 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 oneager-closure-budgets.test.ts(new module in the facade closure) and Smoke on anxcrun --show-sdk-versionETIMEDOUT probe before the test ran (ci(apple): characterize bounded xcrun stalls in iOS preflight and fold-helper gate #2940).TimeoutErrorabort during launch leaves the launch signal unaborted, the sessionstarting, 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.--json --debug:open com.apple.Preferences, killed the runner, removed the cached xctestrun, then threewait text General 4000back to back. fix: skill should work, even if the npm package is not installed #1 returned in 4181 ms withwait_readiness_exhausted/readinessPhase: runner-startwhile its request log showsrunner_xctestrun_cache: missing_xctestrunthenbuilt_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: oneensure_xctestrun, onelaunch_xcodebuild, threesession_reuse, zerorunner_startup_request_canceled, zerorunner_launch_budget_exhausted.ensure_xctestrunrejects 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.