Skip to content

fix(web, mobile): a failed terminal resize is sent again - #32047

Open
juliusmarminge wants to merge 9 commits into
mainfrom
fix/issue-31013
Open

juliusmarminge wants to merge 9 commits into
mainfrom
fix/issue-31013

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

Problem

When a terminal.resize request fails while the WebSocket stays up, the client never sends the size again. The terminal surface only reports a grid when its measured size changes, and the resize result was dropped (reportFailure: false, result ignored). The PTY then keeps the old width indefinitely: stty size disagrees with the drawn pane, full-width output wraps, and spinners staircase, until the next drag to a new width. That matches the report: stty size printed 51 110 while the panel showed about 86 columns, and dragging again fixed it.

Reproduced on main (6577c6b), web dev server, right-panel bash terminal, a 100 ms spinner, divider dragged mid-run. With every resize succeeding the PTY follows the pane (41 68 → 41 57). With exactly one resize request rejected by the server while the socket stays connected, the pane draws 44–46 columns but stty size stays at the previous width (53/60 columns) and the full-width line wraps, still stale more than a minute later with nothing touched.

This closes the failed-request path. What made the request fail on the reporter's macOS desktop is not confirmed (a node-pty resize throw surfaces as TerminalResizeError); the reproduction injects the failure.

Change

After a resize request fails, and the failure is not an interruption, the client resends the terminal's current grid once:

  • Web and desktop, ThreadTerminalDrawer: on failure the drawer records the writable session generation and calls the surface's resendSize(), so the retry carries the current measured grid through the existing 150 ms debounce (resize commands are a latest-wins lane, so the failed size is never replayed over a newer one). The next report consumes the record; a retry's failure is not retried, and a record never carries over to another session generation.
  • Mobile, createTerminalSession in packages/client-runtime: sendResize(retry) calls itself once with retry = true after a non-interrupt failure, re-checking the attachment generation and operate access. No shared state survives a detach or reconnect.

Failures stay silent. A request that keeps failing is sent at most twice; the existing per-generation replay on reconnect still applies. No server, contract or settings change.

Scope and approval

Fixes #31013

Triaged via the automated issue flow; reproduced on main with one injected resize failure.

Verification

Focused tests, each failing on main first:

  • apps/web/src/components/ThreadTerminalDrawer.browser.test.tsx: resends the terminal size after a resize request fails (main: only one 57×41 resize reached the command) and resends a failed resize once in each writable session (a retry suppressed after the session stopped being writable, and the bound on persistent failure). 4/4 pass.
  • packages/client-runtime/src/state/terminal.test.ts: sends each failed resize again once, in every attachment generation (a failure after detach is not resent, generation 2's failure is resent once, its retry's failure is not, a new grid gets its own retry). 13/13 pass.
  • A mutation check removing the retry bound makes both suites fail (unbounded resend).
  • vp run check:changed: format, lint, knip, typecheck, tests and React compiler pass.

Real-app recordings, web dev server in Chrome 156 on Linux, 1440×900, DPR 2, dark theme, fresh database per side, same driver on both sides. Each recording first shows a successful drag (68 → 53 columns), then one drag narrower whose resize frame is rewritten in the page to cols: 0 so the server rejects it ("Expected a value greater than or equal to 1"), with the socket open throughout:

Before 6577c6b After db28804
Rejected 44×46 resize failure, no further resize sent failure, then a valid 44×46 resize 184 ms later, success
stty size after idle (+60 s) 46 53 in a 44-column pane; full-width line wraps 46 44; full-width line ends at the pane edge

The head adds only f6525a2 after the captured build, which swaps in the equivalent isAtomCommandInterrupted helper and edits tests. An independent rebuild at the head reproduced the recovery (retry 178 ms after the rejected send, PTY back at 44 columns). The recordings (as GIFs) are in a PR comment; the final before/after stills and a zero-threshold diff are in the reply to the CodeRabbit summary. The differences between the final stills are the probe output and wrapping, the completed spinner glyphs and the sidebar thread age.

Not verified: the reporter's natural failure on the macOS desktop app; mobile on a device (covered by the client-runtime test; no mobile rendering changed); the server path where resizeLocked ignores a resize from an unregistered viewer while another viewer has a size, which returns success and is not covered by this retry.

AI-assisted; verified with the tests and tools listed above.

The surface reports each settled grid once, and the drawer dropped the
resize command's result, so a failed request left the PTY at its old
width until the next drag. After a failure (not an interrupt), ask the
surface to resend its current grid once; a retry that also fails stops.

Fixes #31013
The shared terminal session dropped the resize command's result and only
sends again on a new grid or output generation, so a failed request left
the PTY at its old width. After a failure (not an interrupt), resend the
current grid once; a retry that also fails stops, and a success resets it.

Fixes #31013
…generation

The retry allowance was a shared flag, set even when the retry never went out
(the attachment had already dropped), so the next generation's first failed
resize was not retried. The retry is now an argument of the send, and the test
settles each resize itself instead of polling.
A retry that reached a non-writable session left the retried flag set, so the
next writable session's first failed resize was not retried. The pending retry
now belongs to the generation that failed and is consumed by the next report.
Drop fake timers the resend test does not need, and note why the
client-runtime test flushes microtasks.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 6, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Nov 6, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f7eb0e8

Macroscope's review found this PR approvable — Failed terminal resizes are retried once using the latest measured dimensions, while retaining session and write-access checks. The effect is limited to terminal sizing and bounded additional resize traffic.

Notes:

  • All code in this push has already been reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The client runtime retries eligible failed terminal resize commands once. The terminal drawer resends its measured size after a resize failure for a writable session generation. Tests cover retries, reconnects, and changed grid sizes.

Changes: Terminal resize retry flow

Layer / File(s) Summary
Client runtime resize retry
packages/client-runtime/src/state/terminal.ts, packages/client-runtime/src/state/terminal.test.ts
The runtime retries a failed resize command once unless the failure is an interruption. The retry uses the current grid size. Tests cover failures across attachment generations and after the grid size changes.
Drawer resend by writable generation
apps/web/src/components/ThreadTerminalDrawer.tsx, apps/web/src/components/ThreadTerminalDrawer.browser.test.tsx
After an eligible resize failure, the drawer resends the terminal surface’s measured size. It tracks retries by writable session generation. Browser tests verify request parameters and behavior across a reconnect.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: yash-singh1, only21mil

Merge Risk: Minimal · up to f7eb0

