Skip to content

Add local tmux support to the remote-tmux mirror with two-way sync - #8421

Open
naveengovind wants to merge 3 commits into
manaflow-ai:mainfrom
naveengovind:local-tmux-support
Open

naveengovind wants to merge 3 commits into
manaflow-ai:mainfrom
naveengovind:local-tmux-support

Conversation

@naveengovind

@naveengovind naveengovind commented Jul 18, 2026 •

Copy link
Copy Markdown

Summary

Makes the tmux -CC mirror work against the local machine's tmux server as a first-class endpoint — no SSH — reusing the entire existing control-mode stack (parser, control connection, session/window mirrors, sizing) unchanged. Sync is two-way exactly like a remote mirror: splits, closes, renames, reorders, and new windows made in cmux propagate to tmux, and tmux-side changes (including from a plain tmux client co-attached to the same session) appear in cmux.

How

  • RemoteTmuxHost gains a Kind (.ssh / .local) and a canonical .local value. The local endpoint keys its own connectionHash namespace (the literal "local", which can never collide with an SSH host's 16-hex digest — an ssh alias actually named local stays a distinct endpoint), skips all ControlMaster socket work, and builds direct tmux argv through the same resolver script the SSH path uses (so Homebrew/MacPorts tmux is found from a GUI app's bare PATH, and missing-tmux classification is identical).
  • RemoteTmuxSSHTransport runs local one-shots directly (/bin/sh resolver argv for tmux …, /usr/bin/env otherwise); ensureMasterReady / ssh -O check / ssh -O exit degenerate to no-ops, so RemoteTmuxController needed no local-vs-ssh branching.
  • RemoteTmuxControlConnection spawns the local tmux -CC client under a locally-allocated PTY (new RemoteTmuxLocalPTY): control mode requires a controlling tty (bare pipe → tcgetattr failed); ssh -tt supplies it remotely, cmux supplies it locally. The slave is made raw before launch (no echo/ONLCR can touch the control stream), the writer gets a dup of the master so reader/writer closes can't double-close, and stderr stays a separate pipe so failure classification keeps working. The child env strips TMUX/TMUX_PANE and guarantees a TERM.
  • Socket API: every remote.tmux.* method accepts local: true in place of host.
  • CLI: new cmux tmux [--no-focus] [--new-window] command (help localized in all 20 catalog locales).
  • Image paste into a local mirror inserts the macOS path directly instead of scp-uploading.
  • Docs: "Local tmux" section on the remote-tmux docs page (en/ja catalogs).

Verification

  • New cmuxTests/RemoteTmuxLocalEndpointTests (wired into the pbxproj): identity invariants, argv shapes, socket-param parsing, transport local exec, and a live end-to-end test that creates a session on an isolated (TMUX_TMPDIR) local server, attaches the real control connection under a local PTY, waits for %enter + topology publication, lands an out-of-band split-window in mirror state (tmux→cmux), and lands a control-stream split-window on the server (cmux→tmux). Skips silently when no tmux is installed.
  • All 5 tests green locally (cmux-unit scheme); tagged Debug build green.
  • App-level dogfood against a live lab server via the debug socket: cmux tmux mirrored the session; tmux-side split + rename-window appeared in cmux (pane surfaces + tab title); cmux-side new tab routed to new-window, workspace rename routed to rename-session; remote.tmux.detach left the session alive.

Localization audit

New user-facing strings: CLI help (cli.help.tmux) added to Resources/Localizable.xcstrings with translations for all 20 locales the catalog carries; docs page strings added to web/messages/en.json and web/messages/ja.json (the remote-tmux docs page is en/ja-only per remoteTmuxDocsLocales). No new shortcuts, settings rows, alerts, or error strings.

🤖 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 first‑class support for mirroring the local tmux server with full two‑way sync, plus auto‑mirroring of new sessions and routing “New Workspace” to tmux when a local mirror is selected. Also adds sane working directories, focus‑gated sizing, and close‑to‑kill for local sessions.

  • New Features

    • cmux tmux [--no-focus] [--new-window] mirrors local sessions; sync is two‑way.
    • Socket API: every remote.tmux.* method accepts local: true instead of host.
    • Auto‑mirror: %sessions-changed triggers a debounced re‑discovery that mirrors only new sessions (skips during an explicit attach).
    • “New Workspace” in a window with a selected local tmux mirror creates tmux new-session -d, mirrors it, and selects it; falls back to a plain workspace on failure.
    • New‑tab cwd: falls back to -c '#{pane_current_path}' when no concrete path is known; local “New Workspace” starts at $HOME.
    • Focus‑gated sizing: while the app isn’t frontmost, send refresh-client -f ignore-size; re‑assert on return and after reconnect.
    • Closing a local mirror workspace kills its tmux session; SSH mirrors still detach.
    • Image paste into a local mirror inserts the local file path (no upload).
    • Docs: added “Local tmux” section and localized CLI help.
  • Refactors

    • RemoteTmuxHost adds Kind (.ssh/.local) and a canonical .local endpoint with its own connectionHash; builds direct tmux argv via the shared resolver.
    • RemoteTmuxSSHTransport runs local commands directly; ControlMaster warmup/check/exit are no‑ops for local.
    • RemoteTmuxControlConnection spawns local tmux -CC under RemoteTmuxLocalPTY; strips TMUX/TMUX_PANE, ensures TERM; %sessions-changed now notifies observers for reconcile; reconnect re‑applies size authority.

Written for commit 5f2b6c3. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added support for running cmux tmux against a local tmux server (no SSH), including local bidirectional mirroring.
    • Extended remote tmux socket commands to support local: true targeting.
    • Improved mirrored session behavior with server-wide %sessions-changed reconciliation and local workspace routing.
  • Bug Fixes

    • Improved command suggestions to recognize tmux.
    • Prevented local mirror panes from using SSH-based upload target resolution.
  • Documentation

    • Added localized cmux tmux help text.
    • Updated remote tmux landing pages with “local tmux” usage and examples.

Make the tmux -CC mirror work against the local machine's tmux server as a
first-class endpoint, reusing the whole existing control-mode stack (parser,
connection, session/window mirrors, sizing) unchanged:

