Add tmux control mode integration - #1867
Open
jamesainslie wants to merge 11 commits into
Open
jamesainslie wants to merge 11 commits into
jamesainslie wants to merge 11 commits into
Conversation
Update ghostty submodule to include tmux control mode C API: - TmuxControl action with Event enum (enter/exit/windows_changed/pane_output/etc) - JSON serialization for windows payload with recursive layout tree - Surface message forwarding for pane_output, windows, exit events - ghostty.h updated with tmux types and cmux fork additions Points to jamesainslie/ghostty fork (PR manaflow-ai#16 to manaflow-ai/ghostty).
- TmuxTypes.swift: connection state enum, TmuxEvent with C parser, Codable types for windows payload and recursive layout tree - TmuxGateway.swift: bridges C API actions to controller, write gating with command buffering, global dispatch routing per surface - TmuxController.swift: manages tmux connection lifecycle and state, entity maps (window->workspace, pane->client), gateway burial - Wire GHOSTTY_ACTION_TMUX_CONTROL in GhosttyTerminalView action handler - Add isBuriedGateway flag to Workspace, filter sidebar to hide gateway
- TmuxPaneClient: creates Manual I/O TerminalSurface for tmux panes, feeds output via ghostty_surface_process_output, captures keystrokes via io_write_cb for forwarding to tmux server - TerminalSurface.ManualIOConfig: configures io_mode/write callback during surface creation for surfaces without a local PTY - TmuxController.routeOutput: routes pane output to correct client - History loading deferred (requires Zig-side pane_output emission)
- TmuxLayoutEngine: converts N-ary tmux layout trees to binary trees via right-folding, computes divider fractions accounting for tmux's 1-cell dividers, layout diffing for resize-only vs rebuild detection - TmuxController.openWindows: creates workspace per tmux window, registers TmuxPaneClient for each pane in the layout tree - TmuxController.removeWorkspaceForWindow: tears down pane clients and removes workspace on window close
- TmuxKeyEncoder: classifies bytes as literal ([a-zA-Z0-9+/):,_ ]) or hex (everything else), batches with limits (1000 literal, 125 hex), generates tmux send-keys commands with -l flag for literals - TmuxController.sendKeys: encodes keystroke data and sends each command through the gateway to the tmux server - Mouse events use the same hex encoding path (escape sequences contain non-UTF-8 bytes)
- Debounced window resize (100ms) with version-appropriate commands: tmux >= 3.4 uses refresh-client -C, older uses resize-window - Feedback loop prevention: outstandingResizeCount suppresses echoed layout changes from our own resize commands - Window rename events propagate to workspace titles - Proper pane client teardown on disconnect - Clipboard sync deferred (lower priority)
Phase 6: Session Persistence + Reconnection - Add TmuxSessionInfo Codable type for tmux session metadata - Add tmuxSession field to SessionWorkspaceSnapshot - Add tmuxControllerId back-reference on Workspace - Filter buried gateways and deduplicate tmux workspaces in snapshots - Skip terminal scrollback for tmux workspaces (tmux re-sends on reconnect) - Auto-reconnect via tmux -CC attach on session restore - Persist @cmux_id as tmux session option for double-attach detection Phase 7: UI Integration - Add tmux badge to sidebar TabItemView (with Equatable compliance) - Add tmux menu with Detach command (Ctrl+Cmd+D) - Add isTmuxClient property to TerminalPanel with distinct icon - Add detach confirmation dialog when closing tmux workspaces - TmuxController.hasActiveConnection and detachAll() static helpers
…Phase 8) - Add TmuxCapabilities struct with version-gated feature detection (pause mode >= 3.2, variable window size >= 2.9, per-window refresh >= 3.4) - Implement pause mode: auto-pause after 120s, 1s delay auto-unpause with capture-pane and refresh-client continue - Add double-attach detection via @cmux_id session option check - Add command timeout detection (5s) for unresponsive tmux servers - Clean up all timers (resize debounce, pause, timeout) in teardown
- BrowserConfigTests: add missing #endif for #if compiler(>=6.2) block - WindowAndDragTests: remove stray #endif, add missing #endif for #if DEBUG, move InternalTabDragConfigurationTests to BrowserConfigTests where private types are accessible - WorkspaceUnitTests: remove stray #endif - WorkspaceManualUnreadTests: add @mainactor, mark function throws - AppDelegateShortcutRoutingTests: remove duplicate top-level declaration - BrowserPanelTests: remove duplicate top-level declarations
40 tests across 6 test classes covering all pure/testable tmux components: - TmuxKeyEncoderTests (13): literal/hex encoding, batch limits, mixed modes - TmuxLayoutEngineTests (11): N-ary to binary conversion, right-folding, diffing - TmuxTypesTests (9): Codable roundtrips, JSON decoding, layout nodes - TmuxCapabilitiesTests (5): version-gated feature detection - TmuxConnectionStateTests (1): raw value existence - SessionWorkspaceSnapshotTmuxTests (2): backward compatibility, tmuxSession roundtrip
|
@jamesainslie is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughIntroduces tmux control-mode integration with new infrastructure for managing tmux sessions and panes. Adds six Swift modules handling connection management, event routing, layout rendering, and key encoding. Updates existing UI components to display tmux-specific badges and status, with session persistence for reconnection. Changes
Sequence DiagramsequenceDiagram
participant GhosttyC as Ghostty (C)
participant GTV as GhosttyTerminalView
participant TGW as TmuxGateway
participant TCtr as TmuxController
participant TPC as TmuxPaneClient
participant TS as TerminalSurface
GhosttyC->>GTV: GHOSTTY_ACTION_TMUX_CONTROL (enter)
GTV->>TGW: handleGlobalAction(action, surface)
TGW->>TGW: Create new TmuxGateway & TmuxController
TGW->>TCtr: Wire controller & gateway references
TCtr->>TS: Create gateway surface
TGW->>TGW: Store in activeGateways
GhosttyC->>GTV: GHOSTTY_ACTION_TMUX_CONTROL (windows)
GTV->>TGW: handleGlobalAction(action, surface)
TGW->>TCtr: handleEvent(windowsChanged)
TCtr->>TPC: Create TmuxPaneClient per pane
TPC->>TS: Create TerminalSurface in manual IO mode
TPC->>TS: Set ManualIOConfig with callback
GhosttyC->>GTV: Terminal output for pane
GTV->>TS: ghostty_surface_process_output (manual IO)
TS->>TPC: Invoke io_write_cb callback
TPC->>TCtr: sendKeys(data, toPane)
TCtr->>TGW: sendCommand(send-keys)
TGW->>GhosttyC: Write to gateway surface
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
# Conflicts: # cmuxTests/AppDelegateShortcutRoutingTests.swift # cmuxTests/BrowserPanelTests.swift
Contributor
Greptile SummaryThis PR adds native tmux control mode ( Issues found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant GTV as GhosttyTerminalView
participant GW as TmuxGateway
participant TC as TmuxController
participant TPC as TmuxPaneClient
participant TM as TabManager
Note over GTV,TM: Connection setup (ENTER event)
GTV->>GW: handleGlobalAction(TMUX_ENTER, surface)
GW->>TC: new TmuxController(gatewayPanelId)
GW->>TC: handleEvent(.enter)
TC->>TM: buryGatewayWorkspace()
Note over GTV,TM: Initial sync (WINDOWS_CHANGED)
GTV->>GW: handleGlobalAction(WINDOWS_CHANGED, payload)
GW->>TC: handleEvent(.windowsChanged)
TC->>TM: addWorkspace() per tmux window
TC->>TPC: new TmuxPaneClient(paneId) per pane
TC->>GW: enableWrite() → flush write queue
TC->>GW: sendCommand("set-option @cmux_id …")
Note over GTV,TM: Steady-state I/O
GTV->>GW: handleGlobalAction(PANE_OUTPUT, data)
GW->>TC: handleEvent(.paneOutput)
TC->>TPC: feedOutput(data)
TPC-->>GTV: ghostty_surface_process_output()
TPC-->>GW: ioWriteCallback (keystroke)
GW->>TC: sendKeys(data, paneId)
TC->>GW: sendCommand("send-keys …")
GW-->>GTV: ghostty_surface_text() → PTY
Note over GTV,TM: Disconnection (EXIT event)
GTV->>GW: handleGlobalAction(TMUX_EXIT, surface)
GW->>TC: handleEvent(.exit)
TC->>TPC: teardown() all clients
TC->>TM: unburyGatewayWorkspace()
GW->>GW: activeGateways.removeValue(panelId)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| [submodule "ghostty"] | ||
| path = ghostty | ||
| url = https://github.com/manaflow-ai/ghostty.git | ||
| url = https://github.com/jamesainslie/ghostty.git |
Contributor
There was a problem hiding this comment.
The ghostty submodule URL has been changed from https://github.com/manaflow-ai/ghostty.git (the organisation fork) to https://github.com/jamesainslie/ghostty.git (the PR author's personal fork). Other contributors and CI will now clone from the personal fork. Per the Ghostty submodule workflow in CLAUDE.md, changes should be committed and pushed to manaflow-ai/ghostty before updating the parent repo's submodule pointer. This should be reverted to the org fork URL (or the fork changes should be merged there first).
Suggested change
| url = https://github.com/jamesainslie/ghostty.git | |
| url = https://github.com/manaflow-ai/ghostty.git |
| DispatchQueue.main.asyncAfter( | ||
| deadline: .now() + Self.commandTimeoutInterval, | ||
| execute: work | ||
| ) |
Contributor
There was a problem hiding this comment.
paneToClient typed as [Int: AnyObject] with stale comment
The comment // TmuxPaneClient in Phase 2 is now out of date — TmuxPaneClient is fully implemented and used. Using AnyObject here requires casts at every access site and loses type safety. Since the map is only ever populated with TmuxPaneClient instances, it should be typed directly:
Suggested change
| ) | |
| private(set) var paneToClient: [Int: TmuxPaneClient] = [:] |
All call sites that cast clientObj as? TmuxPaneClient can then be simplified to direct access.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 24 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:8122">
P2: `index` is now derived from `visibleTabs`, but `TabItemView` still uses it as an index into `tabManager.tabs` (move/reorder and shift-selection). If any hidden `isBuriedGateway` tabs exist, indices no longer align and move/selection/reorder will target the wrong workspaces.</violation>
</file>
<file name="Sources/TabManager.swift">
<violation number="1" location="Sources/TabManager.swift:5004">
P2: Tmux restore rebuilds tabs with tmux gateways appended at the end, but selection is restored by the old snapshot index, causing wrong tab selection/order after relaunch.</violation>
</file>
<file name="Sources/Tmux/TmuxController.swift">
<violation number="1" location="Sources/Tmux/TmuxController.swift:193">
P2: Mutating `windowToWorkspace` while iterating it can trap at runtime; `removeWorkspaceForWindow` removes from the dictionary inside the loop.</violation>
<violation number="2" location="Sources/Tmux/TmuxController.swift:296">
P1: Dictionary `paneToClient` is mutated during its own iteration, which can cause a runtime trap during window cleanup.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| guard let workspaceId = windowToWorkspace[windowId] else { return } | ||
|
|
||
| // Tear down pane clients for this window | ||
| for (paneId, clientObj) in paneToClient { |
There was a problem hiding this comment.
P1: Dictionary paneToClient is mutated during its own iteration, which can cause a runtime trap during window cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Tmux/TmuxController.swift, line 296:
<comment>Dictionary `paneToClient` is mutated during its own iteration, which can cause a runtime trap during window cleanup.</comment>
<file context>
@@ -0,0 +1,482 @@
+ guard let workspaceId = windowToWorkspace[windowId] else { return }
+
+ // Tear down pane clients for this window
+ for (paneId, clientObj) in paneToClient {
+ if let client = clientObj as? TmuxPaneClient, client.tmuxWindowId == windowId {
+ client.teardown()
</file context>
|
|
||
| LazyVStack(spacing: tabRowSpacing) { | ||
| ForEach(Array(tabManager.tabs.enumerated()), id: \.element.id) { index, tab in | ||
| ForEach(Array(visibleTabs.enumerated()), id: \.element.id) { index, tab in |
There was a problem hiding this comment.
P2: index is now derived from visibleTabs, but TabItemView still uses it as an index into tabManager.tabs (move/reorder and shift-selection). If any hidden isBuriedGateway tabs exist, indices no longer align and move/selection/reorder will target the wrong workspaces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 8122:
<comment>`index` is now derived from `visibleTabs`, but `TabItemView` still uses it as an index into `tabManager.tabs` (move/reorder and shift-selection). If any hidden `isBuriedGateway` tabs exist, indices no longer align and move/selection/reorder will target the wrong workspaces.</comment>
<file context>
@@ -8118,7 +8119,7 @@ struct VerticalTabsSidebar: View {
LazyVStack(spacing: tabRowSpacing) {
- ForEach(Array(tabManager.tabs.enumerated()), id: \.element.id) { index, tab in
+ ForEach(Array(visibleTabs.enumerated()), id: \.element.id) { index, tab in
let selectedContextIds: Set<UUID> = selectedTabIds.contains(tab.id) ? selectedTabIds : [tab.id]
let contextTargetIds = tabManager.tabs.compactMap { workspace in
</file context>
| ) | ||
| workspace.owningTabManager = self | ||
| wireClosedBrowserTracking(for: workspace) | ||
| newTabs.append(workspace) |
There was a problem hiding this comment.
P2: Tmux restore rebuilds tabs with tmux gateways appended at the end, but selection is restored by the old snapshot index, causing wrong tab selection/order after relaunch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/TabManager.swift, line 5004:
<comment>Tmux restore rebuilds tabs with tmux gateways appended at the end, but selection is restored by the old snapshot index, causing wrong tab selection/order after relaunch.</comment>
<file context>
@@ -4945,6 +4987,23 @@ extension TabManager {
+ )
+ workspace.owningTabManager = self
+ wireClosedBrowserTracking(for: workspace)
+ newTabs.append(workspace)
+ }
+
</file context>
Comment on lines
+193
to
+195
| for (windowId, _) in windowToWorkspace where !activeWindowIds.contains(windowId) { | ||
| removeWorkspaceForWindow(windowId) | ||
| } |
There was a problem hiding this comment.
P2: Mutating windowToWorkspace while iterating it can trap at runtime; removeWorkspaceForWindow removes from the dictionary inside the loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Tmux/TmuxController.swift, line 193:
<comment>Mutating `windowToWorkspace` while iterating it can trap at runtime; `removeWorkspaceForWindow` removes from the dictionary inside the loop.</comment>
<file context>
@@ -0,0 +1,482 @@
+
+ // Remove workspaces for windows that no longer exist
+ let activeWindowIds = Set(windows.map(\.id))
+ for (windowId, _) in windowToWorkspace where !activeWindowIds.contains(windowId) {
+ removeWorkspaceForWindow(windowId)
+ }
</file context>
Suggested change
| for (windowId, _) in windowToWorkspace where !activeWindowIds.contains(windowId) { | |
| removeWorkspaceForWindow(windowId) | |
| } | |
| let obsoleteWindowIds = windowToWorkspace.keys.filter { !activeWindowIds.contains($0) } | |
| for windowId in obsoleteWindowIds { | |
| removeWorkspaceForWindow(windowId) | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
8208-8238:⚠️ Potential issue | 🟠 MajorDon't reuse the filtered sidebar index as the raw workspace index.
Line 8221 now enumerates
visibleTabs, butTabItemViewstill usesindexagainsttabManager.tabsfor shift-range selection, move up/down, and reorder targets.lastSidebarSelectionIndexis also still seeded from rawtabManager.tabs.firstIndex(...)elsewhere in this file, so once a buried gateway exists those actions will target the wrong workspace. The shortcut badge can drift for the same reason: digit routing inSources/AppDelegate.swift:9481-9488andSources/cmuxApp.swift:814-819still maps overmanager.tabs.count, andSources/TabManager.swift:3089-3095selects the unfiltered index. Pass a raw index separately (or resolve bytab.id) so the sidebar and shortcut routing share one ordering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 8208 - 8238, The visibleTabs enumeration creates a filtered index that must not be used as the raw workspace index; compute the unfiltered/raw index for each tab (e.g. find rawIndex = tabManager.tabs.firstIndex(where: { $0.id == tab.id })) and pass that rawIndex into TabItemView (instead of the filtered `index`) and into workspaceShortcutDigit/WorkspaceShortcutMapper.commandDigitForWorkspace so shift-range selection, move up/down, reorder targets and lastSidebarSelectionIndex all use the same unfiltered ordering; alternatively resolve actions by `tab.id` everywhere (ensure TabItemView, lastSidebarSelectionIndex seeding, and shortcut routing consistently use the raw index or id).
🧹 Nitpick comments (4)
Sources/Tmux/TmuxLayoutEngine.swift (1)
67-78: Consider adding a debug assertion for the empty children case.The guard correctly handles an invalid state, but silently returning a dummy leaf (paneId: -1) could mask bugs. A debug assertion would help catch unexpected states during development.
💡 Optional: add debug assertion
private static func foldChildren( _ children: [TmuxLayoutNode], orientation: BinaryNode.SplitOrientation ) -> BinaryNode { guard !children.isEmpty else { // Should never happen in valid tmux layouts + `#if` DEBUG + assertionFailure("TmuxLayoutEngine: foldChildren called with empty children") + `#endif` return .leaf(paneId: -1, width: 0, height: 0, x: 0, y: 0) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Tmux/TmuxLayoutEngine.swift` around lines 67 - 78, In foldChildren(_:orientation:) add a debug assertion before returning the sentinel dummy leaf to catch invalid states in development: replace the silent guard-return path for empty children with an assertionFailure (or assert) that includes a clear message like "foldChildren called with empty children" and any relevant context (orientation), then still return the existing .leaf(paneId: -1, width: 0, height: 0, x: 0, y: 0) to preserve behavior in release builds; reference the function foldChildren and the dummy leaf return when locating where to add the assertion.cmuxTests/TmuxTests.swift (1)
529-533: Use a real suffixed tmux version in this capability fixture.tmux commonly reports versions like
3.5a, butversionComparedrops non-numeric suffixes, sotestVersion35aSupportsAllFeaturescurrently only proves3.5. Either normalize suffixes in the helper or add a fixture that passes an actual suffixed version string.Also applies to: 653-665
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TmuxTests.swift` around lines 529 - 533, The test fixture uses versionCompare("3.5", ...) but should simulate a real suffixed tmux version; update the TmuxCapabilities versionCheck closure in testVersion35aSupportsAllFeatures to call versionCompare("3.5a", atLeast: minimum) (or adjust the versionCompare helper to normalize/strip suffixes before comparison) so the test actually exercises a suffixed version; apply the same change to the other fixtures referenced around the test (the cases covering lines 653-665) that intend to use suffixed versions.Sources/TabManager.swift (1)
2213-2231: Extract the tmux detach/close decision into a shared helper.This branch is the only close flow that bypasses
confirmCloseHandler, and the bulk-close path still jumps straight tocloseWorkspaceIfRunningProcess(...)instead of reusing the tmux logic. Centralizing the decision would keep single-close, multi-close, and tests consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TabManager.swift` around lines 2213 - 2231, The tmux detach/close prompt logic duplicated here (checking workspace.tmuxControllerId, TmuxController.controller(forId:), showing NSAlert, and calling controller.detach()) should be moved into a shared helper (e.g., a new function like confirmDetachFromTmux(for workspace: Workspace) -> Bool or handleTmuxCloseDecision(for:workspace:, completion:)), and existing callers (single-close path here, the bulk-close path that currently calls closeWorkspaceIfRunningProcess(...), and unit tests) should call that helper instead of duplicating the logic; ensure the helper returns a decision or invokes a completion so callers can proceed to closeWorkspaceIfRunningProcess(...) only when appropriate, and update tests to exercise the new helper rather than bypassing confirmCloseHandler.Sources/Tmux/TmuxController.swift (1)
418-420: Usedloginstead ofThis is DEBUG-only instrumentation, so it should go through the shared debug event log instead of stdout.
As per coding guidelines, "All debug events (keys, mouse, focus, splits, tabs) in DEBUG builds go to the debug event log. Use free function `dlog("message")` to log events. Wrap all call sites in `#if DEBUG` / `#endif`. The entire implementation is wrapped in `#if DEBUG`."🪵 Suggested fix
`#if` DEBUG - print("[tmux] command timeout after \(Self.commandTimeoutInterval)s — tmux may be unresponsive") + dlog("[tmux] command timeout after \(Self.commandTimeoutInterval)s — tmux may be unresponsive") `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Tmux/TmuxController.swift` around lines 418 - 420, Replace the debug-only stdout print in the TmuxController timeout handler with the shared debug event logger: inside the existing `#if` DEBUG / `#endif` block, swap print("[tmux] command timeout after \(Self.commandTimeoutInterval)s — tmux may be unresponsive") for dlog("[tmux] command timeout after \(Self.commandTimeoutInterval)s — tmux may be unresponsive"); keep the same interpolated message and DEBUG guard so the event goes to the debug event log (reference: TmuxController and Self.commandTimeoutInterval).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitmodules:
- Line 3: Update the ghostty submodule URL to point to the canonical
organization fork instead of the personal fork: locate the "ghostty" submodule
entry in .gitmodules and replace the url value
"https://github.com/jamesainslie/ghostty.git" with the organization fork URL
(e.g., the manaflow-ai/ghostty repository), then run git submodule sync && git
submodule update --init --recursive to ensure the change is applied and
committed; ensure subsequent ghostty changes are made and pushed inside the
ghostty submodule itself.
In `@ghostty`:
- Line 1: The submodule update for ghostty is on a detached HEAD and must be
pushed to the manaflow-ai/ghostty remote main branch and documented; check out
the ghostty submodule commit locally, create/force-update a branch or push the
detached commit to the remote main (git push manaflow-ai HEAD:refs/heads/main)
so the submodule commit exists on manaflow-ai/ghostty main, then update
docs/ghostty-fork.md to describe the tmux control mode API additions and any
other fork-specific changes introduced by this update (mention relevant API
names and behaviors) before updating the parent repo pointer and merging.
In `@Sources/cmuxApp.swift`:
- Around line 447-448: The CommandMenu usage calls String(localized:
"menu.tmux.title", ...) and "menu.tmux.detach" which are missing from the
xcstrings catalog; open Resources/Localizable.xcstrings and add entries for
"menu.tmux.title" and "menu.tmux.detach" with English and Japanese translations
(provide the English defaults "tmux" and "Detach" and equivalent Japanese
strings), save the file and ensure the keys match exactly so CommandMenu and
Button find the localized values at runtime.
- Line 452: Add the missing localization entries "menu.tmux.title" and
"menu.tmux.detach" to Resources/Localizable.xcstrings with English and Japanese
translations, and make the menu's disabled state reactive by subscribing to the
tmux connection publisher: replace the static use of
TmuxController.hasActiveConnection with a reactive observer (for example use
.onReceive(TmuxController.shared.$activeControllers) or .onChange(of:
TmuxController.shared.activeControllers) on the Button or surrounding Menu) so
the disabled binding updates when TmuxController.activeControllers (or the
published connection state) changes.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2556-2568: The surface config may inherit manual I/O fields from
configTemplate when manualIOConfig is nil; in createSurface(for:) reset the I/O
fields on the copied surfaceConfig to the PTY defaults (clear io_mode, set
io_write_cb and io_write_userdata to their non-manual/NULL defaults) before
applying any manualIOConfig override so manualIOConfig/isManualIOMode remains
the single source of truth; update the same reset logic for the second
occurrence referenced around the lines noted (similar to 3152-3157).
In `@Sources/TabManager.swift`:
- Around line 4910-4924: The code collapses multiple tmux snapshots by tracking
seenTmuxControllers and building restorableTabs, but loses original positions so
selectedWorkspaceIndex can point to the wrong tab; update the collapse logic to
record the original snapshot index of the first seen tmux controller (e.g. store
a mapping from controller UUID to its first index) and when building
restorableTabs insert that first tmux gateway at its original snapshot position
instead of just appending later; then adjust any selection mapping logic
(selectedWorkspaceIndex) to map to the preserved in-place entry for that
controller (use the controller->firstIndex map) — apply the same fix in the
analogous block around the code handling restoration at the later range (the
block referenced at 4964-5005).
In `@Sources/Tmux/TmuxController.swift`:
- Around line 198-213: The method handleLayoutChange decodes the layout but then
discards it ("_ = layout"), so runtime split/resize/add events never update the
workspace tree or pane mappings; replace that no-op with logic to reconcile the
decoded TmuxLayoutNode with the workspace identified by workspaceId: traverse
the layout tree, update the workspace/tab structure in tabManager (e.g., call or
implement a function like applyLayoutToWorkspace(workspaceId: Int, layout:
TmuxLayoutNode) or tabManager?.applyLayout(workspaceId:workspaceId,
layout:layout)), and update paneToClient by adding mappings for newly discovered
pane IDs and removing mappings for panes that no longer exist; ensure any
created/removed panes update client state and output routing so cmux stays in
sync with tmux.
- Around line 270-273: The code is auto-selecting the first workspace during
passive sync by passing select: windowToWorkspace.isEmpty to
tabManager.addWorkspace; change this so passive/restore paths never mutate
selection—only explicit focus-intent code should. Update the call in
TmuxController to pass select: false (or a newly-propagated explicitFocusIntent
boolean) instead of windowToWorkspace.isEmpty, and if needed thread an explicit
focus flag from the caller into the place that calls tabManager.addWorkspace so
only intentional “open/focus” actions can pass true. Ensure references:
TmuxController, tabManager.addWorkspace, and windowToWorkspace are updated
accordingly.
- Around line 171-179: Read the existing tmux session option `@cmux_id` before you
overwrite it: call the routine that queries the current session option (instead
of immediately invoking gateway.sendCommand("set-option -s `@cmux_id`
\(id.uuidString)\n")), capture that value and pass it into checkDoubleAttach()
(or let checkDoubleAttach() perform the read) so checkDoubleAttach() observes
the previous owner; only after probing and handling double-attach should you
persist the new controller ID with gateway.sendCommand, while keeping
enablePauseModeIfSupported() behavior unchanged.
- Around line 436-457: In teardown(), before clearing pane/window maps, iterate
the workspaces referenced in windowToWorkspace and tell the tabManager to remove
each workspace so the sidebar doesn't retain dead tabs; specifically, after
unburyGatewayWorkspace() and before clearing
paneToClient/paneToPanelId/windowToWorkspace, call
tabManager.removeWorkspace(...) (or the appropriate method on tabManager) for
each workspace in windowToWorkspace, then proceed with tearing down pane clients
and removing the maps.
- Around line 392-412: The command-timeout logic is never used; call
resetCommandTimeout() before sending any tmux command and call
cancelCommandTimeout() when tmux responses/events arrive so
handleCommandTimeout() can fire on timeout. Concretely, introduce (or use) a
central sendCommand(_:) wrapper that invokes resetCommandTimeout() then
delegates to gateway.sendCommand(...), replace existing direct
gateway.sendCommand(...) call sites to route through sendCommand(_:), and add
cancelCommandTimeout() to the tmux response/event ingress path (the method
handling inbound tmux data) so the timer is cancelled whenever a response is
received; look for resetCommandTimeout(), cancelCommandTimeout(),
handleCommandTimeout(), sendCommand(_:), and gateway.sendCommand to locate where
to wire these calls.
In `@Sources/Tmux/TmuxGateway.swift`:
- Around line 109-119: The controller is being wired to
AppDelegate.shared?.tabManager (the active-window manager) which may be wrong
for this surface; instead resolve the owning TabManager from the surface/panel
identity first. In the block that creates TmuxGateway and TmuxController
(symbols: TmuxGateway, TmuxController, gateway.controller,
controller.gatewaySurface, controller.tabManager, activeGateways, panelId,
surface), call AppDelegate.shared?.locateSurface(surfaceId:) (or equivalent
surface lookup) to get the correct TabManager for this surface and assign that
to controller.tabManager, and only fall back to AppDelegate.shared?.tabManager
if locateSurface returns nil. Ensure you still set gateway.controller,
controller.gatewaySurface, and insert activeGateways[panelId] = gateway.
In `@Sources/Tmux/TmuxKeyEncoder.swift`:
- Around line 29-33: In TmuxKeyEncoder.literalCharacterSet remove the insertion
of space (0x20) from the literalCharacterSet so spaces are not treated as
literal characters; instead ensure spaces are encoded as hex (0x20) by the
encoder or handled by quoting the literal argument when building the send-keys
command; also remove any other set.insert(0x20) occurrences in the same
file/class so the send-keys -lt path emits "hello world" as a single
quoted/hex-encoded argument rather than separate arguments.
In `@Sources/Tmux/TmuxPaneClient.swift`:
- Around line 38-44: In teardown(), ensure the C callback cannot dereference a
freed object by clearing or invalidating the Manual I/O callback before
releasing the retained self: explicitly set surface.manualIOConfig = nil (or
otherwise replace writeCallback/userdata with safe no-op values) prior to
calling retainedSelf?.release() so that ioWriteCallback will not be invoked with
userdata pointing at a released Unmanaged instance; alternatively, postpone
releasing the Unmanaged.passRetained(self) (retainedSelf) until after the
surface is fully torn down and freed. Reference: ioWriteCallback, retainedSelf,
teardown(), surface.manualIOConfig, ManualIOConfig,
Unmanaged.passRetained(self).
In `@Sources/Workspace.swift`:
- Around line 154-155: The current logic sets effectiveIncludeScrollback =
tmuxControllerId == nil && includeScrollback which disables scrollback whenever
a tmuxControllerId exists even if tmuxInfo lookup failed; change the condition
to include scrollback when includeScrollback is true and either there's no tmux
controller or the controller lookup produced no tmuxInfo (i.e.
effectiveIncludeScrollback = includeScrollback && (tmuxControllerId == nil ||
tmuxInfo == nil)). Update the same conditional logic in the other occurrences
referenced (around the 201–212 block) so snapshots retain scrollback when
tmuxInfo is nil and ensure reconnect metadata handling is still preserved.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 8208-8238: The visibleTabs enumeration creates a filtered index
that must not be used as the raw workspace index; compute the unfiltered/raw
index for each tab (e.g. find rawIndex = tabManager.tabs.firstIndex(where: {
$0.id == tab.id })) and pass that rawIndex into TabItemView (instead of the
filtered `index`) and into
workspaceShortcutDigit/WorkspaceShortcutMapper.commandDigitForWorkspace so
shift-range selection, move up/down, reorder targets and
lastSidebarSelectionIndex all use the same unfiltered ordering; alternatively
resolve actions by `tab.id` everywhere (ensure TabItemView,
lastSidebarSelectionIndex seeding, and shortcut routing consistently use the raw
index or id).
---
Nitpick comments:
In `@cmuxTests/TmuxTests.swift`:
- Around line 529-533: The test fixture uses versionCompare("3.5", ...) but
should simulate a real suffixed tmux version; update the TmuxCapabilities
versionCheck closure in testVersion35aSupportsAllFeatures to call
versionCompare("3.5a", atLeast: minimum) (or adjust the versionCompare helper to
normalize/strip suffixes before comparison) so the test actually exercises a
suffixed version; apply the same change to the other fixtures referenced around
the test (the cases covering lines 653-665) that intend to use suffixed
versions.
In `@Sources/TabManager.swift`:
- Around line 2213-2231: The tmux detach/close prompt logic duplicated here
(checking workspace.tmuxControllerId, TmuxController.controller(forId:), showing
NSAlert, and calling controller.detach()) should be moved into a shared helper
(e.g., a new function like confirmDetachFromTmux(for workspace: Workspace) ->
Bool or handleTmuxCloseDecision(for:workspace:, completion:)), and existing
callers (single-close path here, the bulk-close path that currently calls
closeWorkspaceIfRunningProcess(...), and unit tests) should call that helper
instead of duplicating the logic; ensure the helper returns a decision or
invokes a completion so callers can proceed to
closeWorkspaceIfRunningProcess(...) only when appropriate, and update tests to
exercise the new helper rather than bypassing confirmCloseHandler.
In `@Sources/Tmux/TmuxController.swift`:
- Around line 418-420: Replace the debug-only stdout print in the TmuxController
timeout handler with the shared debug event logger: inside the existing `#if`
DEBUG / `#endif` block, swap print("[tmux] command timeout after
\(Self.commandTimeoutInterval)s — tmux may be unresponsive") for dlog("[tmux]
command timeout after \(Self.commandTimeoutInterval)s — tmux may be
unresponsive"); keep the same interpolated message and DEBUG guard so the event
goes to the debug event log (reference: TmuxController and
Self.commandTimeoutInterval).
In `@Sources/Tmux/TmuxLayoutEngine.swift`:
- Around line 67-78: In foldChildren(_:orientation:) add a debug assertion
before returning the sentinel dummy leaf to catch invalid states in development:
replace the silent guard-return path for empty children with an assertionFailure
(or assert) that includes a clear message like "foldChildren called with empty
children" and any relevant context (orientation), then still return the existing
.leaf(paneId: -1, width: 0, height: 0, x: 0, y: 0) to preserve behavior in
release builds; reference the function foldChildren and the dummy leaf return
when locating where to add the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ff60dec-a14e-45c0-9949-d10a55f65867
📒 Files selected for processing (20)
.gitmodulesGhosttyTabs.xcodeproj/project.pbxprojSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Panels/TerminalPanel.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/Tmux/TmuxController.swiftSources/Tmux/TmuxGateway.swiftSources/Tmux/TmuxKeyEncoder.swiftSources/Tmux/TmuxLayoutEngine.swiftSources/Tmux/TmuxPaneClient.swiftSources/Tmux/TmuxTypes.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmuxTests/BrowserConfigTests.swiftcmuxTests/TmuxTests.swiftcmuxTests/WindowAndDragTests.swiftghosttyghostty.h
💤 Files with no reviewable changes (1)
- cmuxTests/WindowAndDragTests.swift
| [submodule "ghostty"] | ||
| path = ghostty | ||
| url = https://github.com/manaflow-ai/ghostty.git | ||
| url = https://github.com/jamesainslie/ghostty.git |
There was a problem hiding this comment.
Use the canonical org fork for ghostty submodule URL.
Line 3 switches ghostty to a personal fork, which can break reproducible checkouts/CI and team submodule workflow expectations. Please keep the submodule URL on the organization fork.
🔧 Proposed fix
- url = https://github.com/jamesainslie/ghostty.git
+ url = https://github.com/manaflow-ai/ghostty.gitBased on learnings: “Ghostty changes must be committed in the ghostty submodule and pushed to the manaflow-ai/ghostty fork.”
📝 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
| url = https://github.com/jamesainslie/ghostty.git | |
| url = https://github.com/manaflow-ai/ghostty.git |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitmodules at line 3, Update the ghostty submodule URL to point to the
canonical organization fork instead of the personal fork: locate the "ghostty"
submodule entry in .gitmodules and replace the url value
"https://github.com/jamesainslie/ghostty.git" with the organization fork URL
(e.g., the manaflow-ai/ghostty repository), then run git submodule sync && git
submodule update --init --recursive to ensure the change is applied and
committed; ensure subsequent ghostty changes are made and pushed inside the
ghostty submodule itself.
| @@ -1 +1 @@ | |||
| Subproject commit bc9be90a21997a4e5f06bf15ae2ec0f937c2dc42 | |||
| Subproject commit d0ac04eac99a41a06a7f87f12566af2f3f093b35 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the ghostty submodule commit is pushed to remote main
cd ghostty || exit 1
# Check if the current commit is an ancestor of origin/main
if git merge-base --is-ancestor HEAD origin/main 2>/dev/null; then
echo "✓ Submodule commit is on remote main"
exit 0
else
echo "✗ Submodule commit is NOT on remote main (detached HEAD or unpushed branch)"
exit 1
fiRepository: manaflow-ai/cmux
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Description: Check if docs/ghostty-fork.md mentions tmux or recent updates
if [ -f docs/ghostty-fork.md ]; then
echo "=== Checking docs/ghostty-fork.md for tmux-related updates ==="
rg -i "tmux|control.mode|d0ac04ea" docs/ghostty-fork.md || echo "No tmux-related content found in fork docs"
else
echo "⚠️ docs/ghostty-fork.md does not exist"
fiRepository: manaflow-ai/cmux
Length of output: 165
Push the ghostty commit to remote main and update docs/ghostty-fork.md before merging.
The submodule commit is currently on a detached HEAD and has not been pushed to the manaflow-ai/ghostty remote main branch. Per submodule best practices, this must be pushed before updating the parent repo pointer.
Additionally, docs/ghostty-fork.md has not been updated to document the tmux control mode API additions and other fork-specific changes introduced in this update.
Please:
- Push the ghostty commit to the
manaflow-ai/ghosttyrepository'smainbranch - Update
docs/ghostty-fork.mdto document the tmux control mode changes and any other fork modifications
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ghostty` at line 1, The submodule update for ghostty is on a detached HEAD
and must be pushed to the manaflow-ai/ghostty remote main branch and documented;
check out the ghostty submodule commit locally, create/force-update a branch or
push the detached commit to the remote main (git push manaflow-ai
HEAD:refs/heads/main) so the submodule commit exists on manaflow-ai/ghostty
main, then update docs/ghostty-fork.md to describe the tmux control mode API
additions and any other fork-specific changes introduced by this update (mention
relevant API names and behaviors) before updating the parent repo pointer and
merging.
Comment on lines
+447
to
+448
| CommandMenu(String(localized: "menu.tmux.title", defaultValue: "tmux")) { | ||
| Button(String(localized: "menu.tmux.detach", defaultValue: "Detach")) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
XCSTR=$(fd -t f 'Localizable\.xcstrings$' Resources | head -n 1)
test -n "${XCSTR}"
jq '.strings["menu.tmux.title"], .strings["menu.tmux.detach"]' "$XCSTR"Repository: manaflow-ai/cmux
Length of output: 69
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the xcstrings file
XCSTR=$(fd -t f 'Localizable\.xcstrings$' Resources | head -n 1)
if [ -z "$XCSTR" ]; then
echo "ERROR: Localizable.xcstrings not found"
exit 1
fi
echo "=== File found: $XCSTR ==="
echo "=== File size ==="
wc -l "$XCSTR"
echo "=== Root keys in JSON ==="
jq 'keys' "$XCSTR" | head -20
echo "=== Checking for any tmux-related keys ==="
jq '.strings | keys[] | select(contains("tmux"))' "$XCSTR" 2>/dev/null || echo "No tmux keys found"
echo "=== Checking .strings key count ==="
jq '.strings | length' "$XCSTR"
echo "=== Verifying our target keys ==="
jq '.strings | has("menu.tmux.title"), has("menu.tmux.detach")' "$XCSTR"Repository: manaflow-ai/cmux
Length of output: 371
Add menu.tmux.title and menu.tmux.detach keys to Resources/Localizable.xcstrings.
The code at lines 447–448 references localization keys "menu.tmux.title" and "menu.tmux.detach" that are not present in the xcstrings catalog. Both keys must be added with English and Japanese translations to satisfy the localization requirement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` around lines 447 - 448, The CommandMenu usage calls
String(localized: "menu.tmux.title", ...) and "menu.tmux.detach" which are
missing from the xcstrings catalog; open Resources/Localizable.xcstrings and add
entries for "menu.tmux.title" and "menu.tmux.detach" with English and Japanese
translations (provide the English defaults "tmux" and "Detach" and equivalent
Japanese strings), save the file and ensure the keys match exactly so
CommandMenu and Button find the localized values at runtime.
| TmuxController.detachAll() | ||
| } | ||
| .keyboardShortcut("d", modifiers: [.control, .command]) | ||
| .disabled(!TmuxController.hasActiveConnection) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify how hasActiveConnection is defined and whether changes are observable.
fd -t f 'TmuxController.swift$' Sources
rg -n -C3 '\bhasActiveConnection\b|@Published|ObservableObject|objectWillChange|NotificationCenter|detachAll\s*\(' Sources/Tmux/TmuxController.swift Sources/cmuxApp.swiftRepository: manaflow-ai/cmux
Length of output: 6708
🏁 Script executed:
fd -t f 'Localizable.xcstrings' ResourcesRepository: manaflow-ai/cmux
Length of output: 91
🏁 Script executed:
rg -n 'menu\.tmux\.' Resources/Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
rg -n -B5 -A5 'CommandMenu.*menu.tmux' Sources/cmuxApp.swiftRepository: manaflow-ai/cmux
Length of output: 613
🏁 Script executed:
cat Resources/Localizable.xcstrings | head -100Repository: manaflow-ai/cmux
Length of output: 2215
🏁 Script executed:
rg '"menu\.tmux' Resources/Localizable.xcstringsRepository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
wc -l Resources/Localizable.xcstrings && rg -i 'tmux' Resources/Localizable.xcstringsRepository: manaflow-ai/cmux
Length of output: 1232
🏁 Script executed:
rg '"menu\.tmux\.(title|detach)"' Resources/Localizable.xcstringsRepository: manaflow-ai/cmux
Length of output: 42
Add missing localization keys and fix non-reactive disabled state.
The tmux menu references localization keys "menu.tmux.title" and "menu.tmux.detach" (lines 447–448), but these keys are absent from Resources/Localizable.xcstrings. Add both keys with English and Japanese translations.
Additionally, line 452 reads TmuxController.hasActiveConnection without any subscription mechanism. The disabled state will not update reactively when the tmux connection state changes. Subscribe to connection state changes using .onReceive() on the button or wrap the menu in .onChange() to observe TmuxController.activeControllers or a similar published source.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` at line 452, Add the missing localization entries
"menu.tmux.title" and "menu.tmux.detach" to Resources/Localizable.xcstrings with
English and Japanese translations, and make the menu's disabled state reactive
by subscribing to the tmux connection publisher: replace the static use of
TmuxController.hasActiveConnection with a reactive observer (for example use
.onReceive(TmuxController.shared.$activeControllers) or .onChange(of:
TmuxController.shared.activeControllers) on the Button or surrounding Menu) so
the disabled binding updates when TmuxController.activeControllers (or the
published connection state) changes.
Comment on lines
+2556
to
+2568
| /// Configuration for Manual I/O mode (used by tmux virtual surfaces). | ||
| /// When set, the surface is created without a PTY; keystrokes are routed | ||
| /// through the write callback instead of a child process. | ||
| struct ManualIOConfig { | ||
| let writeCallback: ghostty_io_write_cb | ||
| let userdata: UnsafeMutableRawPointer | ||
| } | ||
|
|
||
| /// Set before surface creation to enable Manual I/O mode. | ||
| var manualIOConfig: ManualIOConfig? | ||
|
|
||
| /// Whether this surface is in Manual I/O mode (tmux virtual pane). | ||
| var isManualIOMode: Bool { manualIOConfig != nil } |
There was a problem hiding this comment.
Clear inherited I/O fields when manualIOConfig is unset.
These lines make manualIOConfig the switch for manual mode, but createSurface(for:) only overwrites the io_* fields inside the if let manualIO block. Because surfaceConfig is copied from configTemplate earlier in the method, a template taken from a tmux pane can leave io_mode, io_write_cb, or io_write_userdata behind even when manualIOConfig == nil. That can create the new surface in the wrong I/O mode or keep a stale raw userdata pointer alive. Reset those fields to the normal PTY defaults before applying the manual override so manualIOConfig stays the single source of truth.
Also applies to: 3152-3157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 2556 - 2568, The surface
config may inherit manual I/O fields from configTemplate when manualIOConfig is
nil; in createSurface(for:) reset the I/O fields on the copied surfaceConfig to
the PTY defaults (clear io_mode, set io_write_cb and io_write_userdata to their
non-manual/NULL defaults) before applying any manualIOConfig override so
manualIOConfig/isManualIOMode remains the single source of truth; update the
same reset logic for the second occurrence referenced around the lines noted
(similar to 3152-3157).
Comment on lines
+436
to
+457
| private func teardown() { | ||
| connectionState = .disconnected | ||
|
|
||
| resizeDebounceTimer?.cancel() | ||
| resizeDebounceTimer = nil | ||
| commandTimeoutTimer?.cancel() | ||
| commandTimeoutTimer = nil | ||
| for (_, timer) in unpauseTimers { timer.cancel() } | ||
| unpauseTimers.removeAll() | ||
| pausedPanes.removeAll() | ||
|
|
||
| unburyGatewayWorkspace() | ||
|
|
||
| // Tear down all pane clients | ||
| for (_, clientObj) in paneToClient { | ||
| (clientObj as? TmuxPaneClient)?.teardown() | ||
| } | ||
|
|
||
| // Clean up entity maps | ||
| paneToClient.removeAll() | ||
| paneToPanelId.removeAll() | ||
| windowToWorkspace.removeAll() |
There was a problem hiding this comment.
Remove the tmux workspaces before clearing the controller maps.
This tears down pane clients and clears windowToWorkspace, but it never removes those workspaces from tabManager. After detach/exit, the sidebar can retain dead tmux tabs with no backing controller.
🧹 Suggested fix
- // Tear down all pane clients
- for (_, clientObj) in paneToClient {
- (clientObj as? TmuxPaneClient)?.teardown()
- }
-
- // Clean up entity maps
- paneToClient.removeAll()
- paneToPanelId.removeAll()
- windowToWorkspace.removeAll()
+ for windowId in Array(windowToWorkspace.keys) {
+ removeWorkspaceForWindow(windowId)
+ }
+
+ paneToClient.removeAll()
+ paneToPanelId.removeAll()
+ windowToWorkspace.removeAll()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Tmux/TmuxController.swift` around lines 436 - 457, In teardown(),
before clearing pane/window maps, iterate the workspaces referenced in
windowToWorkspace and tell the tabManager to remove each workspace so the
sidebar doesn't retain dead tabs; specifically, after unburyGatewayWorkspace()
and before clearing paneToClient/paneToPanelId/windowToWorkspace, call
tabManager.removeWorkspace(...) (or the appropriate method on tabManager) for
each workspace in windowToWorkspace, then proceed with tearing down pane clients
and removing the maps.
Comment on lines
+109
to
+119
| if action.event == GHOSTTY_TMUX_ENTER { | ||
| // Create a new gateway and controller for this surface | ||
| let gateway = TmuxGateway() | ||
| let controller = TmuxController( | ||
| gatewayPanelId: panelId, | ||
| gateway: gateway | ||
| ) | ||
| gateway.controller = controller | ||
| controller.gatewaySurface = surface | ||
| controller.tabManager = AppDelegate.shared?.tabManager | ||
| activeGateways[panelId] = gateway |
There was a problem hiding this comment.
Resolve the owning TabManager from this surface, not from AppDelegate.shared?.tabManager.
AppDelegate.shared?.tabManager is the active-window manager, not necessarily the one that owns panelId. In multi-window tmux sessions this can wire the controller to the wrong window model. Resolve the manager from the surface/panel identity first, e.g. via locateSurface(surfaceId:).
Based on learnings: panel/surface-routed work must fall back to AppDelegate.shared?.locateSurface(surfaceId:) to find the correct TabManager across windows and avoid active-window bias.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Tmux/TmuxGateway.swift` around lines 109 - 119, The controller is
being wired to AppDelegate.shared?.tabManager (the active-window manager) which
may be wrong for this surface; instead resolve the owning TabManager from the
surface/panel identity first. In the block that creates TmuxGateway and
TmuxController (symbols: TmuxGateway, TmuxController, gateway.controller,
controller.gatewaySurface, controller.tabManager, activeGateways, panelId,
surface), call AppDelegate.shared?.locateSurface(surfaceId:) (or equivalent
surface lookup) to get the correct TabManager for this surface and assign that
to controller.tabManager, and only fall back to AppDelegate.shared?.tabManager
if locateSurface returns nil. Ensure you still set gateway.controller,
controller.gatewaySurface, and insert activeGateways[panelId] = gateway.
Comment on lines
+29
to
+33
| // Special literal characters: +/):,_ | ||
| for ch: UInt8 in [0x2B, 0x2F, 0x29, 0x3A, 0x2C, 0x5F] { set.insert(ch) } | ||
| // Space is also literal | ||
| set.insert(0x20) | ||
| return set |
There was a problem hiding this comment.
Space in literal batch breaks command parsing.
Including space (0x20) in literalCharacterSet causes incorrect behavior. When the literal batch contains spaces, the generated command:
send-keys -lt %0 hello world
is parsed by tmux as separate arguments hello and world, not as the string "hello world".
Either remove space from the literal set (encode it as 0x20 hex), or quote the literal argument.
🐛 Suggested fix: remove space from literal set
// Special literal characters: +/):,_
for ch: UInt8 in [0x2B, 0x2F, 0x29, 0x3A, 0x2C, 0x5F] { set.insert(ch) }
- // Space is also literal
- set.insert(0x20)
return setAlso applies to: 48-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Tmux/TmuxKeyEncoder.swift` around lines 29 - 33, In
TmuxKeyEncoder.literalCharacterSet remove the insertion of space (0x20) from the
literalCharacterSet so spaces are not treated as literal characters; instead
ensure spaces are encoded as hex (0x20) by the encoder or handled by quoting the
literal argument when building the send-keys command; also remove any other
set.insert(0x20) occurrences in the same file/class so the send-keys -lt path
emits "hello world" as a single quoted/hex-encoded argument rather than separate
arguments.
Comment on lines
+38
to
+44
| // Retain self for the C callback and configure Manual I/O | ||
| let retained = Unmanaged.passRetained(self) | ||
| self.retainedSelf = retained | ||
| surface.manualIOConfig = TerminalSurface.ManualIOConfig( | ||
| writeCallback: Self.ioWriteCallback, | ||
| userdata: retained.toOpaque() | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Manual I/O callback installation / clearing:"
rg -nP "manualIOConfig\s*=\s*nil|manualIOConfig" Sources/GhosttyTerminalView.swift Sources/Tmux -C 2
echo
echo "Callback userdata lifetime and teardown ordering:"
rg -n "ioWriteCallback|retainedSelf|ghostty_surface_free|teardown\(" Sources/Tmux/TmuxPaneClient.swift Sources/Tmux/TmuxController.swift -C 3Repository: manaflow-ai/cmux
Length of output: 6262
Callback userdata outlives its retained context in teardown().
The ioWriteCallback dereferences userdata via takeUnretainedValue(), but teardown() releases the retained self before freeing the surface. If the callback fires after retainedSelf?.release() but before the surface is fully torn down, it will dereference a freed object. Invalidate the callback (e.g., surface.manualIOConfig = nil) before releasing retainedSelf, or ensure the retained object stays alive until the surface is completely freed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Tmux/TmuxPaneClient.swift` around lines 38 - 44, In teardown(),
ensure the C callback cannot dereference a freed object by clearing or
invalidating the Manual I/O callback before releasing the retained self:
explicitly set surface.manualIOConfig = nil (or otherwise replace
writeCallback/userdata with safe no-op values) prior to calling
retainedSelf?.release() so that ioWriteCallback will not be invoked with
userdata pointing at a released Unmanaged instance; alternatively, postpone
releasing the Unmanaged.passRetained(self) (retainedSelf) until after the
surface is fully torn down and freed. Reference: ioWriteCallback, retainedSelf,
teardown(), surface.manualIOConfig, ManualIOConfig,
Unmanaged.passRetained(self).
Comment on lines
+154
to
+155
| // tmux workspaces skip scrollback — tmux re-sends content on reconnect | ||
| let effectiveIncludeScrollback = tmuxControllerId == nil && includeScrollback |
There was a problem hiding this comment.
Prevent snapshot data loss when tmux lookup fails.
effectiveIncludeScrollback is disabled as soon as tmuxControllerId exists, but tmuxInfo can still end up nil (missing controller / empty session name). In that case, the snapshot stores neither reconnect metadata nor scrollback.
💡 Proposed fix
func sessionSnapshot(includeScrollback: Bool) -> SessionWorkspaceSnapshot {
- // tmux workspaces skip scrollback — tmux re-sends content on reconnect
- let effectiveIncludeScrollback = tmuxControllerId == nil && includeScrollback
+ // Build tmux session info first so scrollback fallback is preserved if tmux metadata is unavailable.
+ let tmuxInfo: TmuxSessionInfo? = tmuxControllerId.flatMap { controllerId in
+ guard let controller = TmuxController.controller(forId: controllerId),
+ !controller.sessionName.isEmpty else {
+ return nil
+ }
+ return TmuxSessionInfo(
+ sessionName: controller.sessionName,
+ connectionCommand: "tmux -CC attach -t \(controller.sessionName)"
+ )
+ }
+ // tmux workspaces skip scrollback only when reconnect metadata is actually available.
+ let effectiveIncludeScrollback = includeScrollback && tmuxInfo == nil
@@
- // Build tmux session info if this workspace is managed by a tmux controller
- let tmuxInfo: TmuxSessionInfo? = tmuxControllerId.flatMap { controllerId in
- guard let controller = TmuxController.controller(forId: controllerId),
- !controller.sessionName.isEmpty else {
- return nil
- }
- return TmuxSessionInfo(
- sessionName: controller.sessionName,
- connectionCommand: "tmux -CC attach -t \(controller.sessionName)"
- )
- }Also applies to: 201-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 154 - 155, The current logic sets
effectiveIncludeScrollback = tmuxControllerId == nil && includeScrollback which
disables scrollback whenever a tmuxControllerId exists even if tmuxInfo lookup
failed; change the condition to include scrollback when includeScrollback is
true and either there's no tmux controller or the controller lookup produced no
tmuxInfo (i.e. effectiveIncludeScrollback = includeScrollback &&
(tmuxControllerId == nil || tmuxInfo == nil)). Update the same conditional logic
in the other occurrences referenced (around the 201–212 block) so snapshots
retain scrollback when tmuxInfo is nil and ensure reconnect metadata handling is
still preserved.
|
+1 on this feature, as a heavy tmux user this would be great |
teamleaderleo
added
S2: major
Collaborator
|
The repository already has remote tmux control mode (#5553), but this PR proposes a broader native integration; leaving open for scope and product review. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tmux -CC) integration, allowing cmux to act as a GUI frontend for tmux sessions#if/#endif, MainActor isolation, duplicate declarations)New files
Sources/Tmux/TmuxController.swift— manages tmux connection lifecycle, entity maps, event dispatchSources/Tmux/TmuxGateway.swift— bridges Ghostty C API actions to controller, write gatingSources/Tmux/TmuxTypes.swift— shared types (events, layout tree, capabilities, session info)Sources/Tmux/TmuxKeyEncoder.swift— literal/hex keystroke encoding with batch limitsSources/Tmux/TmuxLayoutEngine.swift— N-ary to binary layout conversion, layout diffingSources/Tmux/TmuxPaneClient.swift— virtual surface per tmux pane with Manual I/OcmuxTests/TmuxTests.swift— comprehensive unit testsModified files
ghosttysubmodule — tmux embedder API (pane_output, windows, layout_change actions)ghostty.h— C API types for tmux eventsGhosttyTerminalView.swift— action handler for GHOSTTY_ACTION_TMUX_CONTROL, ManualIOConfigWorkspace.swift— tmuxControllerId, isBuriedGateway, session snapshot filteringTabManager.swift— tmux workspace creation, reconnection, detach dialogContentView.swift— sidebar tmux badge (Equatable-safe)SessionPersistence.swift— TmuxSessionInfo in workspace snapshotscmuxApp.swift— tmux menu with Detach commandTerminalPanel.swift— isTmuxClient indicatorTest plan
CMUX_SKIP_ZIG_BUILD=1tmux -CCin tagged build triggers gateway detection and workspace creationtmux split-windowcreates native splitSummary by cubic
Adds native tmux control mode (
tmux -CC) so cmux can act as a GUI for tmux. Implements a protocol bridge, virtual panes with manual I/O, native layouts, and session reconnect. Rebased on latestmainand resolved test conflicts; no functional changes.New Features
send-keys; pane output rendered directly.TmuxSessionInfo, auto‑reconnects on launch, skips scrollback; gateway workspace is hidden; detach dialog and tmux menu (Ctrl+Cmd+D).Dependencies
ghosttysubmodule to a fork with tmux embedder C API:GHOSTTY_ACTION_TMUX_CONTROL, events (enter/exit/windows_changed/pane_output), and manual I/O support inghostty.h.Written for commit 5e711d8. Summary will update on new commits.
Summary by CodeRabbit