Skip to content

Fix stale SSH PTY resize: retry pty_resize delivery with bounded backoff - #6821

Closed
austinywang wants to merge 10 commits into
mainfrom
issue-6306-ssh-workspace-tui-resize-can-remain
Closed

austinywang wants to merge 10 commits into
mainfrom
issue-6306-ssh-workspace-tui-resize-can-remain

Conversation

@austinywang

@austinywang austinywang commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6306.

Problem

In SSH workspaces, a terminal TUI pane could keep a stale resize state: after a pane-width change the remote TUI no longer reconciled to the visible size, and only Reconnect Workspace restored the self-healing behavior. cmux refresh-surfaces did not reliably repair it.

cmux ssh-pty-attach forwards size changes to the remote PTY via workspace.remote.pty_resize over a bounded 50 ms RPC (SSHPTYResizeMonitor). When that send failed — e.g. it raced a stale or blocked remote-session control path and the bounded RPC timed out — drainPendingResizes recorded the pending size, resumed input waiters, and returned without scheduling any further delivery. The size was only re-attempted on the next SIGWINCH or input edge, so a settled pane (a TUI the user is just viewing) could keep a stale size indefinitely. Output kept flowing, so the terminal looked alive, but the remote PTY/TUI never reconciled until a manual reconnect rebuilt the session path — exactly the report in the issue.

Fix

Make resize delivery self-healing in CLI/SSHPTYResizeMonitor.swift:

  • On delivery failure, keep the latest desired size pending and schedule a backoff retry that re-drains it without waiting for an external event.
  • Backoff starts at 100 ms and doubles to a 2 s cap; it resets on the first successful send.
  • Input waiters still resume immediately on failure, so typing is never blocked behind a wedged resize (input-edge ordering stays bounded).
  • The retry task is cancelled on teardown, so retries are bounded by the attach connection's lifetime — the output read loop already cancels the monitor on bridge EOF.

This matches the issue's "small first fix … add coalesced retry around the resize send." The heavier "trigger a remote-session reconnect / PTY reattach after repeated failures" path is intentionally out of scope — it's riskier (can disrupt the live session) and the bounded-backoff retry already lets the common transient-failure case converge on its own.

Test

Two-commit red/green. The regression test (added to the already-wired CLISSHPTYResizeInputTests suite) runs the real ssh-pty-attach CLI against a mock daemon + bridge. It opens the PTY at the target size so the attach-time reconcile is the single forced resize, makes the daemon reject the first pty_resize, and asserts the helper re-delivers the same size automatically with no further SIGWINCH or input edge.

  • Commit 1 (test only) is red: without a self-driven retry the second delivery never arrives.
  • Commit 2 (fix) is green.

Localization

No user-facing strings added or changed (CLI internal control path only); no Localizable.xcstrings / web message updates required.

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Adds a bounded backoff retry for SSH PTY resize delivery so TUI panes don’t stick at a stale size in SSH workspaces. Fixes #6306 and removes the need to reconnect after a resize RPC timeout.

  • Bug Fixes

    • On failed workspace.remote.pty_resize, keep the latest size pending and retry with backoff (100ms doubling to 2s) via a single cancellable DispatchWorkItem timer; reset on success and cancel on teardown in CLI/SSHPTYResizeMonitor.swift.
    • Resume input waiters immediately on failure to keep typing responsive and preserve input-edge ordering.
    • Added a regression test in cmuxTests/CLISSHPTYResizeInputTests.swift that forces a failed first resize and asserts automatic re-delivery without a new SIGWINCH or input.
  • CI

    • Regenerated .github/swift-file-length-budget.tsv for the larger test file and tightened a few budgets.

Written for commit 105ab95. Summary will update on new commits.

Review in cubic

cmux and others added 2 commits June 26, 2026 00:45
…ivery

Covers #6306. SSH workspace TUI
panes can keep a stale size when a workspace.remote.pty_resize delivery fails
(e.g. it races a stale/blocked remote-session control path and the bounded RPC
times out). The attach helper records the pending size on failure but never
re-attempts delivery on its own — it only retries on the next SIGWINCH or input
edge — so the remote PTY/TUI stays frozen until a manual workspace reconnect.

The test drives a single forced resize (the attach-time reconcile) whose first
delivery the mock daemon rejects, then asserts the helper re-delivers the same
size automatically with no further SIGWINCH or input. This fails today: with no
self-driven retry, the second delivery never arrives.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes #6306.

`cmux ssh-pty-attach` forwards SIGWINCH/input-edge size changes to the remote
PTY via `workspace.remote.pty_resize` over a bounded (50ms) RPC. When that send
failed — e.g. it raced a stale or blocked remote-session control path and timed
out — `drainPendingResizes` recorded the pending size and returned, resuming
input waiters but scheduling no further delivery. The size was only re-attempted
on the *next* SIGWINCH or input edge, so a settled TUI pane could keep a stale
size indefinitely. Output kept flowing, so the terminal looked alive, but the
remote PTY/TUI never reconciled until a manual Reconnect Workspace rebuilt the
session path.

Add a self-driven retry: on delivery failure the monitor keeps the latest
desired size pending (input waiters still resume immediately, so typing is never
blocked behind a wedged resize) and schedules a backoff retry (100ms doubling to
a 2s cap) that re-drains the pending size without waiting for an external event.
The backoff resets on the first successful send, and the retry task is cancelled
on teardown — so retries are bounded by the attach connection's lifetime (the
output read loop cancels the monitor on bridge EOF). This keeps the scope to the
"coalesced retry around the resize send" the issue calls for; the heavier
remote-session reconnect path is intentionally left out.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jul 4, 2026 11:05pm
cmux-staging Building Building Preview, Comment Jul 4, 2026 11:05pm

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@austinywang, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5b721486-66e9-4954-94e4-3d81c989b06d

📥 Commits

Reviewing files that changed from the base of the PR and between 9c91710 and 105ab95.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (2)
  • CLI/SSHPTYResizeMonitor.swift
  • cmuxTests/CLISSHPTYResizeInputTests.swift
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-6306-ssh-workspace-tui-resize-can-remain

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.

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes stale SSH PTY resize state in SSHPTYResizeMonitor by adding a bounded-backoff retry for failed workspace.remote.pty_resize deliveries. Previously, a timed-out RPC would leave the remote PTY at a stale size until a SIGWINCH or input edge arrived; now a cancellable DispatchWorkItem timer re-drives the drain after 100 ms (doubling to a 2 s cap), input waiters are still resumed immediately on failure, and the retry is torn down cleanly on actor cancellation.

  • Production fix (CLI/SSHPTYResizeMonitor.swift): adds scheduleResizeRetry() / resetResizeRetry() / retryPendingResize() to the actor, using a one-shot cancellable DispatchWorkItem timer identical to the existing websocket-keepalive idiom; the retry is properly cancelled in markCancelled() and reset on any successful send.
  • Regression test (cmuxTests/CLISSHPTYResizeInputTests.swift): exercises the full ssh-pty-attach CLI against a mock daemon that rejects the first pty_resize and asserts the helper re-delivers the same size automatically, no SIGWINCH or input needed.
  • CI budget (.github/swift-file-length-budget.tsv): updated row for the now-larger test file.

Confidence Score: 5/5

Safe to merge — the change is actor-isolated, the retry timer is properly cancellable, and the regression test verifies end-to-end re-delivery without any external event.

The backoff state machine is straightforward: scheduleResizeRetry / resetResizeRetry / retryPendingResize are all actor-isolated, the DispatchWorkItem is stored and cancelled on every success and on teardown, and isCancelled guards the post-cancellation Task hop. Edge cases (timer fires during an active drain, SIGWINCH racing the retry window, multiple consecutive failures) are all handled correctly by the actor's serial execution and the isDraining guard in startDrainIfNeeded. The regression test provides real process-level coverage of the failure → retry → re-delivery path.

No files require special attention.

Important Files Changed

Filename Overview
CLI/SSHPTYResizeMonitor.swift Adds bounded-backoff retry logic via a cancellable DispatchWorkItem; actor isolation is correct, backoff state management and cancellation paths are sound.
cmuxTests/CLISSHPTYResizeInputTests.swift Adds a red/green regression test that forces a failed first resize and verifies automatic re-delivery; test-only NSLock usage is within established scaffolding norms.
.github/swift-file-length-budget.tsv Mechanically updated to reflect the enlarged test file; no logic changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SIGWINCH / input edge\nor retry timer fires] --> B[recordPendingResize /\nretryPendingResize]
    B --> C{isDraining?}
    C -- Yes --> D[Return — drain loop\nwill pick up new size]
    C -- No --> E[startDrainIfNeeded\nisDraining = true]
    E --> F[drainPendingResizes loop]
    F --> G{pendingSize set?}
    G -- No --> H[resumeInputWaiters\nisDraining = false\nreturn]
    G -- Yes --> I[sendResize via GCD\nbounded 50 ms RPC]
    I --> J{sent OK?}
    J -- Yes --> K[lastSentSize = size\nresetResizeRetry\nbackoff reset\ncancel retryWorkItem]
    K --> G
    J -- No --> L[pendingSize = size\nresumeInputWaiters immediately]
    L --> M[scheduleResizeRetry\nDispatchWorkItem asyncAfter backoff]
    M --> N[retryWorkItem stored\nbackoff doubled up to 2 s]
    N --> O[isDraining = false\nreturn]
    O -.->|timer fires| A
    P[markCancelled / cancel] --> Q[retryWorkItem.cancel\nretryWorkItem = nil\nisCancelled = true]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[SIGWINCH / input edge\nor retry timer fires] --> B[recordPendingResize /\nretryPendingResize]
    B --> C{isDraining?}
    C -- Yes --> D[Return — drain loop\nwill pick up new size]
    C -- No --> E[startDrainIfNeeded\nisDraining = true]
    E --> F[drainPendingResizes loop]
    F --> G{pendingSize set?}
    G -- No --> H[resumeInputWaiters\nisDraining = false\nreturn]
    G -- Yes --> I[sendResize via GCD\nbounded 50 ms RPC]
    I --> J{sent OK?}
    J -- Yes --> K[lastSentSize = size\nresetResizeRetry\nbackoff reset\ncancel retryWorkItem]
    K --> G
    J -- No --> L[pendingSize = size\nresumeInputWaiters immediately]
    L --> M[scheduleResizeRetry\nDispatchWorkItem asyncAfter backoff]
    M --> N[retryWorkItem stored\nbackoff doubled up to 2 s]
    N --> O[isDraining = false\nreturn]
    O -.->|timer fires| A
    P[markCancelled / cancel] --> Q[retryWorkItem.cancel\nretryWorkItem = nil\nisCancelled = true]
Loading

Reviews (9): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@blacksmith-sh

This comment has been minimized.

The new regression test grew cmuxTests/CLISSHPTYResizeInputTests.swift past its
recorded 560-line ceiling. Regenerated the budget with
`python3 scripts/swift_file_length_budget.py --write-budget` (the canonical
path; the TSV is never hand-edited). The same run also tightens a few unrelated
entries down to current actual sizes, which is exactly what the budget header
asks for ("Reduce counts as files shrink").

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread CLI/SSHPTYResizeMonitor.swift
cmux and others added 2 commits June 26, 2026 02:05
….sleep

Aziz concurrency policy disallows runtime `Task.sleep` (allowed in tests only;
runtime waits need real signals, callbacks, or state changes). Schedule the
backoff retry with a cancellable `DispatchWorkItem` via `asyncAfter` — the same
one-shot timer-callback idiom the CLI already uses for the websocket keepalive —
and hop back onto the actor from the callback. Behavior is unchanged: one
pending retry at a time, cancelled on teardown, backoff reset on success.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ace-tui-resize-can-remain

# Conflicts:
#	.github/swift-file-length-budget.tsv

This branch was successfully deployed

1 active deployment
Preview – cmux — 105ab959 Deployed Jul 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH workspace TUI resize can remain stale until workspace reconnect

3 participants