- RemoteTmuxHost gains a Kind (.ssh/.local) and a canonical `.local` value.
  The local endpoint keys its own connectionHash namespace ("local", which can
  never collide with an SSH host's 16-hex digest), skips ControlMaster socket
  work, and builds direct tmux argv through the shared resolver script.
- RemoteTmuxSSHTransport runs local commands directly (no ssh); the
  ControlMaster lifecycle (warmup, -O check, -O exit) degenerates to a no-op
  so the controller needs no local/ssh branching.
- RemoteTmuxControlConnection spawns the local `tmux -CC` client under a
  locally-allocated PTY (new RemoteTmuxLocalPTY: raw slave termios, dup'd
  master for the writer, CLOEXEC) because control mode requires a controlling
  tty — the local counterpart of `ssh -tt`. stderr stays a separate pipe so
  failure classification keeps working.
- The remote.tmux.* socket methods accept `local: true` in place of `host`;
  a string host named "local" remains an SSH alias.
- New `cmux tmux [--no-focus] [--new-window]` CLI command mirrors local
  sessions (localized help in all 20 catalog locales).
- Image paste into a local mirror inserts the local path directly instead of
  scp-uploading.
- Docs: Local tmux section on the remote-tmux page (en/ja).

Two-way sync comes from the existing mirror layer and is covered by a live
end-to-end test (RemoteTmuxLocalEndpointTests) that attaches a real local
tmux under an isolated TMUX_TMPDIR, observes an out-of-band split arrive
(tmux→cmux), and lands a control-stream split on the server (cmux→tmux).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds local tmux endpoint support with direct execution, PTY-backed tmux -CC control connections, CLI and socket integration, session reconciliation, workspace routing, sizing-state handling, end-to-end tests, and localized documentation.

Changes

Local tmux support

Layer / File(s) Summary
Endpoint model and direct transport
Sources/RemoteTmuxHost.swift, Sources/RemoteTmuxSSHTransport.swift
Adds local endpoint identity, direct tmux invocation, local command execution, and SSH ControlMaster no-ops.
PTY-backed control mode
Sources/RemoteTmuxLocalPTY.swift, Sources/RemoteTmuxControlConnection.swift
Allocates a raw PTY and launches local tmux -CC with sanitized environment handling.
Session reconciliation and workspace routing
Sources/RemoteTmuxConnectionObservers.swift, Sources/RemoteTmuxControlConnection+Observation.swift, Sources/RemoteTmuxController*.swift, Sources/AppDelegate.swift, Sources/Workspace.swift, Sources/TabManager.swift
Propagates %sessions-changed, reconciles newly discovered sessions, routes new workspaces to local tmux, and applies local-specific mirror lifecycle behavior.
Background sizing authority
Sources/RemoteTmuxControlConnection+Sizing.swift, Sources/RemoteTmuxControlConnection+Commands.swift, Sources/RemoteTmuxController.swift
Tracks released size authority and reapplies ignore-size behavior across connection and application state changes.
CLI and command integration
CLI/cmux.swift, CLI/CMUXCLI+CommandSuggestions.swift, Sources/TerminalController+RemoteTmux.swift, Sources/RemoteTmuxController+Decisions.swift, Resources/Localizable.xcstrings
Routes cmux tmux to local mode, validates local arguments, constructs {local: true} requests, and updates command generation and help text.
Validation and documentation
cmuxTests/RemoteTmuxLocalEndpointTests.swift, cmuxTests/RemoteTmuxMirrorNewTabPlacementTests.swift, cmuxTests/RemoteTmuxNewWindowCwdTests.swift, cmux.xcodeproj/project.pbxproj, web/app/[locale]/(landing)/docs/remote-tmux/page.tsx, web/messages/*.json
Adds endpoint, transport, control-mode, session, sizing, and command-generation tests, plus local tmux documentation and translations.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant TerminalController
  participant RemoteTmuxHost
  participant RemoteTmuxControlConnection
  participant tmux
  CLI->>TerminalController: request local tmux with local: true
  TerminalController->>RemoteTmuxHost: create local endpoint
  RemoteTmuxControlConnection->>RemoteTmuxHost: build local tmux -CC argv
  RemoteTmuxControlConnection->>tmux: launch through local PTY
  tmux-->>RemoteTmuxControlConnection: return control-mode updates
  RemoteTmuxControlConnection-->>TerminalController: synchronize local tmux state
Loading

Possibly related PRs

Suggested reviewers: austinywang, lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (5 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Swift Blocking Runtime ❌ Error Sources/RemoteTmuxController+Attach.swift adds a 300ms Task.sleep debounce in production code, which the rule flags. Replace the fixed sleep with a real tmux completion/signal or actor-driven queued reconcile on state transition, not elapsed time.
Cmux Swift Concurrency ❌ Error routeNewWorkspaceToLocalTmux launches an unowned Task { ... } for session creation; it isn’t stored or cancelled, matching the forbidden fire-and-forget pattern. Make the route helper async and await it from the AppKit action, or store/cancel the task with the workspace/window lifecycle.
Cmux Swift Package Boundaries ❌ Error RemoteTmuxHost/SSHTransport/ControlConnection/LocalPTY are reusable, headless domain code under Sources/; the rule says that belongs behind a SwiftPM package boundary. Move the core remote-tmux stack into a small package target (e.g. CmuxRemoteTmux) exposing RemoteTmuxHost and transport/control APIs; leave AppDelegate/TabManager routing as app glue.
Cmux User-Facing Error Privacy ❌ Error Local tmux failures now use RemoteTmuxError messages like “failed to launch SSH” and “cmux ssh-tmux…”, exposing SSH/remote details in a local endpoint. Use local-tmux wording for the new local path, and split missing-tmux/launch-failure messages so local errors never mention SSH or ssh-tmux.
Cmux Architecture Rethink ❌ Error Production %sessions-changed handling adds a 300ms sleep and a cached seen-set, creating a second owner for session membership and papering over races. Keep session membership owned by one discovery path; reconcile immediately from the current tmux snapshot on event delivery, without delayed sleep or a separate seen-set cache.
Docstring Coverage ⚠️ Warning Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (19 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The local-tmux additions stay in existing @MainActor controllers/UI types or the SSH actor; no new implicit-MainActor models or unisolated shared mutable Sendable refs.
Cmux Browser Automation Off-Main ✅ Passed PR only changes remote-tmux/local tmux code; no browser.* socket-worker/WebKit wait-routing paths were touched.
Cmux Expensive Synchronous Load ✅ Passed PASS: PR diff adds tmux routing/PTY code; no new RestorableAgentSessionIndex.load() or large JSON loads were added to main-actor/interactive paths.
Cmux Cache Substitution Correctness ✅ Passed No persistence/history/undo/snapshot path swapped a fresh read for a stale cache; the new session-set cache has a cold fallback and event-driven refresh.
Cmux No Hacky Sleeps ✅ Passed No covered TS/JS/shell runtime code added timers; the only new waits are Swift or test-only, and the rule explicitly excludes Swift timing.
Cmux Algorithmic Complexity ✅ Passed New scans are linear over session/window sets or tiny static command lists; no nested rescans or repeated sort/filter hot paths were introduced.
Cmux Swift @Concurrent ✅ Passed No changed nonisolated async work lacks @concurrent, and new async helpers either hop to actors or are intentionally UI-bound.
Cmux Swiftpm Lockfiles ✅ Passed No Package.resolved or .gitignore changes appear in the PR diff, and the pbxproj edit is file registration only—not a SwiftPM package-reference change.
Cmux Swift Logging ✅ Passed No new production logging was added or changed; touched runtime NSLogs are DEBUG-gated and the CLI prints are user-facing output.
Cmux Full Internationalization ✅ Passed PASS: cli.help.tmux has all 20 catalog locales, and the remote-tmux web docs are intentionally limited to en/ja with matching en/ja message entries.
Cmux Swiftui State Layout ✅ Passed Touched Swift files are action/model code; no new GeometryReader, lazy-row store refs, or render-time state writes were introduced, and existing @Published state was only incidental.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: PR adds tmux/session logic and tests only; no new standalone NSWindow/NSPanel/WindowGroup code or cmux.* identifier changes were introduced.
Cmux Source Artifacts ✅ Passed Changed paths are source, tests, docs, localization, and project config only; no temp/build/cache artifacts or scratch dirs appear.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Touched production Sources files add local tmux logic only; no new #if DEBUG blocks, test-only members, or widened test/debug seams were introduced.
Cmux No Ambient Global State ✅ Passed New state stays on owned types (RemoteTmuxController/Observers); no new file-scope API or singleton runtime state was added, only a canonical RemoteTmuxHost.local constant.
Title check ✅ Passed The title accurately summarizes the main change: adding first-class local tmux support with two-way sync.
Description check ✅ Passed The description covers the summary, implementation details, and verification, but it omits the demo video, review trigger, and checklist sections.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47b5abcb60

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

guard let executable = argv.first else {
throw RemoteTmuxError.launchFailed("empty local command")
}
return try await runProcess(executable: executable, arguments: Array(argv.dropFirst()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep local one-shot commands on the same tmux server

When cmux is launched from inside a tmux client connected to a non-default server (for example, tmux -L work), this process inherits TMUX, so these discovery and mutation commands target that inherited server. The control-stream launch explicitly removes TMUX in localControlModeEnvironment(), causing it to attach to the default server instead. As a result, cmux tmux can discover sessions on one server and then fail to attach (or mirror/mutate a different server); scrub TMUX/TMUX_PANE for local one-shots too, or consistently preserve and explicitly target the inherited server.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds the local machine's tmux server as a first-class endpoint in the remote-tmux mirror stack — reusing the existing control-mode parser, session/window mirrors, and sizing machinery with no SSH involvement. A new RemoteTmuxLocalPTY allocates and raw-modes a PTY pair so the local tmux -CC client gets the controlling tty it requires; RemoteTmuxSSHTransport dispatches local one-shots directly through the same resolver script; and %sessions-changed drives a debounced reconcile that auto-mirrors newly created sessions.

  • New cmux tmux CLI command mirrors all local sessions with full two-way sync; every remote.tmux.* socket method gains local: true in place of host.
  • Focus-gated size authority: while cmux is backgrounded, refresh-client -f ignore-size hands window-size control to co-attached terminals; cmux reclaims it on foreground.
  • Workspace-close semantics differ by host kind: closing a local-mirror workspace kills its tmux session (two-way ownership), while SSH mirrors retain the existing detach-and-keep behavior.

Confidence Score: 4/5

The local-PTY wiring, SSH transport bypass, host-kind routing, and two-way sync design are all sound; the one issue is a Task.sleep in the %sessions-changed reconcile path.

The reconcile debounce in scheduleSessionSetReconcile uses Task.sleep(300ms) for two stated purposes: collapsing the broadcast burst from N clients and letting tmux finish creating the session. The second purpose is a timing race — if the local machine is under load, the 300ms window may not be enough and the reconcile silently misses the new session without retrying. The cmux-swift-blocking-runtime rule explicitly prohibits Task.sleep used as synchronization. Everything else in the PR — PTY allocation and raw-mode setup, SSH transport bypass, connectionHash namespace isolation, focus-gated size authority, workspace-close kill-vs-detach policy, i18n coverage — is correct and well-guarded.

Sources/RemoteTmuxController+Attach.swift — specifically the scheduleSessionSetReconcile function and its Task.sleep debounce.

Important Files Changed

Filename Overview
Sources/RemoteTmuxController+Attach.swift Adds session-set reconcile on %sessions-changed. Uses Task.sleep(300ms) as both a debounce AND as synchronization to "let tmux finish creating the session" — timing-based synchronization prohibited by the blocking-runtime rule.
Sources/RemoteTmuxLocalPTY.swift New file: allocates a local PTY pair for tmux -CC control mode. Sets raw mode and CLOEXEC on master/dup before use; slave fd CLOEXEC is handled by the caller (already addressed in prior review thread).
Sources/RemoteTmuxControlConnection.swift Spawns local tmux -CC under RemoteTmuxLocalPTY; adds focus-gated size-authority state (sizeAuthorityReleased). Slave closed post-launch; master/write-dup get CLOEXEC. %sessions-changed now notifies observers.
Sources/RemoteTmuxController.swift Adds focus-gated size authority via NSApplication activation observers; registers %sessions-changed reconcile. discoveredSessionIdsByHost and sessionSetReconcileTasks are new internal state that never gets pruned on disconnect.
Sources/RemoteTmuxHost.swift Adds Kind enum (.ssh/.local), static .local endpoint, connectionHash shortcut, and local-specific argv helpers. Clean design with clear separation from SSH paths.
Sources/RemoteTmuxSSHTransport.swift Local commands dispatch through runLocal using the same tmux resolver script; ensureMasterReady and shutdownMaster degenerate to no-ops for local. Clean, no new issues.
Sources/TabManager.swift Closing a local-mirror workspace now kills its tmux session; SSH mirrors retain detach-and-keep behavior. Logic is gated on mirrorHostIsLocal check.
CLI/cmux.swift Adds cmux tmux subcommand (local:true path); reuses runRemoteTmux with local flag; rejects --port/--identity and handles auth-required guard for the local path.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CLI as "cmux tmux CLI"
    participant RC as "RemoteTmuxController"
    participant T as "RemoteTmuxSSHTransport"
    participant PTY as "RemoteTmuxLocalPTY"
    participant CC as "RemoteTmuxControlConnection"
    participant tmux as "local tmux server"

    CLI->>RC: mirrorAllSessions(host:.local)
    RC->>T: discoverMirrorSessions
    T->>tmux: resolver → tmux list-sessions
    tmux-->>T: session list
    T-->>RC: sessions
    RC->>CC: start() per session
    CC->>PTY: open() openpty + cfmakeraw + CLOEXEC on master+dup
    PTY-->>CC: masterRead, masterWrite, slave fds
    CC->>tmux: exec resolver → tmux -CC attach-session
    tmux-->>CC: control stream over master fd
    CC->>RC: topology published, workspace per session

    tmux->>CC: percent-sessions-changed
    CC->>RC: onSessionsChanged
    RC->>RC: scheduleSessionSetReconcile 300ms Task.sleep debounce
    RC->>T: discoverMirrorSessions createIfEmpty false
    T-->>RC: fresh sessions
    RC->>RC: mirrorDiscoveredSessions new sessions only

    RC->>T: runTmux kill-session on workspace close
    T->>tmux: tmux kill-session
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"}}}%%
sequenceDiagram
    participant CLI as "cmux tmux CLI"
    participant RC as "RemoteTmuxController"
    participant T as "RemoteTmuxSSHTransport"
    participant PTY as "RemoteTmuxLocalPTY"
    participant CC as "RemoteTmuxControlConnection"
    participant tmux as "local tmux server"

    CLI->>RC: mirrorAllSessions(host:.local)
    RC->>T: discoverMirrorSessions
    T->>tmux: resolver → tmux list-sessions
    tmux-->>T: session list
    T-->>RC: sessions
    RC->>CC: start() per session
    CC->>PTY: open() openpty + cfmakeraw + CLOEXEC on master+dup
    PTY-->>CC: masterRead, masterWrite, slave fds
    CC->>tmux: exec resolver → tmux -CC attach-session
    tmux-->>CC: control stream over master fd
    CC->>RC: topology published, workspace per session

    tmux->>CC: percent-sessions-changed
    CC->>RC: onSessionsChanged
    RC->>RC: scheduleSessionSetReconcile 300ms Task.sleep debounce
    RC->>T: discoverMirrorSessions createIfEmpty false
    T-->>RC: fresh sessions
    RC->>RC: mirrorDiscoveredSessions new sessions only

    RC->>T: runTmux kill-session on workspace close
    T->>tmux: tmux kill-session
Loading

Reviews (3): Last reviewed commit: "Local tmux: sane new-tab cwd, focus-gate..." | Re-trigger Greptile

Comment on lines +32 to +68
static func open() throws -> RemoteTmuxLocalPTY {
var master: Int32 = -1
var slave: Int32 = -1
guard openpty(&master, &slave, nil, nil, nil) == 0 else {
throw RemoteTmuxError.launchFailed("openpty: \(String(cString: strerror(errno)))")
}

var tio = termios()
if tcgetattr(slave, &tio) == 0 {
cfmakeraw(&tio)
_ = tcsetattr(slave, TCSANOW, &tio)
}
// A sane default client size for the brief pre-attach window; once the
// mirror is live, cmux drives sizing with `refresh-client -C` claims and
// a control client never becomes tmux's "latest" client anyway.
var size = winsize(ws_row: 24, ws_col: 80, ws_xpixel: 0, ws_ypixel: 0)
_ = ioctl(slave, TIOCSWINSZ, &size)

// Keep the parent-side descriptors out of every other child cmux spawns:
// a leaked master in an unrelated long-lived child would hold the pty
// open and delay EOF. (Foundation dup2s the slave for this child itself.)
_ = fcntl(master, F_SETFD, FD_CLOEXEC)
let writeFD = dup(master)
guard writeFD >= 0 else {
let error = String(cString: strerror(errno))
close(master)
close(slave)
throw RemoteTmuxError.launchFailed("dup pty master: \(error)")
}
_ = fcntl(writeFD, F_SETFD, FD_CLOEXEC)

return RemoteTmuxLocalPTY(
masterReadHandle: FileHandle(fileDescriptor: master, closeOnDealloc: true),
masterWriteHandle: FileHandle(fileDescriptor: writeFD, closeOnDealloc: true),
slaveHandle: FileHandle(fileDescriptor: slave, closeOnDealloc: true)
)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 PTY slave fd leaked to concurrently-spawned processes

slave never has FD_CLOEXEC set, so any Process.run() call that RemoteTmuxSSHTransport.runProcess() (or any other Foundation Process) fires concurrently with RemoteTmuxControlConnection.start() — or after openpty() but before the post-launch slaveHandleToClose?.close() — will inherit the slave fd. Foundation dup2s the slave to stdin/stdout before exec(), so setting FD_CLOEXEC on the original fd is safe: the tmux child keeps the pty via fd 0/1; every other child never has it at all.

If that concurrent process is long-lived (e.g., a keep-alive: true ssh or a lingering discovery probe), it holds the slave open after the tmux client exits. read(master, …) then never returns EIO, so the stdout pipe reader in RemoteTmuxControlConnection hangs indefinitely instead of triggering the normal exit-cleanup path — the mirror workspace sticks around with a dead connection.

The fix is one line after the TIOCSWINSZ call:

_ = fcntl(slave, F_SETFD, FD_CLOEXEC)

Fix in Cursor

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@cmuxTests/RemoteTmuxLocalEndpointTests.swift`:
- Around line 111-129: Update the test setup around the tmux invocation and
remove the in-process TMUX_TMPDIR mutation via setenv/unsetenv. Pass the
generated root path through the child Process.environment used by
Self.runTmuxSynchronously, or run the live tmux test in an isolated subprocess,
so other test suites cannot observe the temporary server path; preserve cleanup
of the temporary directory.

In `@Sources/RemoteTmuxLocalPTY.swift`:
- Around line 39-61: Update the PTY setup in the launch flow around tcgetattr,
tcsetattr, ioctl, and both fcntl calls to validate every return value and fail
atomically with RemoteTmuxError.launchFailed on error. Apply FD_CLOEXEC to both
master and slave descriptors, and ensure every failure path closes all
descriptors allocated so far, including the duplicated writeFD when applicable;
preserve the existing successful setup behavior.

In `@web/app/`[locale]/(landing)/docs/remote-tmux/page.tsx:
- Around line 79-83: Update the local-mode documentation in the requirements
section and the method/socket reference near the `host` field: describe SSH
`host` and local mode’s `local: true` as alternatives, remove the implication
that `host` is mandatory for every method, and ensure the existing local tmux
examples remain consistent.
- Around line 78-84: Reorder the documentation headings so the SSH-specific
“Permission denied” troubleshooting H3 remains under the SSH attachment section
rather than becoming a subsection of the new local-tmux H2. Move the local-tmux
section after that H3, or otherwise promote/restructure the troubleshooting
heading to preserve the intended hierarchy.
🪄 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: 491244bb-a417-4090-a73a-89ab5b707cb5

📥 Commits

Reviewing files that changed from the base of the PR and between 2c07967 and 47b5abc.

📒 Files selected for processing (14)
  • CLI/CMUXCLI+CommandSuggestions.swift
  • CLI/cmux.swift
  • Resources/Localizable.xcstrings
  • Sources/RemoteTmuxControlConnection.swift
  • Sources/RemoteTmuxController.swift
  • Sources/RemoteTmuxHost.swift
  • Sources/RemoteTmuxLocalPTY.swift
  • Sources/RemoteTmuxSSHTransport.swift
  • Sources/TerminalController+RemoteTmux.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/RemoteTmuxLocalEndpointTests.swift
  • web/app/[locale]/(landing)/docs/remote-tmux/page.tsx
  • web/messages/en.json
  • web/messages/ja.json

Comment on lines +111 to +129
let root = URL(
fileURLWithPath: "/tmp/cmux-lt-\(UUID().uuidString.prefix(8))",
isDirectory: true
)
try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
let previousTmpdir = getenv("TMUX_TMPDIR").map { String(cString: $0) }
setenv("TMUX_TMPDIR", root.path, 1)
// Defers run LIFO: the env restore is registered FIRST so the
// kill-server below still sees the isolated TMUX_TMPDIR — a kill that
// ran after the restore would hit the user's real tmux server.
defer {
if let previousTmpdir {
setenv("TMUX_TMPDIR", previousTmpdir, 1)
} else {
unsetenv("TMUX_TMPDIR")
}
try? FileManager.default.removeItem(at: root)
}
defer { Self.runTmuxSynchronously(["kill-server"]) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C4 'TMUX_TMPDIR|setenv\(|unsetenv\(|`@Suite`\(\.serialized\)' cmuxTests Sources

Repository: manaflow-ai/cmux

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## RemoteTmuxLocalEndpointTests.swift\n'
sed -n '1,240p' cmuxTests/RemoteTmuxLocalEndpointTests.swift

printf '\n## runTmuxSynchronously definition(s)\n'
rg -n -C4 'runTmuxSynchronously|TMUX_TMPDIR' cmuxTests Sources/Debug Sources -g '!**/DerivedData/**'

printf '\n## nearby test isolation patterns\n'
rg -n -C3 '`@Suite`\(\.serialized\)|setenv\("TMUX_TMPDIR"|unsetenv\("TMUX_TMPDIR"|setenv\("CMUX_SOCKET_PATH"|unsetenv\(' cmuxTests -g '*.swift' | head -n 120

Repository: manaflow-ai/cmux

Length of output: 32598


🌐 Web query:

Swift Testing @suite(.serialized) semantics suite-only or global process concurrency environment mutation

💡 Result:

In the Swift Testing framework, the @Suite(.serialized) trait is used to ensure that the tests and sub-suites contained within an annotated suite run sequentially rather than in parallel [1]. Semantics and Scope: The .serialized trait affects only the execution of the children of the suite or test it is applied to [1]. It does not provide isolation across the entire process, nor does it guarantee that the suite will run in isolation from other tests in the test plan [1][2]. Specifically, while tests within a single .serialized suite are guaranteed to run one after another, they can still execute concurrently with tests located in other suites—even if those other suites are also marked with .serialized [2][3]. Global Concurrency and Environment Mutation: Because the basic .serialized trait does not provide global process-level serialization, using it to protect tests that mutate global state (such as environment variables, static properties, or shared databases) is often insufficient if other tests in the test plan also access that same state [2][3]. If multiple tests mutate shared global state, they may still conflict unless they are all contained within the same serialized hierarchy [2]. To address this, the framework has introduced experimental support for data-dependent serialization, such as .serialized(for:) [3][4]. This trait allows authors to specify a dependency (e.g., \ProcessInfo.environment) or use a wildcard (*) to indicate that a test depends on the complete state of the current process [5][4]. When tests specify the same dependency, the testing library ensures they are serialized with respect to one another, providing a mechanism to safely perform operations that require exclusive access to shared process-level resources [3][4]. Tests using the wildcard .serialized(for: *) are effectively serialized against all other tests that also claim a dependency on the global state [5][4].

Citations:


Avoid mutating TMUX_TMPDIR in-process here. .serialized only serializes this suite’s children, not the whole test process, so other suites can still observe the temporary server path. Pass the directory through the child Process.environment or isolate the live tmux test in a subprocess.

🤖 Prompt for 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.

In `@cmuxTests/RemoteTmuxLocalEndpointTests.swift` around lines 111 - 129, Update
the test setup around the tmux invocation and remove the in-process TMUX_TMPDIR
mutation via setenv/unsetenv. Pass the generated root path through the child
Process.environment used by Self.runTmuxSynchronously, or run the live tmux test
in an isolated subprocess, so other test suites cannot observe the temporary
server path; preserve cleanup of the temporary directory.

Source: Coding guidelines

Comment on lines +39 to +61
var tio = termios()
if tcgetattr(slave, &tio) == 0 {
cfmakeraw(&tio)
_ = tcsetattr(slave, TCSANOW, &tio)
}
// A sane default client size for the brief pre-attach window; once the
// mirror is live, cmux drives sizing with `refresh-client -C` claims and
// a control client never becomes tmux's "latest" client anyway.
var size = winsize(ws_row: 24, ws_col: 80, ws_xpixel: 0, ws_ypixel: 0)
_ = ioctl(slave, TIOCSWINSZ, &size)

// Keep the parent-side descriptors out of every other child cmux spawns:
// a leaked master in an unrelated long-lived child would hold the pty
// open and delay EOF. (Foundation dup2s the slave for this child itself.)
_ = fcntl(master, F_SETFD, FD_CLOEXEC)
let writeFD = dup(master)
guard writeFD >= 0 else {
let error = String(cString: strerror(errno))
close(master)
close(slave)
throw RemoteTmuxError.launchFailed("dup pty master: \(error)")
}
_ = fcntl(writeFD, F_SETFD, FD_CLOEXEC)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Fail PTY setup atomically instead of ignoring configuration errors.

If raw mode fails, echoed/transformed bytes can corrupt the control stream. Also mark the slave FD_CLOEXEC; an unrelated child inheriting it can suppress EOF after tmux exits. Check tcgetattr, tcsetattr, ioctl, and each fcntl, closing all allocated descriptors before throwing.

🤖 Prompt for 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.

In `@Sources/RemoteTmuxLocalPTY.swift` around lines 39 - 61, Update the PTY setup
in the launch flow around tcgetattr, tcsetattr, ioctl, and both fcntl calls to
validate every return value and fail atomically with
RemoteTmuxError.launchFailed on error. Apply FD_CLOEXEC to both master and slave
descriptors, and ensure every failure path closes all descriptors allocated so
far, including the duplicated writeFD when applicable; preserve the existing
successful setup behavior.

Sources: Coding guidelines, Path instructions

Comment on lines +78 to +84
<DocsHeading level={2} id="local-tmux">{t("localTitle")}</DocsHeading>
<p>{t("localIntro")}</p>
<CodeBlock lang="bash">{`cmux tmux\ncmux tmux --new-window`}</CodeBlock>
<p>{t("localSync")}</p>
<p>{t("localSocket")}</p>
<CodeBlock lang="json">{`{ "method": "remote.tmux.mirror", "params": { "local": true } }`}</CodeBlock>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep “Permission denied” under the SSH attachment section.

The new H2 at Line 78 makes the existing H3 at Line 85 a subsection of “Local tmux,” although that troubleshooting guidance is SSH-specific. Move this section after the H3 or promote/restructure the troubleshooting heading.

🤖 Prompt for 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.

In `@web/app/`[locale]/(landing)/docs/remote-tmux/page.tsx around lines 78 - 84,
Reorder the documentation headings so the SSH-specific “Permission denied”
troubleshooting H3 remains under the SSH attachment section rather than becoming
a subsection of the new local-tmux H2. Move the local-tmux section after that
H3, or otherwise promote/restructure the troubleshooting heading to preserve the
intended hierarchy.

Comment on lines +79 to +83
<p>{t("localIntro")}</p>
<CodeBlock lang="bash">{`cmux tmux\ncmux tmux --new-window`}</CodeBlock>
<p>{t("localSync")}</p>
<p>{t("localSocket")}</p>
<CodeBlock lang="json">{`{ "method": "remote.tmux.mirror", "params": { "local": true } }`}</CodeBlock>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Update the existing requirements and socket reference for local mode.

Lines 65-66 still require an SSH host, while Lines 115-119 still present host as mandatory for every method. Document the host versus local: true alternatives in those authoritative sections.

🤖 Prompt for 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.

In `@web/app/`[locale]/(landing)/docs/remote-tmux/page.tsx around lines 79 - 83,
Update the local-mode documentation in the requirements section and the
method/socket reference near the `host` field: describe SSH `host` and local
mode’s `local: true` as alternatives, remove the implication that `host` is
mandatory for every method, and ensure the existing local tmux examples remain
consistent.

Complete the two-way sync story for the local tmux mirror in both
directions that previously needed a manual step:

tmux -> cmux (new sessions): %sessions-changed was decoded but only
logged. It now fans out through a new observer channel; the controller
registers per cached connection and runs a per-host debounced reconcile
(300ms — every control client on the server receives the broadcast at
once) that re-discovers the session set and mirrors only sessions NEW
relative to the last discovery baseline, so `tmux new-session` from any
terminal appears as a sidebar workspace without re-running `cmux tmux`.
The new-only filter keeps a workspace the user deliberately
detached-but-kept-open from resurrecting; destroyed sessions already
tear down via their own connection's %exit. The reconcile skips (rather
than queues) when an explicit attach is in flight, and never creates a
session on an empty server (that would fight kill-on-close teardown).

cmux -> tmux (new workspaces): when the window's SELECTED workspace
mirrors the local server, a plain New Workspace becomes a real
`tmux new-session -d` that is mirrored and selected — the
workspace-level counterpart of the in-mirror new-tab -> new-window
routing. Wired at the shared creation funnel
(performNewWorkspaceCreationAction, after the user's configured
override, terminal variant only) plus the two entrypoints that bypass
it (the configured built-in action and the surface tab-bar button).
Scripted workspace.create keeps exact plain semantics deliberately.
Falls back to a plain workspace if session creation fails so the action
never silently no-ops. Selected-workspace gating matches the sidebar's
existing mirror routing (dedicated windows can hold dragged-in locals).

Covered by a new live test (sessionsChangedNotificationReachesObservers)
attaching a real tmux under an isolated TMUX_TMPDIR. Live-verified
against a real server: out-of-band new-session appears as a workspace
in ~1s; kill-session closes it.

No user-facing strings added (localization audit: no localizable
surfaces touched).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a93659c4a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +209 to +211
// Collapse the per-client broadcast burst and let tmux finish
// creating the session before discovery lists it.
try? await Task.sleep(for: .milliseconds(300))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Replace the fixed settle sleep with an event-driven reconcile

AGENTS.md explicitly forbids Task.sleep used to let state “settle,” and the cmux-architecture skill requires an injected clock even for permitted bounded delays. This call is expressly intended to “let tmux finish,” making session reconciliation depend on an untestable wall-clock interval rather than the authoritative session-change signal; replace it with event/completion-driven coordination instead of a fixed 300 ms delay.

Useful? React with 👍 / 👎.

Comment on lines +296 to +299
mirrorDiscoveredSessions(host: host, sessions: [session], into: manager)
let key = Self.connectionKey(host: host, sessionName: session.name)
if let workspace = sessionMirrors[key]?.mirroredWorkspace {
manager.selectWorkspace(workspace)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back when the new session cannot be mirrored

When new-session succeeds but starting its control connection fails—for example, PTY allocation or Process.run() fails—mirrorDiscoveredSessions catches the per-session error internally and returns without creating a workspace. This method then silently skips the optional selection even though routeNewWorkspaceToLocalTmux already returned true, leaving an unmirrored tmux session and no new workspace; verify that this specific session acquired a mirror and invoke the documented fallback or roll back the session otherwise.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`+Attach.swift:
- Around line 205-216: Replace the arbitrary Task.sleep in
scheduleSessionSetReconcile with explicit state verification or a completion
signal before reconcileSessionSet, ensuring discovery does not depend on a
timing delay. If the task only debounces %sessions-changed notifications, remove
the readiness-wait dependency while preserving cancellation, task replacement,
and cleanup behavior.
- Around line 261-264: Replace the unstored Task in the attach flow with a
lifecycle-managed task stored by RemoteTmuxController, keyed as needed for the
session or manager. Update teardown to cancel and remove the stored task, while
preserving the existing createAndMirrorLocalSession call and weak-self behavior.
🪄 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: d06b13f3-ad38-40c2-89bb-620b640e9dd9

📥 Commits

Reviewing files that changed from the base of the PR and between 47b5abc and 3a93659.

📒 Files selected for processing (8)
  • Sources/AppDelegate.swift
  • Sources/RemoteTmuxConnectionObservers.swift
  • Sources/RemoteTmuxControlConnection+Observation.swift
  • Sources/RemoteTmuxControlConnection.swift
  • Sources/RemoteTmuxController+Attach.swift
  • Sources/RemoteTmuxController.swift
  • Sources/Workspace.swift
  • cmuxTests/RemoteTmuxLocalEndpointTests.swift

Comment on lines +205 to +216
func scheduleSessionSetReconcile(host: RemoteTmuxHost) {
let hash = host.connectionHash
sessionSetReconcileTasks[hash]?.cancel()
sessionSetReconcileTasks[hash] = Task { [weak self] in
// Collapse the per-client broadcast burst and let tmux finish
// creating the session before discovery lists it.
try? await Task.sleep(for: .milliseconds(300))
guard !Task.isCancelled, let self else { return }
self.sessionSetReconcileTasks[hash] = nil
await self.reconcileSessionSet(host: host)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Avoid Task.sleep for delayed coordination.

The Task.sleep is used as a readiness wait to "let tmux finish creating the session". As per coding guidelines, do not patch symptoms with sleeps, delayed dispatch, or polling for delayed coordination or readiness waits.

Rely on an explicit completion signal or state verification rather than assuming an arbitrary time delay is required for subsequent discovery reads. If the sleep is solely for debouncing the burst of %sessions-changed notifications, remove the readiness-wait justification and ensure the logic doesn't depend on the delay for correctness.

🤖 Prompt for 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.

In `@Sources/RemoteTmuxController`+Attach.swift around lines 205 - 216, Replace
the arbitrary Task.sleep in scheduleSessionSetReconcile with explicit state
verification or a completion signal before reconcileSessionSet, ensuring
discovery does not depend on a timing delay. If the task only debounces
%sessions-changed notifications, remove the readiness-wait dependency while
preserving cancellation, task replacement, and cleanup behavior.

Source: Coding guidelines

Comment on lines +261 to +264
Task { [weak self] in
await self?.createAndMirrorLocalSession(in: manager)
}
return true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Avoid unstructured, fire-and-forget lifecycle tasks.

Creating an unstructured, fire-and-forget Task { ... } to create a tmux session and mutate the workspace violates the guideline against unstored, uncancellable lifecycle work. If the manager is closed or the app shuts down before the task finishes, it will still execute and could cause unintended side effects on the tmux server or race with local teardown.

As per coding guidelines, do not create fire-and-forget Task { ... } work with meaningful lifecycle unless it is stored, cancellable, or tied to a caller-owned operation. Store the task (for example, in a dictionary managed by RemoteTmuxController) and cancel it on teardown.

🤖 Prompt for 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.

In `@Sources/RemoteTmuxController`+Attach.swift around lines 261 - 264, Replace
the unstored Task in the attach flow with a lifecycle-managed task stored by
RemoteTmuxController, keyed as needed for the session or manager. Update
teardown to cancel and remove the stored task, while preserving the existing
createAndMirrorLocalSession call and weak-self behavior.

Source: Coding guidelines

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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f2b6c3e32

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

/// attach is in flight — that attach's own discovery will see the new set.
private func reconcileSessionSet(host: RemoteTmuxHost) async {
guard existingMirrorManager(for: host) != nil else { return }
guard windowRegistry.beginAttach(hostHash: host.connectionHash) else { return }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve reconciles blocked by an active attach

When a new session is created after attachHost has completed discovery but while that same host's attach guard is still held—for example, during a slow multi-session mirror—the %sessions-changed task reaches this guard and returns permanently. The in-flight attach cannot discover that later-created session, and tmux emits no second notification, so it is not mirrored until an unrelated session change occurs; retain the reconcile request and run it after endAttach instead of dropping it.

Useful? React with 👍 / 👎.

Comment thread Sources/TabManager.swift
Comment on lines +2019 to +2020
if controller?.mirrorHostIsLocal(workspaceId: workspace.id) == true {
controller?.handleWorkspaceClosed(workspaceId: workspace.id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Kill local sessions when closing the last mirror workspace

When a local mirror is the window's last workspace—common for cmux tmux --new-window with one session—closeWorkspace returns at line 2006 before this new local-host branch runs. The last-workspace path in closeWorkspaceIfRunningProcess instead calls the still-no-op markRemoteTmuxKillOnWindowCloseIfNeeded (TabManager.swift:2240-2249) and the window-close handler detaches, so closing that workspace leaves its local tmux session alive even though non-last local workspace closes kill it.

Useful? React with 👍 / 👎.

Comment thread CLI/cmux.swift
index += 1
default:
if local {
throw CLIError(message: "tmux: unexpected argument '\(arg)' (the local tmux server takes no destination; options: --no-focus, --new-window)")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Localize the new tmux CLI errors

The repository's AGENTS.md requires every user-facing command/help/error string to be localized for every supported locale, but the new local command emits this raw English error; the new auth-response error at line 8837 and local progress/success text are also unlocalized. Users running cmux in Japanese or any other supported locale therefore receive English-only output on these cmux tmux paths; add catalog keys and translations rather than bare literals.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
Sources/RemoteTmuxController+Attach.swift (2)

205-216: 🩺 Stability & Availability | 🟠 Major | ⚖️ Poor tradeoff

Avoid Task.sleep for delayed coordination.

The Task.sleep is used as a readiness wait to "let tmux finish creating the session". As per coding guidelines, do not patch symptoms with sleeps, delayed dispatch, or polling for delayed coordination or readiness waits.

Rely on an explicit completion signal or state verification rather than assuming an arbitrary time delay is required for subsequent discovery reads. If the sleep is solely for debouncing the burst of %sessions-changed notifications, remove the readiness-wait justification and ensure the logic doesn't depend on the delay for correctness.

🤖 Prompt for 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.

In `@Sources/RemoteTmuxController`+Attach.swift around lines 205 - 216, Update
scheduleSessionSetReconcile to remove the Task.sleep-based readiness wait and
avoid relying on an arbitrary delay before reconcileSessionSet. Preserve
cancellation and per-host task replacement, and use an explicit completion/state
verification mechanism if session creation readiness is required; otherwise
perform reconciliation directly while retaining only intentional notification
coalescing.

Source: Coding guidelines


261-264: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Avoid unstructured, fire-and-forget lifecycle tasks.

Creating an unstructured, fire-and-forget Task { ... } to create a tmux session and mutate the workspace violates the guideline against unstored, uncancellable lifecycle work. If the manager is closed or the app shuts down before the task finishes, it will still execute and could cause unintended side effects on the tmux server or race with local teardown.

As per coding guidelines, do not create fire-and-forget Task { ... } work with meaningful lifecycle unless it is stored, cancellable, or tied to a caller-owned operation. Store the task (for example, in a dictionary managed by RemoteTmuxController) and cancel it on teardown.

🤖 Prompt for 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.

In `@Sources/RemoteTmuxController`+Attach.swift around lines 261 - 264, Replace
the unstored Task in the attach flow with a lifecycle-managed task tied to
RemoteTmuxController, storing it in the controller’s task tracking collection
and removing it when complete. Update the controller’s teardown path to cancel
tracked session-creation tasks so createAndMirrorLocalSession cannot continue
after shutdown.

Source: Coding guidelines

🤖 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 `@cmuxTests/RemoteTmuxLocalEndpointTests.swift`:
- Around line 281-298: Update anyClientIgnoresSize in the waitForIgnoreSize flow
so a failed list-clients command or empty client result cannot be interpreted as
ignore-size being cleared. Require transport.runTmux to succeed and return a
nonempty client result before evaluating whether stdout contains "ignore-size";
preserve the existing polling and timeout behavior.

In `@Sources/RemoteTmuxControlConnection`+Commands.swift:
- Around line 273-277: Update the reconnect handling around
sizeAuthorityReleased to invoke the existing applySizeAuthority() action instead
of sending refresh-client -f ignore-size directly. Preserve the
released-authority behavior while routing it through the canonical lifecycle
path and avoiding duplicated command logic.

---

Duplicate comments:
In `@Sources/RemoteTmuxController`+Attach.swift:
- Around line 205-216: Update scheduleSessionSetReconcile to remove the
Task.sleep-based readiness wait and avoid relying on an arbitrary delay before
reconcileSessionSet. Preserve cancellation and per-host task replacement, and
use an explicit completion/state verification mechanism if session creation
readiness is required; otherwise perform reconciliation directly while retaining
only intentional notification coalescing.
- Around line 261-264: Replace the unstored Task in the attach flow with a
lifecycle-managed task tied to RemoteTmuxController, storing it in the
controller’s task tracking collection and removing it when complete. Update the
controller’s teardown path to cancel tracked session-creation tasks so
createAndMirrorLocalSession cannot continue after shutdown.
🪄 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: 2a0bd8bc-6ba5-4e27-a743-2d2a50ae3839

📥 Commits

Reviewing files that changed from the base of the PR and between 3a93659 and 5f2b6c3.

📒 Files selected for processing (11)
  • Sources/RemoteTmuxControlConnection+Commands.swift
  • Sources/RemoteTmuxControlConnection+Sizing.swift
  • Sources/RemoteTmuxControlConnection.swift
  • Sources/RemoteTmuxController+Attach.swift
  • Sources/RemoteTmuxController+Decisions.swift
  • Sources/RemoteTmuxController.swift
  • Sources/RemoteTmuxSSHTransport.swift
  • Sources/TabManager.swift
  • cmuxTests/RemoteTmuxLocalEndpointTests.swift
  • cmuxTests/RemoteTmuxMirrorNewTabPlacementTests.swift
  • cmuxTests/RemoteTmuxNewWindowCwdTests.swift

Comment on lines +281 to +298
func anyClientIgnoresSize() async -> Bool {
let result = try? await transport.runTmux(["list-clients", "-F", "#{client_flags}"])
return result?.stdout.contains("ignore-size") ?? false
}
func waitForIgnoreSize(_ expected: Bool, _ what: String) async throws {
let clock = ContinuousClock()
let deadline = clock.now.advanced(by: .seconds(15))
while await anyClientIgnoresSize() != expected {
if clock.now > deadline { Issue.record("timed out waiting for \(what)"); return }
try await Task.sleep(for: .milliseconds(50))
}
}

connection.setSizeAuthorityReleased(true)
try await waitForIgnoreSize(true, "ignore-size flag set on the control client")

connection.setSizeAuthorityReleased(false)
try await waitForIgnoreSize(false, "ignore-size flag cleared on the control client")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not treat a failed list-clients query as a cleared flag.

try? and ?? false make the final assertion pass when tmux fails or the control client disappears. Require a successful command and a nonempty client result before checking the flag.

Proposed fix
-        func anyClientIgnoresSize() async -> Bool {
-            let result = try? await transport.runTmux(["list-clients", "-F", "#{client_flags}"])
-            return result?.stdout.contains("ignore-size") ?? false
+        func anyClientIgnoresSize() async throws -> Bool {
+            let result = try await transport.runTmux(["list-clients", "-F", "#{client_flags}"])
+            try `#require`(result.succeeded, Comment(rawValue: result.stderr))
+            try `#require`(!result.stdout.isEmpty, "expected an attached tmux control client")
+            return result.stdout.contains("ignore-size")
         }
...
-            while await anyClientIgnoresSize() != expected {
+            while try await anyClientIgnoresSize() != expected {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func anyClientIgnoresSize() async -> Bool {
let result = try? await transport.runTmux(["list-clients", "-F", "#{client_flags}"])
return result?.stdout.contains("ignore-size") ?? false
}
func waitForIgnoreSize(_ expected: Bool, _ what: String) async throws {
let clock = ContinuousClock()
let deadline = clock.now.advanced(by: .seconds(15))
while await anyClientIgnoresSize() != expected {
if clock.now > deadline { Issue.record("timed out waiting for \(what)"); return }
try await Task.sleep(for: .milliseconds(50))
}
}
connection.setSizeAuthorityReleased(true)
try await waitForIgnoreSize(true, "ignore-size flag set on the control client")
connection.setSizeAuthorityReleased(false)
try await waitForIgnoreSize(false, "ignore-size flag cleared on the control client")
func anyClientIgnoresSize() async throws -> Bool {
let result = try await transport.runTmux(["list-clients", "-F", "#{client_flags}"])
try `#require`(result.succeeded, Comment(rawValue: result.stderr))
try `#require`(!result.stdout.isEmpty, "expected an attached tmux control client")
return result.stdout.contains("ignore-size")
}
func waitForIgnoreSize(_ expected: Bool, _ what: String) async throws {
let clock = ContinuousClock()
let deadline = clock.now.advanced(by: .seconds(15))
while try await anyClientIgnoresSize() != expected {
if clock.now > deadline { Issue.record("timed out waiting for \(what)"); return }
try await Task.sleep(for: .milliseconds(50))
}
}
connection.setSizeAuthorityReleased(true)
try await waitForIgnoreSize(true, "ignore-size flag set on the control client")
connection.setSizeAuthorityReleased(false)
try await waitForIgnoreSize(false, "ignore-size flag cleared on the control client")
🤖 Prompt for 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.

In `@cmuxTests/RemoteTmuxLocalEndpointTests.swift` around lines 281 - 298, Update
anyClientIgnoresSize in the waitForIgnoreSize flow so a failed list-clients
command or empty client result cannot be interpreted as ignore-size being
cleared. Require transport.runTmux to succeed and return a nonempty client
result before evaluating whether stdout contains "ignore-size"; preserve the
existing polling and timeout behavior.

Comment on lines +273 to +277
// A fresh control client starts with no flags, so re-apply a released
// size authority (cmux backgrounded across the reconnect) after re-pinning.
if sizeAuthorityReleased {
send("refresh-client -f ignore-size")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Route reconnect release through the canonical authority action.

This duplicates the release branch of applySizeAuthority(), creating a second lifecycle path that can drift from the normal transition.

Proposed fix
 if sizeAuthorityReleased {
-    send("refresh-client -f ignore-size")
+    applySizeAuthority()
 }

As per coding guidelines, “Do not wire the same behavior separately through multiple surfaces; use one shared action path.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// A fresh control client starts with no flags, so re-apply a released
// size authority (cmux backgrounded across the reconnect) after re-pinning.
if sizeAuthorityReleased {
send("refresh-client -f ignore-size")
}
// A fresh control client starts with no flags, so re-apply a released
// size authority (cmux backgrounded across the reconnect) after re-pinning.
if sizeAuthorityReleased {
applySizeAuthority()
}
🤖 Prompt for 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.

In `@Sources/RemoteTmuxControlConnection`+Commands.swift around lines 273 - 277,
Update the reconnect handling around sizeAuthorityReleased to invoke the
existing applySizeAuthority() action instead of sending refresh-client -f
ignore-size directly. Preserve the released-authority behavior while routing it
through the canonical lifecycle path and avoiding duplicated command logic.

Source: Coding guidelines

@stawiski

Copy link
Copy Markdown

I have this problem too, as remote tmux session has better support than local one. I can't get cmux to recognize local tmux session and show it's windows as cmux tabs, but for remote one it works.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

This needs another review pass before landing: please address the unresolved local PTY and attach findings, preserve the caller’s tmux server, and update the local-mode docs. Before we can re-land it, please comment: I have read the CLA Document v2.2 and I hereby sign the CLA.

@github-actions

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


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.

@teamleaderleo teamleaderleo added the review: needs-attention Actionable automated review finding needs an author reply label Sep 30, 2026

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

review: needs-attention Actionable automated review finding needs an author reply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants