Skip to content

fix(ios): hold a canceled request's locks until its runner command ends - #3385

Merged
thymikee merged 3 commits into
mainfrom
fix/3383-canceled-runner-command-holds-lock
Oct 11, 2026
Merged

thymikee merged 3 commits into
mainfrom
fix/3383-canceled-runner-command-holds-lock

Conversation

@thymikee

@thymikee thymikee commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Summary

On iOS, a request canceled with its signal now keeps the session and device locks until the runner reports the command finished. Before, open --relaunch or a screenshot could run while XCTest was still running the canceled command. The client still rejects at once.

  • runner-exchange.ts: after a typed request_canceled abort of a sent command, the exchange polls status/statusCommandId until the journal shows the command as terminal, then settles the charge. The polls are bounded by the command's remaining deadline, not the aborted signal. A deadline, notAccepted, or a status failure ends the drain and leaves the charge abandoned. A cancellation the transport proves unsent (dispatched: no) withdraws its charge and releases at once.
  • The journal vocabulary moves unchanged from recovery into runner-session-types.ts beside RunnerCommandAccounting; client-api.md documents the hold.

Closes #3383

Validation

Commit c8c44a01f: pnpm check:affected --run passed (3220 tests, incl. eager-closure budgets; live run below was on aa533890b). Coverage, Swift builds, and replay lanes are CI-owned; no Swift changed.

The new runner-exchange.test.ts cases cover the terminal drain, a second task waiting on the same keyed lock, abort before send, notAccepted, deadline expiry, and a caller-deadline negative case. With the fix removed, 5 of them fail.

Live on the iPhone 18 Pro simulator: a 5 s longPress aborted after 300 ms, then open --relaunch.

  • Base: simctl launch finished 16:08:24.3Z, while the runner longPress completed at 16:08:29.4Z (overlap).
  • Fix: the runner longPress completed at 16:09:17.48Z, the drain reported terminal at 17.50Z, and simctl launch ran 17.70–17.97Z.

Risk: a command the runner received but has not yet journaled reads as notAccepted and releases early. That window is milliseconds.

🤖 Generated with Claude Code

A request canceled through its signal released the session and device
locks as soon as the runner fetch aborted, while the XCTest runner kept
executing the command. A next request whose device work bypasses the
runner (simctl launch, privacy, screenshots) could overtake it.

The exchange now drains a canceled command it already sent: it polls the
runner journal with `status`/`statusCommandId` until the entry is
terminal, bounded by the command's own remaining deadline rather than the
aborted request signal. Terminal evidence settles the abandoned charge
through the shared journal settlement; a drain that runs out of deadline
leaves it abandoned. A request canceled before the send releases at once
without charging the runner. A caller's own deadline abort is unchanged.

Closes #3383

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-11 06:35 UTC

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.16 MB 5.16 MB +1.2 kB
Package (unpacked) 5.16 MB 5.16 MB +1.2 kB
Package (download) 1.55 MB 1.55 MB +406 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.6 ms 27.6 ms -0.0 ms
CLI --help 82.4 ms 82.7 ms +0.3 ms

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 6 files

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

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread website/docs/docs/client-api.md Outdated
Comment thread packages/platform-apple/src/runner/runner-exchange.ts
Comment thread packages/platform-apple/src/runner/runner-exchange.ts Outdated
thymikee and others added 2 commits October 10, 2026 18:38
A cancellation that landed during runner route resolution reached the
POST as an aborted fetch, so the exchange charged and drained a command
the runner never received. The transport now discloses `dispatched: no`
for a cancellation seen before the POST is issued, and the exchange
withdraws the charge of any provably unwritten send instead of marking it
abandoned or reading the journal for it.

Also qualifies the client docs on when a canceled iOS request's hold
ends, and bounds the test helper that waits for the fake runner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ypes

The journal states and terminal-evidence settlement moved to a new module
that the Apple facade entries now evaluated eagerly through
runner-lifecycle -> runner-command-recovery, growing their closures.
They live beside RunnerCommandAccounting in runner-session-types.ts,
which those entries already evaluate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready at c8c44a0. The change holds a canceled request's locks until its runner command ends, and I found no problem that should stop the merge. All 21 checks pass at this commit, and I know of no conflicts. Nothing blocks the merge. Not blocking, and you can take or leave these: (1) the read-only route in waitForRunner throws createRequestCanceledError() with no dispatched, even when no command was written yet, so isRunnerCommandProvablyUnwritten misses it and the exchange runs one status poll instead of withdrawing the charge; passing dispatched: commandMayHaveBeenWritten ? 'unknown' : 'no' there would fix it; (2) withdrawUnsent also matches ECONNREFUSED, so a refused connect that was not canceled now withdraws its charge, and the PR body only describes this for cancellations; (3) no test covers the status_unavailable exit in runner-exchange.test.ts, so changing the break at runner-exchange.ts:360 turns nothing red, and a case where the status script answers 'started' and then exits would cover it. On the open threads, the P2 thread on the held-lock timing in the client API docs is fixed at this commit and can be resolved. The P2 thread on the abort-once test helper is also fixed at this commit and can be resolved. The P2 thread on holding the exchange was declined correctly, because the keyed lock is held by the handler promise and the client rejects on its own abort guard, so it can be resolved. The P2 thread on the network and usbmux routes is fixed for those routes, and the read-only gap left in it is note (1) above. I could not inspect the live-run logs, which appear only in the PR body and were taken on aa53389 rather than this commit. I did not run the tests or do a live run on a physical device, so the usbmux route is checked only for its pre-POST check. The fail-without-fix claims rest on my reading of the earlier code path.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 10, 2026
@thymikee
thymikee merged commit 116cba7 into main Oct 11, 2026
21 checks passed
@thymikee
thymikee deleted the fix/3383-canceled-runner-command-holds-lock branch October 11, 2026 06:35
thymikee added a commit that referenced this pull request Oct 11, 2026
…en (#3409)

* fix(ios): disclose whether a canceled read-only runner send was written

Follow-up to #3385. The read-only route (waitForRunner) threw a bare
canceled error, so a cancellation that landed before any connect attempt
was charged and drained with one status poll instead of withdrawn. It now
discloses `dispatched: no` when no attempt could have written the
command, and `unknown` once an attempt was interrupted mid-flight or an
earlier attempt may have written it.

Pins two exchange behaviors with tests: a refused connection withdraws
its charge whether or not the request was canceled (the runner never
received the command), and a drain whose status read fails stops at once
with the charge left abandoned.

Refs #3383

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(ios): keep dispatched no when a canceled runner route proves nothing was written

Each route of the read-only connect loop replaced its failure with a bare
canceled error, so a cancellation whose own failure proved nothing was
written (a usbmux connect still in progress, a refused fetch, a simulator
curl that could not connect) reported `dispatched: unknown`. Routes now
keep `no` when that failure is the proof, and the final simulator post
combines the loop's write evidence with its own failure the same way.
An abort that interrupts a POST, or the request's shared abort reason,
still reads as possibly written.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS: a canceled request releases the session lock while its runner command still runs on the device

1 participant