The retry addresses rejected resize requests without an identified merge-blocking issue. The original intermittent desktop failure remains unconfirmed.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly summarizes the main change: retrying a failed terminal resize in web and mobile.
Description check Passed The description covers the problem, change, scope and approval, and verification. It reports focused test results and limitations. It references recordings and screenshots in PR comments rather than e…
Linked Issues check Passed Issue [#31013] requires the PTY to recover its current width after a resize failure, so stty size matches the drawn terminal and output no longer wraps or garbles. In ThreadTerminalDrawer.tsx, a n…
Out of Scope Changes check Passed All changed production code implements the resize retry for [#31013]. The other changes add regression tests for that behavior. The reviewed diff shows no unrelated production changes.
Full details: Description check

Explanation

The description covers the problem, change, scope and approval, and verification. It reports focused test results and limitations. It references recordings and screenshots in PR comments rather than embedding or linking them in the description, and it does not name the AI model and harness; these omissions do not make the otherwise complete description fail.

Full details: Linked Issues check

Explanation

Issue [#31013] requires the PTY to recover its current width after a resize failure, so stty size matches the drawn terminal and output no longer wraps or garbles. In ThreadTerminalDrawer.tsx, a non-interruption failure triggers one resend of the surface’s current grid; the retry is limited to the writable session generation. In terminal.ts, mobile retries once after checking the current attachment and operate access. The web and client-runtime tests cover failed requests, retry bounds, and session or attachment changes. The reported injected-failure reproduction also shows the PTY width recovering.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between f6525a2 and f7eb0e8.

Files selected for processing: 1

  • apps/web/src/components/ThreadTerminalDrawer.browser.test.tsx

Limit details: You’ve used all 10 included reviews currently available.

Copy link
Copy Markdown
Member Author

On the inconclusive linked-issue check: this PR closes the path where a terminal.resize request fails while the connection stays up, which leaves the PTY at the old width until the next drag to a new width. That is the symptom in https://git.cafe/pingdotgg/t3code/issues/31013. On main (6577c6b), a single rejected resize in the web app kept stty size at 53 columns in a 44-column pane. With this branch, the client sends the current 44×46 grid once more about 150 ms after the failure, and stty size matches the pane.

What failed on the reporter's macOS desktop is not confirmed, as the PR body says. The server path that ignores a resize while returning success only applies when another viewer holds a size. The reporter had a single client, so that path is out of scope here.

Before 6577c6b618: one rejected resize, stty size stays 46 53 in a 44-column pane and the line wraps

After db28804cc9: the failed resize is resent once, stty size 46 44 and the line ends at the edge

Zero-threshold diff of the final stills, amplified 8x

Copy link
Copy Markdown
Member Author

Recordings for the Verification section: web dev server, Chrome 156 on Linux, 1440×900, DPR 2, dark theme, a fresh database per side, the same driver on both sides. Shown here as 4 fps GIFs scaled to 960 px.

Steps, the same on both sides:

  1. A 100 ms spinner runs while the divider is dragged once (68 → 53 columns). The resize succeeds.
  2. One outgoing terminal.resize frame is rewritten to cols: 0, so the server rejects it while the socket stays open. The spinner runs again and the divider is dragged narrower once (44 columns). Nothing is dragged or reconnected after that.
  3. The width probe (stty size and a line as wide as the PTY reports) runs again after about 40 s idle.
Before 6577c6b After db28804
Rejected 44×46 resize +29.9 s, no resize follows +25.6 s, valid 44×46 resize at +25.8 s (184 ms later), success
Probe after idle +78 s: 46 53, the line wraps +63 s: 46 44, the line ends at the edge

The only production change between db28804 and the head (f6525a2) swaps in the equivalent isAtomCommandInterrupted helper.

Before 6577c6b618: successful drag, then one rejected resize; the PTY stays at 53 columns

After db28804cc9: successful drag, then one rejected resize; the size is resent and the PTY matches 44 columns

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed labels Nov 6, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review November 10, 2026 21:18

Dismissing prior approval to re-evaluate f7eb0e8

Copy link
Copy Markdown
Member Author

Evidence

Both sides: real web app, Chromium/Linux, dark theme, 1440×900 CSS viewport, separate worktrees and fresh synthetic databases. Composer hidden solely to redact private metadata.

Run a spinner redrawing every 100 ms; drag narrower successfully to 57 columns; rewrite exactly one outgoing terminal.resize request to cols:0; drag narrower again to 47 columns; leave the divider untouched for four seconds; run stty size and print a PTY-width line. The same driver ran on both commits.

Before After
9cd7a04797 f7eb0e8e42
Before recording After recording
Before: PTY retains 57 columns and line wraps After: PTY matches 47 columns and line fits

Base sent no retry. Head sent one valid 47-column retry without another drag or reconnect. Final stills were pixel-identical across captures at least 500 ms apart; all captures were visually inspected.

Browser driver · Run instructions · Request traces

AI-assisted; verified with the tests and tools listed above.

Not shown:

  • Mobile shared-runtime resize retry was not exercised on a native device; evidence covers the controlled web reproduction.
  • Electron desktop was not exercised separately.
  • The intermittent natural macOS reproduction was not established; failure was injected through the requested browser-side WebSocket interceptor.

This branch has not been deployed

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

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Terminal keeps a stale width after dragging the chat/terminal divider, so full-width output wraps and spinners garble

1 participant