Skip to content

test(ios): pin a read-only cancel in the connect retry delay - #3413

Merged
thymikee merged 1 commit into
mainfrom
test/runner-cancel-retry-window
Oct 11, 2026
Merged

thymikee merged 1 commit into
mainfrom
test/runner-cancel-retry-window

Conversation

@thymikee

@thymikee thymikee commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Summary

Follow-up to #3409, covering two of the reviewer's non-blocking notes.

  • Retry-window test (runner-exchange.test.ts): a read-only snapshot goes to a starting runner, the connect attempt is refused (ECONNREFUSED), and the request is canceled in the retry delay that follows. The exchange reports dispatched: 'no', withdraws the charge, and sends nothing after the single refused attempt, so there is no status poll. The test passed on the current code, so no bug here. It fails if the loop reports 'unknown' for that case, as main did before fix(ios): disclose whether a canceled read-only runner send was written #3409.
  • One helper (runner-startup-transport.ts): the rule that an attempt failure counts as possibly written unless it proves otherwise now lives in one local recordAttemptWriteEvidence, used by both setLastError and the cancellation .catch. No behavior change.

Scope note on #3409: waitForRunner's cancel classification also reaches the readiness preflight (runner-exchange.ts ~L425). A canceled preflight now carries dispatched: 'no' or 'unknown' instead of no value. Graceful shutdown (runner-disposal.ts ~L159) also calls waitForRunner, but it passes no signal and swallows every error, so nothing changes there.

2 files touched.

Validation

Commit 3db5e066d: pnpm check:affected --run passed (2960 tests, including fallow), and eager-closure-budgets.test.ts passed. The new test failed when the loop reported 'unknown'.

No live device run: these are unit-level changes (a test and a pure refactor), with no new device path.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

A read-only command sent to a starting runner retries refused connects.
A cancellation that lands in the retry delay after a refused attempt now
has a test at the exchange: the command is reported `dispatched: no`,
its charge is withdrawn, and no status poll is sent. The rule that an
attempt failure counts as possibly written unless it proves otherwise
gets one local helper in waitForRunner instead of two copies.

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.

No issues found across 2 files

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

@github-actions

Copy link
Copy Markdown
Contributor

Size Report

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

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.0 ms 25.7 ms -0.4 ms
CLI --help 74.5 ms 74.6 ms +0.1 ms

@thymikee

Copy link
Copy Markdown
Member Author

The new test at 3db5e06 looks good to me, and I found nothing to change. The recordAttemptWriteEvidence helper keeps the same write classification as before for refused, pre-POST and mid-POST cancels. The new test passes at this head and fails when the outer cancel goes back to the bare error from before #3409, so it pins the retry-delay case as described. All 19 checks pass and there are no conflicts. This is ready for a maintainer to decide.

@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 7e5ed4d into main Oct 11, 2026
19 checks passed
@thymikee
thymikee deleted the test/runner-cancel-retry-window branch October 11, 2026 17:29
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-11 17:30 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