fix: propagate AbortSignal on cancellation to stop iOS runner promptly#194
fix: propagate AbortSignal on cancellation to stop iOS runner promptly#194
Conversation
… promptly When a client disconnects (socket close or HTTP connection drop), in-flight iOS runner operations now receive an AbortSignal that immediately interrupts pending HTTP fetches, retry delays, and retry loops—ensuring detached xcodebuild/runner processes are not driven after the parent workflow is canceled. Closes #193 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2eb4b1c093
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/daemon/http-server.ts
Outdated
| req.on('close', () => { | ||
| if (!res.writableFinished) { | ||
| markRequestCanceled(requestIdForCleanup); | ||
| } |
There was a problem hiding this comment.
Detect HTTP disconnect from response/socket, not request close
req.on('close') is triggered after the request body finishes reading even for normal successful HTTP calls, so this handler runs on healthy /rpc requests before res is finished. With the current !res.writableFinished check, that marks requestId canceled during ordinary processing, which can spuriously abort in-flight iOS runner work even when the client is still connected. The disconnect hook should be tied to response/socket termination semantics instead of request completion.
Useful? React with 👍 / 👎.
| shouldRetry: shouldRetryRunnerConnectError, | ||
| }, | ||
| { deadline, phase: 'ios_runner_connect' }, | ||
| { deadline, phase: 'ios_runner_connect', signal }, |
There was a problem hiding this comment.
Propagate cancellation instead of falling back after abort
Passing signal into retryWithPolicy introduces a cancellation error path here, but this function still catches all retry errors and then continues to simulator fallback/connect-error wrapping. When a client disconnect aborts the signal, waitForRunner can still execute postCommandViaSimulator or surface Runner did not accept connection instead of exiting immediately as canceled, which defeats prompt cancellation and produces misleading failures.
Useful? React with 👍 / 👎.
Summary
AbortSignalthrough the iOS runner request lifecycle so that client disconnection immediately interrupts in-flight HTTP fetches, retry delays, and retry loopsAbortControllerin both socket and HTTP server paths; abort on client disconnectreq.on('close')) and cancels the associated requestretryWithPolicyand its internalsleephelper now respond to abort signals, breaking out of retry/delay cycles immediatelyfetchWithTimeoutin runner transport now combines timeout and request-cancellation signalsCloses #193
Test plan
tsc --noEmit)npm run test:unit)🤖 Generated with Claude Code