Skip to content

Cancel stale mode 2031 retries across resubscribe - #8214

Merged
nwparker merged 1 commit into
mainfrom
nwparker/fix-terminal-mode2031-retry-generation
Jul 11, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/fix-terminal-mode2031-retry-generation

Conversation

@nwparker

Copy link
Copy Markdown
Contributor

Description

Follow-up to #8206. A mode-2031 subscription can arrive before its PTY transport connects, so Orca retries the initial color-scheme reply up to eight times. #8206 stops a retry after a plain unsubscribe, but this sequence still duplicated the reply:

subscribe → queue retry A → unsubscribe → resubscribe → queue retry B → connect

Both A and B saw a currently subscribed pane and answered the newer subscription. A program can consume one reply and leave the duplicate to leak into its next input owner.

This PR gives every subscription attempt an opaque ownership token. A delayed retry sends only if its token still owns that pane. Starting a later subscription invalidates all earlier retries, and pane teardown deletes the token.

Evidence of fix

The deterministic regression test was added before production code. On merged main:

FAIL does not let an old retry answer a later subscription
expected sendInputImmediate 1 time, received 2 times

After the fix:

pnpm exec vitest run --config config/vitest.config.ts \
  src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.test.ts \
  src/renderer/src/components/terminal-pane/terminal-appearance.test.ts \
  src/renderer/src/components/terminal-pane/remote-runtime-pty-query-reply-immediate.test.ts
Test Files  3 passed (3)
Tests       29 passed (29)

The test queues retries for two subscription generations, connects the transport, executes both callbacks, and proves exactly one immediate reply and one recorded mode.

Additional checks passed:

  • pnpm typecheck:web
  • changed-file oxlint
  • pnpm check:max-lines-ratchet
  • pnpm check:reliability-gates

ELI5

Imagine two people take numbered tickets to answer the same terminal question. The first person leaves and a second person asks again. Previously, both old and new tickets could answer once the connection opened. Orca now accepts only the newest ticket, so the old delayed answer cannot spill into the terminal as random text.

User-facing trade-offs

  • Rapid unsubscribe/resubscribe cycles during terminal connection should receive one color-scheme reply instead of duplicates.
  • A superseded retry is intentionally dropped even if it fires first; the current subscription owns its own retry and still receives a reply.
  • Each active pane retains one symbol token, removed during existing pane teardown.
  • No retry interval/count, network call, provider API, terminal byte format, or UI setting changes.

Terminal reliability proof

  • Reliability invariant: terminal-capability.query-reply-generation — a delayed mode-2031 seed reply must belong to the current subscription generation, not merely a currently true subscription boolean.
  • Failure source: adversarial review of Fix delayed terminal color-scheme replies leaking into fish #8206 reproduced two replies for subscribe/unsubscribe/resubscribe before connection.
  • Product change type: runtime lifecycle hardening plus deterministic unit coverage.
  • Manifest status: accepted gap; intended future gate remains terminal-capability.mode-2031-reply-timeliness.
  • Oracle: two queued generations execute after connection but emit exactly one sendInputImmediate call.
  • Provider/platform coverage: generation ownership is renderer state above the provider boundary and applies equally to local, daemon, SSH, WSL, and remote-runtime transports. No OS, filesystem, shell, mobile/relay, or Git-provider code changes. Live SSH/Windows/Linux/mobile runs remain accepted gaps.
  • Performance budget: one symbol allocation and one bounded map entry per subscribing pane; the entry is replaced on resubscribe and deleted on teardown. No new timers, retries, scans, polling, IPC, or buffering.
  • Diagnostics: the focused test reports duplicate send and mode-record counts without logging terminal contents.
  • Promotion status: none; deterministic red/green proof exists but the gate is not registered or cross-platform promoted.
  • Rollback rule: revert or follow up if a current subscription misses its seed or pane-token entries survive teardown.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 735f9741-aa7a-4b3f-9783-31c51cf8933b

📥 Commits

Reviewing files that changed from the base of the PR and between 4df9457 and 5c911d9.

📒 Files selected for processing (3)
  • src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.test.ts
  • src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.ts
  • src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts

📝 Walkthrough

Walkthrough

Mode 2031 seed replies now receive an attempt-validation predicate and recheck it before sending. The terminal pane lifecycle stores a fresh symbol token for each pane attempt and removes tokens when panes close. Tests update the push harness to model attempt identity and verify that stale scheduled retries do not send duplicate input or record duplicate modes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description has useful details, but it doesn't follow the required template sections for Summary, Screenshots, Testing, AI Review Report, Security Audit, or Notes. Rewrite it to match the repo template and add each required section, including screenshots or 'No visual change', testing checklist, AI review, security audit, and notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely describes the main change: preventing stale mode 2031 retries after resubscribe.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant