Repository navigation
fix: upload pasted images in preserved-ended remote PTY panels - #7046
rocker-zhang wants to merge 1 commit into
Conversation
After a cmux ssh session ends with preserveAfterTerminalExit, the panel stays alive for reconnect. resolvedImageTransferTarget() only checked isRemoteTerminalSurface(), which returns false once the surface is removed from activeRemoteTerminalSurfaceIds by markRemotePTYAttachEnded. It then fell through to TerminalSSHSessionDetector, which looks for ssh or et as the foreground TTY process. cmux ssh-pty-attach is the actual foreground process in persistent SSH PTY sessions and is not recognized, so the detector returned nil and the paste inserted a local macOS path that the remote host cannot access. Add isPreservedEndedRemoteTerminalSurface() to check endedPersistentRemotePTYAttachSurfaceIds, and extend the guard in resolvedImageTransferTarget() to return .remote(.workspaceRemote) for these surfaces. The set already has correct lifecycle management: it is populated by markRemotePTYAttachEnded (when preserveAfterTerminalExit) and cleared on disconnect, re-attach, discard, failure, and panel destruction. Fixes the regression introduced in manaflow-ai#4807.
|
@rocker-zhang is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new ChangesRemote image transfer for preserved ended surfaces
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 23 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (23 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 |
Greptile SummaryFixes a regression from #4807 where pasting an image into a
Confidence Score: 5/5Safe to merge — two-line change that closes a known regression in a well-bounded code path. The change is minimal: a new one-liner predicate on No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[resolvedImageTransferTarget] --> B{owningWorkspace?}
B -->|nil| Z[local]
B -->|workspace| C{isRemoteTerminalSurface?}
C -->|yes| R[remote workspaceRemote]
C -->|no| D{isPreservedEndedRemoteTerminalSurface? NEW}
D -->|yes| R
D -->|no| E{remoteTmuxController remoteUploadTarget?}
E -->|found| RF[remote target]
E -->|nil| F{TerminalSSHSessionDetector detect?}
F -->|found| RS[remote detectedSSH]
F -->|nil| Z
style D fill:#90EE90,stroke:#228B22
%%{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[resolvedImageTransferTarget] --> B{owningWorkspace?}
B -->|nil| Z[local]
B -->|workspace| C{isRemoteTerminalSurface?}
C -->|yes| R[remote workspaceRemote]
C -->|no| D{isPreservedEndedRemoteTerminalSurface? NEW}
D -->|yes| R
D -->|no| E{remoteTmuxController remoteUploadTarget?}
E -->|found| RF[remote target]
E -->|nil| F{TerminalSSHSessionDetector detect?}
F -->|found| RS[remote detectedSSH]
F -->|nil| Z
style D fill:#90EE90,stroke:#228B22
Reviews (1): Last reviewed commit: "fix: upload pasted images in preserved-e..." | Re-trigger Greptile |
teamleaderleo
added
area: remote
|
The native SSH path now uploads remotely, but the legacy preserved-ended PTY case remains. Before landing your fix, please comment exactly: |
|
I have read the CLA Document v2.2 and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
This branch has not been deployed
2 participants
Fixes #1660.
What
When a
cmux sshsession ends and the panel is kept alive for reconnect (preserveAfterTerminalExit), pasting an image inserts a local macOS temp path (e.g./var/folders/.../T/clipboard-DATE-UUID.png) instead of uploading the file to the remote host.Root cause
resolvedImageTransferTarget()checksisRemoteTerminalSurface(), which returns false oncemarkRemotePTYAttachEndedremoves the surface fromactiveRemoteTerminalSurfaceIds. The fallback path runsTerminalSSHSessionDetector, which looks forsshoretas the foreground TTY process. In persistent SSH PTY sessions the foreground process iscmux ssh-pty-attach, which the detector does not recognize, so it returns nil and the code falls through to.local.The regression was introduced in #4807, which added
preserveAfterTerminalExitandendedPersistentRemotePTYAttachSurfaceIdsbut did not updateresolvedImageTransferTarget()to handle the new state. Before #4807 the panel closed immediately on session end, so this code path was unreachable.Fix
Add
isPreservedEndedRemoteTerminalSurface()onWorkspacethat checksendedPersistentRemotePTYAttachSurfaceIds, and extend the guard inresolvedImageTransferTarget()to return.remote(.workspaceRemote)for these surfaces.The set already has correct lifecycle management — populated by
markRemotePTYAttachEnded(only whenpreserveAfterTerminalExit) and cleared on disconnect, re-attach, discard, failure, and panel destruction — so no additional cleanup is needed.Changes
Sources/Workspace.swift: addisPreservedEndedRemoteTerminalSurface(_ panelId:)Sources/TerminalImageTransfer.swift: extendresolvedImageTransferTarget()guardNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes image paste in preserved-ended remote PTY panels so images upload to the remote host instead of inserting a local temp path. Applies when a
cmux sshsession ends withpreserveAfterTerminalExit(fixes #1660).resolvedImageTransferTarget()to treat preserved-ended remote terminal surfaces as.remote(.workspaceRemote).Workspace.isPreservedEndedRemoteTerminalSurface(_:)usingendedPersistentRemotePTYAttachSurfaceIds; no lifecycle changes needed.Written for commit e9e097a. Summary will update on new commits.
Summary by CodeRabbit