Repository navigation
Fix stale cmux ssh pane resize: reconcile remote PTY size after arming SIGWINCH - #5989
Merged
austinywang merged 9 commits intoJun 19, 2026
Merged
Conversation
…esize) `cmux ssh-pty-attach` captures the terminal size once for the bridge handshake and opens the remote PTY at that size, then arms its SIGWINCH DispatchSource only afterward. The window between the handshake-size capture and arming the source spans the entire remote `pty.attach` round-trip, so it is wide. Any SIGWINCH delivered in that window hits SIGWINCH's default disposition (ignore) and is lost, and nothing reconciles afterward — so when the surface's final grid size lands during attach/reattach (the common case, since SwiftUI lays the surface out after the helper spawns), the remote PTY stays frozen at the handshake size forever, corrupting full-screen TUIs (roborev, claude, htop, …). Only a later manual resize would fire SIGWINCH and correct it. Fix: after arming the SIGWINCH source, push the current size once to reconcile any resize missed during the attach window. Extract the send into a shared `sendSSHPTYResize` helper used by both the SIGWINCH handler and the reconcile so both read the freshest size and serialize on the same lock. The reconcile is a no-op on the daemon when the size already matches. Verified on macOS 15 / M4 Pro (the affected config): with the fix, a freshly opened remote workspace terminal reports the correct `stty size` immediately on attach and across reconnects, with no manual resize. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@kylejcaron is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSIGWINCH PTY resize handling is refactored in ChangesPTY Resize Helper Consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryFixes a lost-resize race in
Confidence Score: 5/5Safe to merge — the change is a small, well-contained addition: one new nonisolated method and one call site, both consistent with the existing actor/AsyncStream design. The reconcile write goes through the same actor-serialized event pipeline as live SIGWINCH events. The bufferingNewest(1) buffer and the post-send size re-check in drainPendingResizes() prevent stale sizes from persisting. The bridge-ready guard means early-failure paths are never affected. Tests cover the new initial resize across all attach scenarios and verify token propagation. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant GUI as Ghostty / SwiftUI
participant CLI as ssh-pty-attach (CLI)
participant Mon as SSHPTYResizeMonitor (actor)
participant Daemon as cmuxd-remote
GUI->>CLI: spawn ssh-pty-attach
CLI->>Daemon: workspace.remote.pty_bridge (size S0)
Daemon-->>CLI: bridge port + token
CLI->>CLI: "TCP handshake (cols=S0, rows=S0)"
Daemon-->>CLI: ready + attachment_token
Note over GUI,CLI: SIGWINCH dead window (before PR fix, resize lost here)
GUI-->>CLI: (optional) SIGWINCH S1 lost
CLI->>Mon: init(initialSize: S0)
Note over Mon: arms DispatchSource SIGWINCH handler
CLI->>Mon: requestCurrentResize() [NEW]
Mon->>Mon: yield(size: current, force: true)
Mon->>Daemon: workspace.remote.pty_resize (S_current)
loop live resizes
GUI-->>CLI: SIGWINCH S2
CLI->>Mon: yield(size: S2, force: true)
Mon->>Daemon: workspace.remote.pty_resize (S2)
end
%%{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"}}}%%
sequenceDiagram
participant GUI as Ghostty / SwiftUI
participant CLI as ssh-pty-attach (CLI)
participant Mon as SSHPTYResizeMonitor (actor)
participant Daemon as cmuxd-remote
GUI->>CLI: spawn ssh-pty-attach
CLI->>Daemon: workspace.remote.pty_bridge (size S0)
Daemon-->>CLI: bridge port + token
CLI->>CLI: "TCP handshake (cols=S0, rows=S0)"
Daemon-->>CLI: ready + attachment_token
Note over GUI,CLI: SIGWINCH dead window (before PR fix, resize lost here)
GUI-->>CLI: (optional) SIGWINCH S1 lost
CLI->>Mon: init(initialSize: S0)
Note over Mon: arms DispatchSource SIGWINCH handler
CLI->>Mon: requestCurrentResize() [NEW]
Mon->>Mon: yield(size: current, force: true)
Mon->>Daemon: workspace.remote.pty_resize (S_current)
loop live resizes
GUI-->>CLI: SIGWINCH S2
CLI->>Mon: yield(size: S2, force: true)
Mon->>Daemon: workspace.remote.pty_resize (S2)
end
Reviews (7): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 10930-10933: sendSSHPTYResize currently calls
currentCLITerminalSize() before acquiring socketLock which allows samples from
startSSHPTYResizeSource and the post-attach reconcile to race and be serialized
out-of-order; fix by moving the call to currentCLITerminalSize() so it occurs
while socketLock is held (inside the critical section that includes
socketLock.lock()/defer { socketLock.unlock() }) inside sendSSHPTYResize,
ensuring the sampled size used in the params dictionary is the one protected by
the lock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1154f33d-b206-4cb3-8440-7653856046ca
📒 Files selected for processing (1)
CLI/cmux.swift
Address review: the ioctl(TIOCGWINSZ) was sampled before acquiring socketLock, so the SIGWINCH handler and the post-attach reconcile could sample different sizes and serialize only the sends — letting a stale sample win the lock last and overwrite a fresher size on the daemon, re-creating the frozen-PTY symptom. Move the sample inside the lock so it protects both the read and the send. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
Addressed in 0cfeda6. Moved the |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✨ |
Contributor
|
The fix in 0cfeda6 is correct and complete. Moving Tip: You can customize Greptile's behavior for this repo with |
…tach-race # Conflicts: # CLI/cmux.swift # cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
austinywang
added a commit
that referenced
this pull request
Jun 19, 2026
The #5989 merge added 7 lines to CLI/cmux.swift; regenerate the budget so the length check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
austinywang
added a commit
that referenced
this pull request
Jun 19, 2026
* Add regression for Claude restore cwd drift * Restore Claude agent hooks from launch cwd * fix: prefer claude hook tty surface for resume binding * fix: keep non-agent resume bindings unchanged when retargeting * chore: refresh swift file length budget * fix: keep invalid explicit claude surface from falling through * fix: document restore cwd snapshot fallback * test: cover mapped claude surface priority * fix: keep mapped claude session surface authoritative * test: move resume coverage to Swift Testing * test: separate ambient claude hook surface * test: keep claude hook session fixture current * test: cover Claude hook leaked workspace routing * fix: prefer Claude hook TTY routing before leaked env * fix: return Claude hook test server semaphore * fix: harden Claude hook restore routing * fix: localize Claude hook surface errors * fix: initialize shortcut test state before skip * test: rely on main shortcut skip handling * test: cover stale Claude TTY workspace binding * fix: ignore stale Claude TTY workspace bindings * test: cover zsh claude wrapper after user function override * fix: restore Claude wrapper after zsh startup overrides * fix: prefer Claude process binding for hook routing * fix: isolate Claude hook process probe * fix: connect Claude PID probe socket * fix: authenticate Claude PID probe socket * test: cover Claude resume self-heal for mismatched CLAUDE_CONFIG_DIR When cmux is launched with a foreign CLAUDE_CONFIG_DIR (e.g. the .app is opened from a terminal whose agent set one), a restored `claude --resume <id>` resumes against the wrong config root and reports "No conversation found", dropping the user to a bare shell (#6194). This test fails until the wrapper self-heals CLAUDE_CONFIG_DIR to the config root that actually holds the transcript. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: self-heal CLAUDE_CONFIG_DIR on Claude resume A restored `claude --resume <id>` only resolves a session under the current CLAUDE_CONFIG_DIR. When the cmux app inherits a foreign CLAUDE_CONFIG_DIR (e.g. the .app is opened by cmd-clicking a link from a terminal whose agent set one), it propagates that dir to every restored pane, so sessions created under a different config root resume against the wrong namespace and fail with "No conversation found" — leaving the user at a bare shell instead of their conversation (#6194). The wrapper now relocates CLAUDE_CONFIG_DIR to the config root that actually holds the transcript when resuming an explicit session id, and only when the current root lacks it (a correct resume is never repointed). Session ids are filename-token validated before any glob-walk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: cover Claude prompt resume text parsing * fix: stop Claude resume parsing at prompt text * fix: harden Claude resume restore targeting * fix: define Claude resume parser helper before passthrough * fix: tolerate Claude value flags before resume * test: cover Claude resume auth selection * fix: preserve Claude resume auth selection * fix: constrain Claude resume self-heal * fix: bound Claude resume resolution * fix: harden Claude resume wrapper resolution * fix: route Claude hooks past stale tty bindings * test: keep Claude resume expectations out of XCTest diff * Add stale Claude shell wrapper regression * Route Claude shell functions through shim resolver * chore: refresh Swift file length budget after origin/main merge The #5989 merge added 7 lines to CLI/cmux.swift; regenerate the budget so the length check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Closed
5 of 6 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5700
Problem
Resizing a terminal pane in a
cmux sshworkspace does not propagate to the remote PTY. The remote PTY stays frozen at its attach-time size, so full-screen TUIs (roborev, claude, htop, …) render at the wrong size and onlyCtrl+Lpartially redraws (at the wrong dimensions).Root cause
The GUI runs
cmux ssh-pty-attachas the surface's command in a local PTY, and the resize chain exists: ghostty resizes the local PTY →SIGWINCH→ CLI forwardsworkspace.remote.pty_resize→ app →pty.resizeoncmuxd-remote. The bug is a lost-resize race at attach time.In
CLI/cmux.swift(ssh-pty-attach), the CLI:SIGWINCHDispatchSourceonly afterward.The window between (1) and (2) spans the entire remote
pty.attachround-trip, so it is wide. AnySIGWINCHdelivered in that window hitsSIGWINCH's default disposition (ignore) and is lost, and nothing reconciles afterward. When the surface's final grid size lands during attach/reattach — the common case, since SwiftUI lays the surface out after the helper spawns — the remote PTY stays frozen at the handshake size until a later manual resize happens to fireSIGWINCH.Fix
After arming the
SIGWINCHsource, push the current size once to reconcile any resize missed during the attach window. The send logic is extracted into a sharedsendSSHPTYResizehelper used by both theSIGWINCHhandler and the reconcile, so both read the freshest size and serialize on the same lock. The reconcile is a no-op on the daemon when the size already matches.Verification
Verified on macOS 15 / M4 Pro (the affected config). With the fix, a freshly opened remote workspace terminal reports the correct
stty sizeimmediately on attach and across reconnects, with no manual resize; full-screen TUIs render correctly without nudging the window.Notes on testing
This is a
SIGWINCH-delivery timing race inside a monolithic syscall/socket flow; a deterministic automated test would require a full integration harness with a fake bridge + remote daemon, which is disproportionate to the one-line-of-behavior fix. It was instead verified empirically on the reporter's exact configuration as described above. Happy to add a harness-level test if maintainers prefer.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes a lost-resize race in
cmux sshso the remote PTY picks up the pane size right after attach and on reconnect. Full-screen TUIs now render at the correct size without manual nudges.Bug Fixes
SIGWINCH, issue one immediate reconcile viaSSHPTYResizeMonitor.requestCurrentResize().surface_idandattachment_tokenwhen present.workspace.remote.pty_resizeafter bridge ready and validatecols/rows,surface_id, andattachment_token; test harness now includesattachment_tokenin the bridge “ready”.Refactors
ssh-pty-attach;SSHPTYResizeMonitornow exposesrequestCurrentResize().Written for commit b84a0ee. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests