Skip to content

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

Merged
thymikee merged 2 commits into
mainfrom
fix/runner-cancel-drain-followups
Oct 11, 2026
Merged

thymikee merged 2 commits into
mainfrom
fix/runner-cancel-drain-followups

Conversation

@thymikee

@thymikee thymikee commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Summary

Follow-up to #3385 (#3383) with the reviewer's three notes:

  • Read-only route cancellation (runner-startup-transport.ts): waitForRunner used to throw a bare canceled error. A cancellation before any connect attempt was therefore charged and drained with one status poll. It now discloses dispatched: 'no', and the exchange withdraws the charge. An interrupted attempt now counts as possibly written, so a send that was already in flight still drains. The reviewer's suggested one-liner alone would have missed that.
  • Refused connection withdraws its charge whether or not the request was canceled, because the runner never got the command. The doc comments now say so, and a test pins it.
  • status_unavailable exit: a test where status answers started and then the runner exits shows the drain stops at once, leaves the charge abandoned, and reports status_unavailable.

5 files touched, all in packages/platform-apple/src/runner.

Validation

Commit f1a6cfd70. pnpm check:affected --run passed (3182 tests). Each connect route keeps dispatched: no only when its own failure proves nothing was written. eager-closure-budgets.test.ts passed. An earlier run timed out in two interaction-contract tests while host load was about 220. Both pass alone and passed on the rerun.

Each new test failed with its fix reverted: the dispatch disclosure, the canceled-only withdraw, and the missing break.

No live device run: these are unit-level changes to error disclosure and charge accounting, with no new device path.

🤖 Generated with Claude Code

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>

@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 5 files

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

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

Comment thread packages/platform-apple/src/runner/runner-startup-transport.ts Outdated
Comment thread packages/platform-apple/src/runner/runner-startup-transport.ts Outdated
…ing 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>
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.16 MB 5.16 MB +246 B
Package (unpacked) 5.16 MB 5.16 MB +246 B
Package (download) 1.55 MB 1.55 MB +91 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.5 ms 26.1 ms -0.5 ms
CLI --help 82.1 ms 82.3 ms +0.3 ms

@thymikee

Copy link
Copy Markdown
Member Author

The code is ready at f1a6cfd. The change only touches the cancel branches of waitForRunner, so it reports whether a canceled read-only send was written without adding any device request. No conflicts. Smoke Tests is still running and has no captured failure, and a failure there would be unlikely to come from this diff, because an uncanceled run never reaches the changed branches. Smoke Tests must finish green on f1a6cfd before merge.

Not blocking, and you can take or leave these: the "drain whose status read fails" and "refused connection withdraws its charge" tests cover code already on main (the break at runner-exchange.ts:362 and the withdrawUnsent branch at :318), so one executeRunnerExchange test could cancel a read-only command during the retry delay and assert the charge is withdrawn with no status poll; the PR body names only the read-only route, but waitForRunner also serves the readiness preflight (runner-exchange.ts:425) and graceful shutdown (runner-disposal.ts:159), and a canceled preflight now surfaces "no" or "unknown" instead of undefined; and the "possibly written unless provably unwritten" rule lives in both setLastError and the new .catch in runner-startup-transport.ts (:103-108), so one local helper could own it.

The two cubic-dev-ai threads, #3409 (comment) (final simulator post) and #3409 (comment) (usbmux connect-phase "no"), no longer apply at this head, so please resolve them.

I read the code and the pre-change behavior at 6e80737. I did not run the tests and did not do a live run. I did not exercise the reachable window where a cancel lands in the retry delay after ECONNREFUSED while the runner starts.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 11, 2026
@thymikee
thymikee merged commit 37a5ad4 into main Oct 11, 2026
19 checks passed
@thymikee
thymikee deleted the fix/runner-cancel-drain-followups branch October 11, 2026 09:50
@github-actions

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

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.

1 participant