Repository navigation
ssh-tmux: mirror into current window by default (--new-window keeps dedicated window) - #7264
Conversation
|
@0xJord4n 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:
📝 WalkthroughWalkthroughThis PR makes Changesattach_here mirroring feature
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors)
✅ Passed checks (22 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 Summary
Confidence Score: 4/5Safe to merge for all Swift and CLI changes; one web localization gap in the docs page. The Swift, CLI, and socket-routing changes are well-structured and fully tested — attach paths handle all failure modes, close semantics are corrected across every path, and ControlCommandExecutionPolicy was already updated for remote.tmux.window. The only defect is in web/messages/: the new attachNewWindow paragraph key was added to English and Japanese but is absent from the other 18 locale files, so users of the docs site in those locales will see English text rather than a localized translation. The 18 web locale files (ar.json, bs.json, da.json, de.json, es.json, fr.json, it.json, km.json, ko.json, no.json, pl.json, pt-BR.json, ru.json, th.json, tr.json, uk.json, zh-CN.json, zh-TW.json) each need an attachNewWindow entry. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CLI as cmux CLI
participant Socket as Socket Worker
participant Ctrl as RemoteTmuxController
participant App as AppDelegate / TabManager
alt Default (current window)
CLI->>Socket: "remote.tmux.mirror {host, activate}"
Socket->>App: MainActor.run → remoteTmuxAttachWindowTarget
App-->>Socket: windowTarget (.contextualWindow / .explicitWindow)
Socket->>Ctrl: attachHost(windowTarget, activate)
Ctrl->>Ctrl: SSH preflight / discovery
Ctrl->>App: mirrorDiscoveredSessions → existing window
App-->>Ctrl: workspaceIds
Ctrl-->>Socket: .mirrored(windowId, workspaceIds)
Socket-->>CLI: "{mirrored:true, window_id, workspace_ids}"
else --new-window
CLI->>Socket: "remote.tmux.window {host, activate}"
Socket->>Ctrl: attachHost(.dedicatedNewWindow, activate)
Ctrl->>Ctrl: SSH preflight / discovery
Ctrl->>App: createMainWindow()
App-->>Ctrl: newWindowId + TabManager
Ctrl->>App: moveExistingMirrors → new window
Ctrl->>App: mirrorDiscoveredSessions → new window
Ctrl->>App: removeBootstrapTab
App-->>Ctrl: workspaceIds
Ctrl-->>Socket: .mirrored(newWindowId, workspaceIds)
Socket-->>CLI: "{mirrored:true, window_id, workspace_ids}"
end
Note over App: Tab/window/socket close → detachMirrorWorkspaceKeptOpenLocally (never kill-session)
%%{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 CLI as cmux CLI
participant Socket as Socket Worker
participant Ctrl as RemoteTmuxController
participant App as AppDelegate / TabManager
alt Default (current window)
CLI->>Socket: "remote.tmux.mirror {host, activate}"
Socket->>App: MainActor.run → remoteTmuxAttachWindowTarget
App-->>Socket: windowTarget (.contextualWindow / .explicitWindow)
Socket->>Ctrl: attachHost(windowTarget, activate)
Ctrl->>Ctrl: SSH preflight / discovery
Ctrl->>App: mirrorDiscoveredSessions → existing window
App-->>Ctrl: workspaceIds
Ctrl-->>Socket: .mirrored(windowId, workspaceIds)
Socket-->>CLI: "{mirrored:true, window_id, workspace_ids}"
else --new-window
CLI->>Socket: "remote.tmux.window {host, activate}"
Socket->>Ctrl: attachHost(.dedicatedNewWindow, activate)
Ctrl->>Ctrl: SSH preflight / discovery
Ctrl->>App: createMainWindow()
App-->>Ctrl: newWindowId + TabManager
Ctrl->>App: moveExistingMirrors → new window
Ctrl->>App: mirrorDiscoveredSessions → new window
Ctrl->>App: removeBootstrapTab
App-->>Ctrl: workspaceIds
Ctrl-->>Socket: .mirrored(newWindowId, workspaceIds)
Socket-->>CLI: "{mirrored:true, window_id, workspace_ids}"
end
Note over App: Tab/window/socket close → detachMirrorWorkspaceKeptOpenLocally (never kill-session)
Reviews (18): Last reviewed commit: "fix(remote-tmux): cover tab detach and r..." | 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: 4
🤖 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 `@docs/superpowers/specs/2026-07-03-ssh-tmux-current-window-design.md`:
- Around line 76-80: Use the underscore naming consistently for the new tmux
socket method: change the proposed `remote.tmux.attach-here` reference in the
`ssh-tmux` design to `remote.tmux.attach_here` so it matches the naming used
elsewhere and avoids a mismatched copy/paste target. Update any mention in the
spec around the `ssh-tmux` parser/default behavior and the new socket method
naming to use `attach_here` alongside the existing `remote.tmux.mirror`
reference.
In `@Sources/RemoteTmuxController.swift`:
- Line 456: The fallback in RemoteTmuxController’s reuse and success paths is
fabricating a fake window_id with UUID() when appDelegate.windowId(for:) returns
nil. Replace that fallback with explicit error handling in the affected flow so
the socket/API response never returns an ID that does not correspond to a real
window. Use the existing manager lookup logic around tabManagerFor(tabId:),
tabManagerFor(windowId:), and appDelegate.tabManager to surface the inconsistent
registration state instead of masking it, and apply the same change to both
occurrences.
In `@web/messages/en.json`:
- Around line 728-731: The `attachCli` help text reads like an exhaustive list
of supported flags but omits `--no-focus`, which is still valid for the
current-window attach flow. Update the `attachCli` message in the messages JSON
to either include `--no-focus` alongside `--port`, `--identity`, and
`--new-window`, or rephrase the sentence so it clearly presents those flags as
examples rather than an exhaustive list.
In `@web/messages/ja.json`:
- Around line 684-687: Update the attach option descriptions in the japanese
messages entry for attachCli/attachNewWindow so they either explicitly mention
--no-focus alongside --port, --identity, and --new-window, or clearly indicate
the list is only an example of supported flags. Keep the wording aligned with
the existing attach flow wording in attachIntro and attachSockets so users know
--no-focus still applies when attaching into the current window.
🪄 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: fe9c5d1b-5ed6-4b44-a8cb-fea6a6eed3c4
📒 Files selected for processing (14)
CLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxSSHTransport.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swiftcmuxTests/RemoteTmuxCapabilitiesTests.swiftdocs/superpowers/plans/2026-07-03-ssh-tmux-current-window.mddocs/superpowers/specs/2026-07-03-ssh-tmux-current-window-design.mdweb/app/[locale]/docs/remote-tmux/page.tsxweb/messages/en.jsonweb/messages/ja.json
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 14 files
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…doc fixes - RemoteTmuxController: replace '?? UUID()' window_id fallback with a thrown error at both the reuse and success paths (CodeRabbit) — never return a fabricated id the caller would treat as a live window; resolve windowId once. - web/messages en+ja: attachCli lists --no-focus alongside the other flags. - spec: attach-here → attach_here, Status Draft → Implemented (Greptile).
|
Addressed the bot review findings in
Build green, 61 unit tests pass (incl. the |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/RemoteTmuxController`+CurrentWindowMirror.swift:
- Around line 52-65: The current `firstMirroredWorkspaceId` selection can
accidentally use a workspace mirrored in a different `TabManager` because
`mirrorSession` returns a Bool that is ignored here. Update `mirrorSessions`/the
surrounding loop to only assign `firstMirroredWorkspaceId` when
`mirrorSession(host:sessionName:into:)` actually mirrors into the current
`manager`, and never read `sessionMirrors[key]?.mirroredWorkspaceId` unless that
mirror belongs to this manager. Keep `hasMirrorForHost` behavior unchanged, but
ensure the workspace chosen for `manager.selectWorkspace` comes from a session
mirrored into the same manager.
- Around line 18-27: The closure inside prepareMirrorAttach in
RemoteTmuxController+CurrentWindowMirror is capturing sessions before that local
is declared, causing a compile error. Update the call to
completeCurrentWindowMirrorOutcome to use the closure parameter that provides
the discovered sessions (for example, the argument passed into the
prepareMirrorAttach completion) instead of referencing the later sessions
binding, and keep the later let sessions assignment only for the value returned
after preflight.
🪄 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: b87101bc-dffe-442c-9d66-8fd5a1917aba
📒 Files selected for processing (10)
CLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftSources/RemoteTmuxController+CurrentWindowMirror.swiftSources/RemoteTmuxController+MirrorAttachPreflight.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxMirrorAttachPreflight.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxCapabilitiesTests.swift
|
When I introduced the ssh-tmux feature, the idea to keep them in the same window also sparkled in my head. In this case, you would loose a very important feature of working remotely: being able to create new remote workspaces (sessions). It's also not clear for me if you can cleanly disconnect from the remote session (without closing the workspaces, since that would trigger the remote tmux sessions to also be killed)? I actually liked the idea from this issue, under Ofcourse, this is not my call, by the maintainers call; it's just my opinion as someone that does uses this feature daily. But just to be clear: I love the idea, and I definitely would find it useful to have both local and remote sessions in the same window; but it would need to retain the 2 basic features: being able to detach from the remote; and being able to create more workspaces in the remote easily. |
…for kill Reproduces PR manaflow-ai#7264 review finding: closing a mirrored remote-tmux workspace marks its window to kill-session on the remote, so the ssh-tmux author's live session dies when they close the tab. Asserts markRemoteTmuxKillOnWindowCloseIfNeeded never marks a mirror for kill; RED against current kill-on-close behavior. Widens the mark seam from private to internal so the test can drive it directly (the marker is set-then-consumed synchronously inside the real close gesture).
Closing a mirrored remote-tmux workspace now DETACHES from the remote tmux session (leaving it alive on the server for resume) instead of running kill-session. Killing a live session is only ever an explicit disconnect action, never a side effect of closing a tab/window/quit. Addresses the ssh-tmux author's PR manaflow-ai#7264 review: with mirrors as plain workspaces in the current window, the natural close gesture was silently killing their session. - closeWorkspace routes mirrors to detachMirrorWorkspaceKeptOpenLocally. - markRemoteTmuxKillOnWindowCloseIfNeeded is a retained no-op, so the last-tab, window-close, and app-quit paths all fall through to the existing detach handlers. handleWorkspaceClosed + the kill-marker machinery stay dormant for a future explicit disconnect-host action. Turns the RED test from the prior commit GREEN. Verified live against a real tmux host (my-vps): mirrored 9 sessions into the current window, closed all mirror workspaces, every remote session survived (host session set unchanged).
|
@robertnisipeanu thanks for this — you're right on both counts, and the detach point is the one that actually mattered. Detach, not kill (fixed in
Verified live against a real tmux host: mirrored 9 sessions into the current window, closed every mirror workspace, and all 9 sessions survived (host session set unchanged). Added a RED/GREEN regression test ( Creating new remote sessions: agreed this is a genuine gap, not just a nice-to-have — and the #2673 "Sidebar UX example" direction (grouped So this PR now retains detach; new-session creation is the tracked follow-up. Ultimately the maintainers' call on sequencing, but I think this keeps the default experience whole in the way you're describing. |
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 `@Sources/TabManager.swift`:
- Around line 2222-2231: Update the stale caller comment in
closeWorkspacesWithConfirmation so it matches the current detach-on-close
behavior: it should no longer say closing every tab will kill the remote-tmux
session(s) on commit. Refer to the nearby
markRemoteTmuxKillOnWindowCloseIfNeeded no-op and the other updated comments in
this flow, and rewrite the note to describe that closing workspaces detaches
from the remote session rather than killing it.
🪄 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: 8f458788-fb63-432f-add0-26acddcaa314
📒 Files selected for processing (8)
CLI/cmux.swiftSources/AppDelegate.swiftSources/RemoteTmuxController+CurrentWindowMirror.swiftSources/RemoteTmuxController.swiftSources/TabManager.swiftSources/TerminalController+RemoteTmux.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxMirrorCloseDetachTests.swift
The close-every-tab path in closeWorkspacesWithConfirmation still said it kills the remote session on commit; it detaches like every other close path now. Aligns the last remaining stale comment (coderabbit review on manaflow-ai#7264).
|
@0xJord4n not sure this is a step in the right-direction either, because now detach is a possibility, but a synced close is not possible anymore. So while now detaching is possible again, closing & cleaning up sessions/tabs on the remote is degraded (not possible anymore). |
|
@robertnisipeanu agreed — swinging from "always kill" to "always detach" trades one gap for the other. The close gesture is simply ambiguous: it can't express both "get this off my screen" and "I'm done with this session." I think the two verbs need to be separate, like tmux itself (
That would restore synced close without making it the default blast radius — and it's a small diff since the machinery is already in place. Does that shape work for you? If so I'll add it to this PR. |
Three fixes for the local tmux mirror, all live-verified against a real
tmux 3.5a server plus unit/e2e coverage:
1. New tab / session working directory (was: filesystem root). cmux's own
process cwd is `/` when launched from Finder, so a `new-window` or
`new-session` it sent without `-c` inherited `/` as the start dir and
stranded new tabs in the root. `newWindowCommand` now falls back to
`-c '#{pane_current_path}'` (the target window's own active-pane dir,
expanded server-side) when no concrete cwd is known, and a session cmux
creates on the LOCAL server passes `-c $HOME`. SSH sessions are
unchanged (a remote `new-session` already starts in the login home).
2. Focus-gated size authority. While the cmux app is not frontmost, each
mirror connection sends `refresh-client -f ignore-size` so a
co-attached real terminal drives the window size (no filler border in
that terminal); when cmux returns to the foreground it clears the flag
(`!ignore-size`) and re-imposes its own grid. `ignore-size` excludes the
control client from window-size calculation, so a cmux-sole-client
window simply holds its size (no balloon, measured on 3.5a). State is
kept across reconnects (reseed re-applies) and applied to connections
that attach while backgrounded. Driven by NSApplication become/resign
active in RemoteTmuxController.
3. Closing a mirror workspace kills its tmux session (two-way close sync).
For a LOCAL mirror, an explicit workspace close now routes to
handleWorkspaceClosed (kill-session) instead of detach — cmux is the
tmux UI, so closing the workspace closes the session, symmetric with
auto-mirroring a new session on open. handleWorkspaceClosed only kills a
still-live session, so the tmux->cmux session-ended cleanup that also
routes through closeWorkspace detaches without a redundant kill (manaflow-ai#7364).
SSH mirrors keep detach-for-resume (PR manaflow-ai#7264); closing the whole cmux
window still detaches, so quitting never kills every session at once.
Tests: newWindowCommand cwd/placement assertions updated for the
`#{pane_current_path}` fallback; new live e2e
`sizeAuthorityReleaseTogglesIgnoreSizeFlag` asserts the flag toggles via
`list-clients`. 20 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pressing X on a mirrored session workspace looked like it closed the session, but only detached: the session kept running on the machine, and with no per-session kill anywhere in the UI, every new-session + close cycle leaked a live session (observed: auto-named sessions 27-33 piling up on cloudtop, each still running an agent). Explicit per-workspace closes (sidebar X, tab X, Cmd+W, context-menu Close, batch multi-close) now kill the session, the workspace-level analogue of the window-tab X that already kills its tmux window. The confirmation reads the REMOTE session's activity and names the running command; idle sessions close instantly. The kill rides the live control connection and vetoes the local close, so tmux's own %exit tears the workspace down; without a live connection the close degrades to the old detach. This deliberately narrows the blanket detach-on-close rule from the upstream PR manaflow-ai#7264 review to the non-explicit paths: window close, app quit, batch close-all via the window path, and non-interactive socket closes still detach, and the workspace context menu gains an explicit Detach (Keep Session Running) for the resume workflow. Localization audited: four new catalog strings (kill dialog title, both message forms, detach menu item) added in en, ja, ko, and uk; no other user-facing text changed.
3 participants
Summary
cmux ssh-tmux <host>now mirrors a remote host's tmux sessions as plain workspaces in the current window by default — no group, no dedicated new window. A--new-windowflag preserves the old dedicated-window behavior.Previously the command always opened a dedicated new window that quarantined the remote sessions away from your local workspaces. Now remote sessions sit alongside local ones in the same sidebar.
What changed
RemoteTmuxController.mirrorHostInCurrentWindow— hardened from the oldmirrorHost: auth-required handling, no-window fallback (creates a plain window), idempotent reuse,beginAttachguard, cancellation, window-raise on activate, and a fail-loud guard that throws (without discarding the window) when zero sessions mirror.remote.tmux.attach_here(registered at dispatch + worker allowlist + execution policy); retiredremote.tmux.mirror.cmux ssh-tmuxroutes to the current-window mirror by default;--new-windowroutes to the oldremote.tmux.windowpath verbatim.cli.help.ssh-tmuxand docs strings localized in English + Japanese.Behavior decisions
Testing
cmux-unitscheme). Includes a new guard assertingremote.tmux.attach_herestays dispatched and validates params before touching the network, and an updated capabilities assertion for the method rename.--new-windowopens a dedicated window; killing a remote session closes only its workspace while the window survives.Notes
Behind the existing
remoteTmuxbeta flag. No new keyboard shortcut. No schema/config changes.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
cmux ssh-tmux <host>now mirrors remote tmux sessions into your current window by default; use--new-windowfor a dedicated window. Addedremote.tmux.window; socket calls stay focus‑neutral unlessactivate: true, and closing a mirrored workspace detaches (never kills).New Features
--new-windowopens a dedicated window. Help updated and localized.remote.tmux.window(dedicated window) alongsideremote.tmux.mirror(current window). Focus is opt‑in; creates the window after SSH preflight, consolidates any existing mirrors, removes the bootstrap tab, and throws if no sessions mirror.Bug Fixes
window_id, and return a localized “could not create a new window” error; both mirror entry points validate params and fail early with structured errors.Written for commit f6b650e. Summary will update on new commits.
Summary by CodeRabbit
New Features
cmux ssh-tmuxnow mirrors remote tmux sessions into the current window by default.--new-windowto mirror into a dedicated new window (with updated focus/activation behavior).remote.tmux.attach_herefor current-window mirroring (and updated docs to reflect the default entry point).Bug Fixes
Documentation / Tests