Skip to content

Native tmux control mode (tmux -CC) support - #2474

Closed
lawrencecchen wants to merge 8 commits into
mainfrom
issue-560-tmux-control-mode
Closed

lawrencecchen wants to merge 8 commits into
mainfrom
issue-560-tmux-control-mode

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Wire Ghostty's internal tmux viewer (viewer.zig) through the apprt layer to Swift, enabling cmux to detect and react to tmux control mode (tmux -CC)
  • Add C API surface for querying tmux state: ghostty_surface_tmux_active, window_count, window_info, pane_count, pane_ids, set_active_pane
  • Auto-swap the renderer to the first tmux pane's terminal on windows_changed, so the terminal renders tmux pane output instead of going blank
  • Propagate tmuxActive state from TerminalSurface through TerminalPanel to Workspace

When a user runs tmux -CC in a cmux terminal:

  1. Ghostty's Zig viewer detects DCS 1000p and begins parsing the tmux control mode protocol
  2. %output data is fed directly to per-pane Terminal instances inside the viewer (never crosses the Zig/Swift boundary)
  3. The new tmux_state apprt action notifies Swift of entered/exited/windows_changed events
  4. On windows_changed, cmux queries pane IDs via the new C API and swaps the renderer to the first pane's terminal

Ghostty fork branch: issue-560-tmux-control-mode (diff)

Testing

This is a v1 foundation. To test: run tmux -CC in a cmux terminal. The terminal should show the tmux pane output instead of going blank. Debug logs (tmux.entered, tmux.exited, tmux.windowsChanged) are emitted in DEBUG builds.

No automated test added: meaningful behavioral testing requires an actual tmux server which is not practical in the current CI setup.

Related


Summary by cubic

Adds native tmux control mode support (tmux -CC) so cmux detects tmux sessions and renders pane output. Prevents renderer crashes by routing tmux %output directly to the surface terminal (no renderer pointer swaps).

  • New Features

    • Handle GHOSTTY_ACTION_TMUX_STATE (entered/exited/windows_changed); propagate tmuxActive from TerminalSurface → TerminalPanel → Workspace (tmuxControlModeActive), and post .ghosttyTmuxStateChanged.
    • Tmux C API: ghostty_surface_tmux_active, ghostty_surface_tmux_window_count, ghostty_surface_tmux_window_info, ghostty_surface_tmux_pane_count, ghostty_surface_tmux_pane_ids, ghostty_surface_tmux_set_active_pane. On windows_changed, we log/query counts; output already flows to the surface terminal so no renderer swap is needed.
    • Update ghostty.h with tmux types/actions and declarations; bump ghostty submodule (includes %output octal decoding and send-keys wrapping fixes) and refresh GhosttyKit checksum.
  • Bug Fixes

    • Remove set_active_pane renderer pointer swap (use-after-free risk); the stream handler now decodes %output and writes directly to the surface terminal.

Written for commit f6241a8. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Real-time tracking of tmux control mode across terminals.
    • Workspace-level indicator when any terminal enters tmux control mode.
    • Terminal panels now reflect tmux-active state for consistent UI behavior.
    • UI synchronizes with tmux pane/window changes, auto-selects an active pane, and posts notifications when tmux state updates.
  • Chores

    • Updated embedded tmux-related library and associated checksum reference.

When a user runs `tmux -CC` in a cmux terminal, Ghostty's Zig-based
viewer.zig detects the DCS 1000p handshake and manages per-pane Terminal
instances internally. This PR wires that viewer through the apprt layer
to Swift so cmux can react to tmux state changes.

Ghostty fork changes (manaflow-ai/ghostty issue-560-tmux-control-mode):
- New apprt action: tmux_state (entered/exited/windows_changed)
- Wire viewer .windows/.exit actions to surface messages
- C API: ghostty_surface_tmux_active, window_count, window_info,
  pane_count, pane_ids, set_active_pane
- set_active_pane swaps the renderer terminal pointer to a pane terminal

cmux Swift changes:
- Handle GHOSTTY_ACTION_TMUX_STATE in action callback
- Auto-set renderer to first pane on windows_changed
- Propagate tmuxActive from TerminalSurface -> TerminalPanel -> Workspace
- Update ghostty.h with tmux types and function declarations
@vercel

vercel Bot commented Apr 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Apr 2, 2026 2:01am

@coderabbitai

coderabbitai Bot commented Apr 1, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds tmux control-mode reporting: new C tmux APIs and action tag; TerminalSurface publishes tmuxActive and posts ghosttyTmuxStateChanged; TerminalPanel subscribes to surface tmux state; Workspace aggregates per-panel tmux activity into tmuxControlModeActive.

Changes

Cohort / File(s) Summary
Terminal surface & Ghostty action handling
Sources/GhosttyTerminalView.swift
Added @Published var tmuxActive, handle GHOSTTY_ACTION_TMUX_STATE (enter/exit/windows_changed), attempt to set active tmux pane and refresh surface, and post Notification.Name.ghosttyTmuxStateChanged.
Panel propagation
Sources/Panels/TerminalPanel.swift
Added @Published private(set) var tmuxActive and a Combine subscription to surface.$tmuxActive with removeDuplicates() to update panel state.
Workspace aggregation
Sources/Workspace.swift
Added @Published private(set) var tmuxControlModeActive, tmuxControlModeSubscription, and updated configureTerminalPanel(_:) to observe panels' $tmuxActive and assign aggregated state on main queue.
C API additions (ghostty.h)
ghostty.h
Added ghostty_action_tmux_state_e, ghostty_tmux_window_s, GHOSTTY_ACTION_TMUX_STATE action tag and tmux_state union member, plus tmux surface APIs (ghostty_surface_tmux_active, window/pane query functions, ghostty_surface_tmux_set_active_pane).
Submodule & checksum
ghostty, scripts/ghosttykit-checksums.txt
Updated ghostty submodule commit reference and appended checksum mapping for the new submodule SHA.

Sequence Diagram

sequenceDiagram
    participant C_API as Ghostty C API
    participant Surface as TerminalSurface
    participant Panel as TerminalPanel
    participant Workspace as Workspace
    participant UI as UI/Observers

    C_API->>Surface: send GHOSTTY_ACTION_TMUX_STATE
    Surface->>Surface: update tmuxActive (Published)
    Surface->>UI: post Notification.ghosttyTmuxStateChanged (main queue)
    Surface->>C_API: (on WINDOWS_CHANGED) query panes, set active pane, refresh surface
    Surface->>Panel: surface.$tmuxActive emits
    Panel->>Workspace: panel.$tmuxActive emits
    Workspace->>Workspace: assign tmuxControlModeActive (Published)
    Workspace->>UI: publish tmuxControlModeActive changes
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I sniffed a tmux whisper in the code,

Told Surface to shout, let Panels know the mode,
Combine hopped, Workspace chimed in tune,
Little rabbit dances under a dev moon. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding native tmux control mode support, which is the primary objective of the changeset.
Description check ✅ Passed The pull request description comprehensively covers all template sections: Summary (what changed and why), Testing (how tested and verified), and a complete Checklist with all items addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-560-tmux-control-mode

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 and usage tips.

@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: 48cdfab761

ℹ️ 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 thread Sources/Workspace.swift
self?.tmuxControlModeActive = active
}
// Store the subscription (replacing any prior one)
tmuxControlModeSubscription = subscription

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 Track tmux subscriptions per terminal panel

configureTerminalPanel replaces tmuxControlModeSubscription every time a terminal panel is created, so the workspace only observes the most recently configured terminal's tmuxActive state. In a workspace with multiple terminals, tmuxControlModeActive can flip to false when the last-subscribed panel exits tmux even if another panel is still in tmux control mode, which contradicts the property’s “any terminal panel” contract and will produce incorrect workspace-level state.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR wires Ghostty's Zig-side tmux control-mode viewer into Swift by adding a GHOSTTY_ACTION_TMUX_STATE handler, a small C API (ghostty_surface_tmux_*), and a tmuxActive flag that propagates TerminalSurface → TerminalPanel → Workspace. On WINDOWS_CHANGED the renderer is automatically swapped to the first available pane so the terminal shows tmux output instead of going blank.

Prior review threads (still open in the diff) identified two blocking concerns worth resolving before merge: (1) Workspace.tmuxControlModeSubscription tracks only the most-recently-configured panel, so a second panel entering tmux can overwrite the flag with an incorrect false; and (2) every WINDOWS_CHANGED event unconditionally resets the active pane to index 0, silently disrupting user focus on any layout change.

Confidence Score: 4/5

  • Functional foundation for tmux control mode, but two open P1 concerns from prior review threads remain unresolved in the diff and should be addressed before merge.
  • The single tmuxControlModeSubscription in Workspace incorrectly tracks only the last-configured panel (prior thread, still present at line 6058), and the always-pane-0 snap on WINDOWS_CHANGED silently disrupts user focus (prior thread, still present at line 2884). Both are present defects on the changed path. The one new finding in this review is a P2 style inconsistency (missing receive(on:) in TerminalPanel.init). Score of 4 reflects the two unresolved prior P1s.
  • Sources/Workspace.swift (single-subscription multi-panel state bug) and Sources/GhosttyTerminalView.swift (always-pane-0 renderer snap)

Important Files Changed

Filename Overview
Sources/GhosttyTerminalView.swift Adds GHOSTTY_ACTION_TMUX_STATE handler: sets tmuxActive on ENTERED/EXITED, auto-swaps renderer to pane 0 on WINDOWS_CHANGED, and posts ghosttyTmuxStateChanged notification; also adds @published var tmuxActive (without private(set)) to TerminalSurface and the new notification name.
Sources/Panels/TerminalPanel.swift Adds @published private(set) var tmuxActive and mirrors it from the underlying TerminalSurface via a Combine subscription in init; subscription is missing .receive(on: DispatchQueue.main) unlike its counterpart in Workspace.swift.
Sources/Workspace.swift Adds tmuxControlModeActive and tmuxControlModeSubscription; configureTerminalPanel subscribes to terminalPanel.$tmuxActive but stores only one subscription at a time, causing the workspace-level flag to track only the most-recently-configured panel rather than all panels (pre-existing concern from prior review threads).
ghostty.h Adds ghostty_action_tmux_state_e enum, ghostty_tmux_window_s struct, GHOSTTY_ACTION_TMUX_STATE and GHOSTTY_ACTION_COPY_TITLE_TO_CLIPBOARD to the action tag enum, tmux_state to the action union, and six new C API declarations for tmux control mode.

Sequence Diagram

sequenceDiagram
    participant Zig as Ghostty/Zig (viewer.zig)
    participant C as C API (ghostty.h)
    participant GApp as GhosttyApp (Swift)
    participant TS as TerminalSurface
    participant TP as TerminalPanel
    participant WS as Workspace

    Zig->>C: apprt action TMUX_STATE (entered/exited/windows_changed)
    C->>GApp: performAction(GHOSTTY_ACTION_TMUX_STATE)
    GApp->>GApp: DispatchQueue.main.async
    alt ENTERED
        GApp->>TS: tmuxActive = true
    else EXITED
        GApp->>TS: tmuxActive = false
    else WINDOWS_CHANGED
        GApp->>C: ghostty_surface_tmux_pane_count()
        C-->>GApp: paneCount
        GApp->>C: ghostty_surface_tmux_pane_ids(&paneId, 1)
        C-->>GApp: idCount, paneId[0]
        GApp->>C: ghostty_surface_tmux_set_active_pane(paneId[0])
        GApp->>C: ghostty_surface_refresh()
    end
    GApp->>GApp: NotificationCenter.post(.ghosttyTmuxStateChanged)
    TS-->>TP: $tmuxActive (Combine)
    TP-->>WS: $tmuxActive → tmuxControlModeActive (Combine, single subscription)
Loading

Reviews (2): Last reviewed commit: "Trigger CI for updated ghostty submodule" | Re-trigger Greptile

Comment thread Sources/Workspace.swift
Comment on lines +6050 to +6058
// Subscribe to tmux control mode state changes
let subscription = terminalPanel.$tmuxActive
.removeDuplicates()
.receive(on: DispatchQueue.main)
.sink { [weak self] active in
self?.tmuxControlModeActive = active
}
// Store the subscription (replacing any prior one)
tmuxControlModeSubscription = subscription

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 Single subscription overwrites multi-panel tmux state

tmuxControlModeSubscription is a single AnyCancellable?. Every call to configureTerminalPanel cancels the previous subscription and starts a fresh one on the newly configured panel. Because @Published emits its current value immediately upon subscription, subscribing to any new panel that starts with tmuxActive = false will immediately set tmuxControlModeActive = false, clobbering the true that was correctly set by an older panel still inside tmux control mode.

Concrete failure scenario with panels A and B in the same workspace:

  1. Panel A enters tmux → tmuxControlModeActive = true
  2. A new panel B is configured → subscription switches to B
  3. B emits its initial false → tmuxControlModeActive = false ← wrong; A is still in tmux mode
  4. A exits tmux → no subscriber left, the flag is never corrected

The property's doc-comment says "Whether any terminal panel in this workspace is in tmux control mode", but the implementation only tracks the last configured panel.

Fix: track subscriptions per-panel in the existing panelSubscriptions: [UUID: AnyCancellable] dictionary (used for browser/markdown panels), combining their individual tmuxActive states with a derived publisher, or maintain a Set<UUID> of active-tmux panels and compute the boolean from its emptiness.

Comment on lines +2878 to +2892
case GHOSTTY_TMUX_STATE_WINDOWS_CHANGED:
// Auto-set the renderer to the first pane so we can see tmux output
if let surface = terminalSurface.surface {
let paneCount = ghostty_surface_tmux_pane_count(surface)
if paneCount > 0 {
var paneId: UInt = 0
let idCount = ghostty_surface_tmux_pane_ids(surface, &paneId, 1)
if idCount > 0 {
let success = ghostty_surface_tmux_set_active_pane(surface, paneId)
#if DEBUG
dlog("tmux.windowsChanged panes=\(paneCount) activePaneId=\(paneId) setActive=\(success)")
#endif
}
}
}

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.

P2 windows_changed always snaps the renderer to pane index 0

Every GHOSTTY_TMUX_STATE_WINDOWS_CHANGED event calls ghostty_surface_tmux_pane_ids(surface, &paneId, 1) to grab one ID and immediately makes it the active renderer. "First pane" here means whatever ordering the C API returns, with no memory of which pane was previously active.

In practice this fires on any tmux layout change (new window, pane split, pane close). Each such event will silently jump the user's view to pane 0, even if they were looking at a different pane a moment before. Consider tracking the last-set active pane and only auto-switching when the previously active pane is no longer present (i.e. it was deleted).

Comment on lines 3172 to +3173
@Published private(set) var keyboardCopyModeActive: Bool = false
/// Whether this surface is currently in tmux control mode (DCS 1000p).

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.

P2 tmuxActive is missing private(set) — inconsistent with similar state properties

Neighbouring state flags use @Published private(set) var:

@Published private(set) var keyboardCopyModeActive: Bool = false

tmuxActive is declared without private(set):

@Published var tmuxActive: Bool = false

Because the setter is called from inside GhosttyApp's action handler (a different type), private(set) would not compile as-is. A common pattern to keep external mutability explicit is to make the property private(set) and expose a dedicated fileprivate or internal setter method on TerminalSurface, or to restructure GhosttyApp so the mutation flows through TerminalSurface itself. As written, any code with a reference to the surface can write tmuxActive freely, bypassing the intended notification flow.

@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: 3

🧹 Nitpick comments (2)
ghostty.h (1)

631-636: Document the bulk-fill contract for the tmux enumeration APIs.

ghostty_surface_tmux_window_info and ghostty_surface_tmux_pane_ids take caller-owned buffers, but the header doesn't say whether the return value is "items written" vs "items available", whether NULL, 0 is a valid sizing call, or whether ghostty_tmux_window_s.width / height are cells or pixels. Since Swift is consuming this surface directly, putting that contract only in Zig leaves the bridge guessing.

Also applies to: 1174-1184

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghostty.h` around lines 631 - 636, The header lacks a clear bulk-fill
contract for the tmux enumeration APIs: update the comments for
ghostty_surface_tmux_window_info and ghostty_surface_tmux_pane_ids to state
whether the return value is "items written" versus "total items available",
whether passing (NULL, 0) is supported as a sizing query, and what happens when
the provided buffer is too small (partial fill + total count or error). Also
document ghostty_tmux_window_s.width and .height units (cells vs pixels) and any
alignment/zero-value semantics; ensure the same text is added for the related
pane-ID API mentioned around the second range so callers (including Swift) know
exactly how to size buffers and interpret results.
Sources/Panels/TerminalPanel.swift (1)

91-97: Marshal tmux state delivery to main before publishing.

Line 92–97 should enforce main-thread delivery before mutating @Published state. Although upstream mutations in GhosttyTerminalView are already dispatched to main, TerminalPanel is @MainActor-isolated and should not rely on caller-side dispatch. Add .receive(on: DispatchQueue.main) to ensure the sink closure runs on the main thread.

Proposed change
         surface.$tmuxActive
             .removeDuplicates()
+            .receive(on: DispatchQueue.main)
             .sink { [weak self] active in
                 self?.tmuxActive = active
             }
             .store(in: &cancellables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/TerminalPanel.swift` around lines 91 - 97, The subscription to
surface.$tmuxActive must ensure delivery on the main thread before mutating
`@Published` state: insert .receive(on: DispatchQueue.main) into the Combine chain
for the surface.$tmuxActive publisher (before .sink) so the sink closure that
sets self?.tmuxActive executes on the main thread; update the pipeline that
currently reads surface.$tmuxActive.removeDuplicates().sink { ... } to include
.receive(on: DispatchQueue.main) prior to .sink so TerminalPanel (MainActor)
always mutates tmuxActive on the main thread.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghostty`:
- Line 1: CI failed because the Ghostty submodule was updated to commit
4c170d120d5bc87d039fab75c1f2cfeed1951108 but the companion checksum file
scripts/ghosttykit-checksums.txt wasn't updated; open
scripts/ghosttykit-checksums.txt and add a checksum entry/row corresponding
exactly to commit 4c170d120d5bc87d039fab75c1f2cfeed1951108 (matching the format
of existing rows), commit that change alongside the ghostty submodule bump so
the PR contains both the submodule update and its checksum update.

In `@Sources/GhosttyTerminalView.swift`:
- Around line 2863-2901: Capture the C surface pointer before dispatching to the
main queue and bail if it changed: read let sourceSurface =
terminalSurface.surface immediately before DispatchQueue.main.async, then inside
the async closure, before mutating terminalSurface or calling
ghostty_surface_tmux_* functions, guard that
terminalSurface.liveSurfaceForGhosttyAccess(reason: /* appropriate reason */) ==
sourceSurface (or otherwise compare terminalSurface.surface == sourceSurface)
and return early if it differs; update the GHOSTTY_ACTION_TMUX_STATE handling
(where terminalSurface and tmux pane selection occur) to use this guard so you
never call ghostty_surface_tmux_* on a stale/freed pointer.

In `@Sources/Workspace.swift`:
- Around line 5542-5544: The workspace currently overwrites
tmuxControlModeSubscription each time configureTerminalPanel(_:) is called, so
tmuxControlModeActive only reflects the last configured terminal; instead change
tmuxControlModeSubscription from a single AnyCancellable? to a dictionary keyed
by terminal identifier (e.g. [TerminalPanelID: AnyCancellable]) and, when
configuring a terminal in configureTerminalPanel(_:), store that terminal's
cancellable into the dictionary; on terminal teardown/remove (the same places
that update panelSubscriptions) remove and cancel that terminal's cancellable
and then call recomputeTmuxControlModeActive(); implement
recomputeTmuxControlModeActive() to set the `@Published` tmuxControlModeActive by
OR-ing the tmux control mode state across all live terminals (reading each
terminal's current state or the latest published value), ensuring
panelSubscriptions logic remains unchanged except for also cleaning up the new
per-terminal cancellable.

---

Nitpick comments:
In `@ghostty.h`:
- Around line 631-636: The header lacks a clear bulk-fill contract for the tmux
enumeration APIs: update the comments for ghostty_surface_tmux_window_info and
ghostty_surface_tmux_pane_ids to state whether the return value is "items
written" versus "total items available", whether passing (NULL, 0) is supported
as a sizing query, and what happens when the provided buffer is too small
(partial fill + total count or error). Also document ghostty_tmux_window_s.width
and .height units (cells vs pixels) and any alignment/zero-value semantics;
ensure the same text is added for the related pane-ID API mentioned around the
second range so callers (including Swift) know exactly how to size buffers and
interpret results.

In `@Sources/Panels/TerminalPanel.swift`:
- Around line 91-97: The subscription to surface.$tmuxActive must ensure
delivery on the main thread before mutating `@Published` state: insert
.receive(on: DispatchQueue.main) into the Combine chain for the
surface.$tmuxActive publisher (before .sink) so the sink closure that sets
self?.tmuxActive executes on the main thread; update the pipeline that currently
reads surface.$tmuxActive.removeDuplicates().sink { ... } to include
.receive(on: DispatchQueue.main) prior to .sink so TerminalPanel (MainActor)
always mutates tmuxActive on the main thread.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7d975e26-fb94-49ee-a20a-1c26949fcdbe

📥 Commits

Reviewing files that changed from the base of the PR and between e05d425 and 48cdfab.

📒 Files selected for processing (5)
  • Sources/GhosttyTerminalView.swift
  • Sources/Panels/TerminalPanel.swift
  • Sources/Workspace.swift
  • ghostty
  • ghostty.h

Comment thread ghostty Outdated
Comment on lines +2863 to +2901
case GHOSTTY_ACTION_TMUX_STATE:
guard let terminalSurface = surfaceView.terminalSurface else { return true }
let tmuxState = action.action.tmux_state
DispatchQueue.main.async {
switch tmuxState {
case GHOSTTY_TMUX_STATE_ENTERED:
terminalSurface.tmuxActive = true
#if DEBUG
dlog("tmux.entered tab=\(surfaceView.tabId?.uuidString.prefix(5) ?? "nil") surface=\(terminalSurface.id.uuidString.prefix(5))")
#endif
case GHOSTTY_TMUX_STATE_EXITED:
terminalSurface.tmuxActive = false
#if DEBUG
dlog("tmux.exited tab=\(surfaceView.tabId?.uuidString.prefix(5) ?? "nil") surface=\(terminalSurface.id.uuidString.prefix(5))")
#endif
case GHOSTTY_TMUX_STATE_WINDOWS_CHANGED:
// Auto-set the renderer to the first pane so we can see tmux output
if let surface = terminalSurface.surface {
let paneCount = ghostty_surface_tmux_pane_count(surface)
if paneCount > 0 {
var paneId: UInt = 0
let idCount = ghostty_surface_tmux_pane_ids(surface, &paneId, 1)
if idCount > 0 {
let success = ghostty_surface_tmux_set_active_pane(surface, paneId)
#if DEBUG
dlog("tmux.windowsChanged panes=\(paneCount) activePaneId=\(paneId) setActive=\(success)")
#endif
}
}
}
default:
break
}
NotificationCenter.default.post(
name: .ghosttyTmuxStateChanged,
object: terminalSurface,
userInfo: ["state": tmuxState]
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find the definition of liveSurfaceForGhosttyAccess and understand how it validates surfaces
rg -A 10 "func liveSurfaceForGhosttyAccess" Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 660


🏁 Script executed:

# Check how TerminalSurface.surface can be replaced/rebound
rg -B 2 -A 3 "terminalSurface.surface\s*=" Sources/GhosttyTerminalView.swift | head -50

Repository: manaflow-ai/cmux

Length of output: 335


🏁 Script executed:

# Look for similar async dispatch patterns that guard against stale surfaces
rg -B 3 -A 8 "DispatchQueue.main.async.*target.target.surface" Sources/GhosttyTerminalView.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check if other tmux-related operations guard the surface
rg -B 5 -A 5 "ghostty_surface_tmux_" Sources/GhosttyTerminalView.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 1113


🏁 Script executed:

# Verify how surface lifecycle/rebinding works
rg -B 2 -A 5 "\.surface\s*=" Sources/GhosttyTerminalView.swift | grep -A 5 "terminalSurface\|TerminalSurface" | head -60

Repository: manaflow-ai/cmux

Length of output: 449


🏁 Script executed:

# Find the handleAction method signature to understand the target parameter
rg -B 5 "func handleAction.*target.*action" Sources/GhosttyTerminalView.swift | head -40

Repository: manaflow-ai/cmux

Length of output: 421


🏁 Script executed:

# Check what target.target.surface represents at the call site
rg -B 10 "case GHOSTTY_ACTION_TMUX_STATE:" Sources/GhosttyTerminalView.swift | head -30

Repository: manaflow-ai/cmux

Length of output: 612


🏁 Script executed:

# Search for other action handlers to see if they use similar guards
rg -B 2 -A 8 "case GHOSTTY_ACTION_" Sources/GhosttyTerminalView.swift | grep -A 8 "DispatchQueue.main.async" | head -60

Repository: manaflow-ai/cmux

Length of output: 2485


🏁 Script executed:

# Verify if surfaces can truly be unregistered mid-flight during async operations
rg -B 3 -A 5 "unregisterRuntimeSurface\|registry.unregister" Sources/GhosttyTerminalView.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check the exact line where target is obtained in handleAction
sed -n '2800,2920p' Sources/GhosttyTerminalView.swift | grep -B 30 "GHOSTTY_ACTION_TMUX_STATE"

Repository: manaflow-ai/cmux

Length of output: 1619


🏁 Script executed:

# Find the callback registration and invocation to understand what target contains
rg -B 5 -A 10 "surfaceCallbackContext" Sources/GhosttyTerminalView.swift | grep -A 10 "Unmanaged\|handleAction" | head -60

Repository: manaflow-ai/cmux

Length of output: 1444


🏁 Script executed:

# Look for where ghostty_surface callbacks are registered and how target is populated
rg "ghostty_surface_on_action" Sources/GhosttyTerminalView.swift -B 3 -A 3

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check the actual closure at handleAction call site to see when it's invoked
rg -B 15 "handleAction.*target.*action" Sources/GhosttyTerminalView.swift | head -50

Repository: manaflow-ai/cmux

Length of output: 1635


🏁 Script executed:

# Search for similar patterns where ghostty_surface_t is captured before async dispatch
rg -B 5 -A 10 "let.*surface.*=.*target.target.surface" Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check other critical surface operations that happen in async closures
rg -B 8 -A 8 "ghostty_surface_.*\(" Sources/GhosttyTerminalView.swift | grep -B 8 -A 8 "DispatchQueue.main.async" | head -80

Repository: manaflow-ai/cmux

Length of output: 2877


🏁 Script executed:

# Check the Ghostty C API to understand what target.target.surface is
rg "ghostty_target" Sources/GhosttyTerminalView.swift -B 2 -A 2 | head -40

Repository: manaflow-ai/cmux

Length of output: 528


🏁 Script executed:

# Find documentation or comments about what the target parameter contains
grep -r "target.target.surface" Sources/ --include="*.swift"

Repository: manaflow-ai/cmux

Length of output: 199


🏁 Script executed:

# Look at how other handlers use the target parameter
sed -n '2600,2950p' Sources/GhosttyTerminalView.swift | grep -B 5 -A 10 "target.target"

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check if there are any other uses of target in handleAction
rg "private func handleAction" -A 200 Sources/GhosttyTerminalView.swift | grep "target\." | head -20

Repository: manaflow-ai/cmux

Length of output: 215


🏁 Script executed:

# Verify the exact pattern used in clipboard callback - does it capture the source surface?
sed -n '2380,2450p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3253


🏁 Script executed:

# Get the full clipboard callback pattern to see how it captures and validates
sed -n '2380,2430p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 2510


🏁 Script executed:

# Check the deinit to understand surface lifecycle risks
sed -n '1050,1130p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3135


🏁 Script executed:

# See if surfaces can actually be replaced during operation
rg "surface = nil" Sources/GhosttyTerminalView.swift -B 3 -A 3

Repository: manaflow-ai/cmux

Length of output: 1066


🏁 Script executed:

# Verify if liveSurfaceForGhosttyAccess is the proper validation function
sed -n '970,1010p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 1998


🏁 Script executed:

# Get the full read_clipboard_cb to see the capture + validate pattern
sed -n '2345,2430p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 4113


🏁 Script executed:

# Verify the surfaces ARE actually changed/replaced during normal operation
rg "self.surface = " Sources/GhosttyTerminalView.swift | head -20

Repository: manaflow-ai/cmux

Length of output: 238


🏁 Script executed:

# Check if terminalSurface reference is stable even if its .surface property changes
rg "class TerminalSurface" -A 30 Sources/GhosttyTerminalView.swift | head -50

Repository: manaflow-ai/cmux

Length of output: 1624


🏁 Script executed:

# Find the complete clipboard callback including the validation pattern
sed -n '2545,2620p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3622


🏁 Script executed:

# Verify the exact pattern for capturing and validating in async closures
rg -B 8 -A 12 "runtimeSurface == requestSurface" Sources/GhosttyTerminalView.swift | head -50

Repository: manaflow-ai/cmux

Length of output: 1073


🏁 Script executed:

# Check what happens if we try to access target in the async closure - is it captured?
sed -n '2863,2910p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 2402


🏁 Script executed:

# Look for other places where target.target.surface is captured before async dispatch
rg -B 5 -A 15 "let.*=.*target.target.surface" Sources/GhosttyTerminalView.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 944


🏁 Script executed:

# Verify if the `target` parameter can be captured in the closure or goes out of scope
sed -n '2820,2870p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 2575


🏁 Script executed:

# Check lifecycle: can a TerminalSurface's ghostty_surface_t pointer change between callback and async closure?
rg -B 3 -A 5 "hasLiveSurface\|cmuxSurfacePointerAppearsLive" Sources/GhosttyTerminalView.swift | head -60

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check if target is available at the time of DispatchQueue.main.async or if it goes out of scope
sed -n '2840,2880p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 2334


🏁 Script executed:

# Look at the signature and see if target is passed by value (can be captured) or reference
rg "func handleAction" -A 5 Sources/GhosttyTerminalView.swift | head -15

Repository: manaflow-ai/cmux

Length of output: 462


🏁 Script executed:

# Verify what ghostty_target_s is and if it can be safely captured
rg -B 2 -A 2 "ghostty_target_s" Sources/GhosttyTerminalView.swift | head -30

Repository: manaflow-ai/cmux

Length of output: 528


🏁 Script executed:

# Check if surfaces are actually torn down/reparented in normal operation
rg "forceRefreshSurface\|teardown\|reparent" Sources/GhosttyTerminalView.swift | head -20

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Verify the learning about surface re-reading after async operations
sed -n '1,100p' Sources/GhosttyTerminalView.swift | grep -A 10 "Learning:"

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check how liveSurfaceForGhosttyAccess handles stale surfaces
sed -n '970,1000p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 1629


🏁 Script executed:

# Look at the full liveSurfaceForGhosttyAccess implementation to understand the validation
sed -n '900,950p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 1816


🏁 Script executed:

# Check if target can actually be captured in a closure (it's a struct)
rg "ghostty_target" Sources/GhosttyTerminalView.swift -B 2 -A 2 | grep -A 5 "struct\|class"

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Look for examples where target parameters ARE captured before async
rg -B 8 -A 8 "let.*=.*target\." Sources/GhosttyTerminalView.swift | head -40

Repository: manaflow-ai/cmux

Length of output: 1558


🏁 Script executed:

# Verify the learning about surface reparenting to understand if this is a real scenario
rg "forceRefreshSurface\|teardown\|initial-present recovery" Sources/GhosttyTerminalView.swift -B 2 -A 5 | head -40

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Check if target can be captured and if target.target.surface is a valid pointer at closure time
sed -n '2415,2445p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 1355


🏁 Script executed:

# Check if there are actual examples of surface rebinding during operation
rg "surface = ghostty_surface_new\|surface = nil" Sources/GhosttyTerminalView.swift -B 3 -A 1

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Verify if requestBackgroundSurfaceStartIfNeeded or similar can change surfaces
sed -n '1250,1320p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3139


🏁 Script executed:

# Verify that target (a struct parameter) can actually be captured in the closure
python3 << 'EOF'
# Swift struct parameters can be captured in closures - this is standard Swift
# The ghostty_target_s is a struct, so target.target.surface is a value that can be captured
print("Swift closure capture behavior:")
print("✓ Function parameters (value types like structs) CAN be captured")
print("✓ target.target.surface is a raw pointer value that can be captured")
print("✓ The pointer value remains valid across the async dispatch")
EOF

Repository: manaflow-ai/cmux

Length of output: 284


🏁 Script executed:

# Verify the lifecycle: do surfaces actually get rebound to new pointers?
rg "requestBackgroundSurfaceStartIfNeeded\|surface.*ghostty_surface_new" Sources/GhosttyTerminalView.swift -B 2 -A 5 | head -40

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Final check: look at a complete timeline - does terminalSurface.surface actually get replaced?
sed -n '960,1020p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3063


🏁 Script executed:

# Verify the complete flow: callback receives target at callback time, then async dispatch happens
sed -n '2655,2710p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3047


Guard tmux callbacks against stale/replaced runtime surfaces.

Line 2866 dispatches to the main queue, but line 2880 then re-reads terminalSurface.surface instead of validating the source surface that raised this action. Surfaces can be torn down or reparented during layout recovery and workspace churn; if this TerminalSurface's C surface pointer changes between callback delivery and the async closure, the tmux event will mutate stale state and invoke ghostty_surface_tmux_* on a freed or incorrect pointer. Capture target.target.surface before dispatch, then require liveSurfaceForGhosttyAccess(reason:) == sourceSurface before any state mutation or pane selection.

Suggested guard
         case GHOSTTY_ACTION_TMUX_STATE:
             guard let terminalSurface = surfaceView.terminalSurface else { return true }
             let tmuxState = action.action.tmux_state
+            let sourceSurface = target.target.surface
             DispatchQueue.main.async {
+                let liveSurface = MainActor.assumeIsolated {
+                    terminalSurface.liveSurfaceForGhosttyAccess(reason: "tmux.state")
+                }
+                guard let liveSurface, liveSurface == sourceSurface else { return }
                 switch tmuxState {
                 case GHOSTTY_TMUX_STATE_ENTERED:
                     terminalSurface.tmuxActive = true
                     `#if` DEBUG
                     dlog("tmux.entered tab=\(surfaceView.tabId?.uuidString.prefix(5) ?? "nil") surface=\(terminalSurface.id.uuidString.prefix(5))")
                     `#endif`
                 case GHOSTTY_TMUX_STATE_EXITED:
                     terminalSurface.tmuxActive = false
                     `#if` DEBUG
                     dlog("tmux.exited tab=\(surfaceView.tabId?.uuidString.prefix(5) ?? "nil") surface=\(terminalSurface.id.uuidString.prefix(5))")
                     `#endif`
                 case GHOSTTY_TMUX_STATE_WINDOWS_CHANGED:
                     // Auto-set the renderer to the first pane so we can see tmux output
-                    if let surface = terminalSurface.surface {
-                        let paneCount = ghostty_surface_tmux_pane_count(surface)
-                        if paneCount > 0 {
-                            var paneId: UInt = 0
-                            let idCount = ghostty_surface_tmux_pane_ids(surface, &paneId, 1)
-                            if idCount > 0 {
-                                let success = ghostty_surface_tmux_set_active_pane(surface, paneId)
-                                `#if` DEBUG
-                                dlog("tmux.windowsChanged panes=\(paneCount) activePaneId=\(paneId) setActive=\(success)")
-                                `#endif`
-                            }
-                        }
+                    let paneCount = ghostty_surface_tmux_pane_count(liveSurface)
+                    if paneCount > 0 {
+                        var paneId: UInt = 0
+                        let idCount = ghostty_surface_tmux_pane_ids(liveSurface, &paneId, 1)
+                        if idCount > 0 {
+                            let success = ghostty_surface_tmux_set_active_pane(liveSurface, paneId)
+                            `#if` DEBUG
+                            dlog("tmux.windowsChanged panes=\(paneCount) activePaneId=\(paneId) setActive=\(success)")
+                            `#endif`
+                        }
                     }
                 default:
                     break
                 }
                 NotificationCenter.default.post(
                     name: .ghosttyTmuxStateChanged,
                     object: terminalSurface,
                     userInfo: ["state": tmuxState]
                 )
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 2863 - 2901, Capture the C
surface pointer before dispatching to the main queue and bail if it changed:
read let sourceSurface = terminalSurface.surface immediately before
DispatchQueue.main.async, then inside the async closure, before mutating
terminalSurface or calling ghostty_surface_tmux_* functions, guard that
terminalSurface.liveSurfaceForGhosttyAccess(reason: /* appropriate reason */) ==
sourceSurface (or otherwise compare terminalSurface.surface == sourceSurface)
and return early if it differs; update the GHOSTTY_ACTION_TMUX_STATE handling
(where terminalSurface and tmux pane selection occur) to use this guard so you
never call ghostty_surface_tmux_* on a stale/freed pointer.

Comment thread Sources/Workspace.swift
Comment on lines +5542 to +5544
/// Whether any terminal panel in this workspace is in tmux control mode.
@Published private(set) var tmuxControlModeActive: Bool = false
private var tmuxControlModeSubscription: AnyCancellable?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's find and inspect the relevant lines in Workspace.swift
wc -l Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 89


🏁 Script executed:

# Read the section around lines 5542-5544 to see the property definition
sed -n '5535,5550p' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 1191


🏁 Script executed:

# Read the section around lines 6050-6058 to see the subscription setup
sed -n '6040,6070p' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 1351


🏁 Script executed:

# Search for the entire configureTerminalPanel method to understand the subscription logic
rg -A 30 'func configureTerminalPanel' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 1406


🏁 Script executed:

# Look for panel removal/teardown paths to see if subscriptions are cleaned up
rg -B 3 -A 5 'panelSubscriptions.*removeValue|panel.*deinit|removeTerminalPanel|panelWasRemoved' Sources/Workspace.swift | head -100

Repository: manaflow-ai/cmux

Length of output: 2044


🏁 Script executed:

# Check if Workspace supports multiple terminals by looking at panel management
rg -A 3 'var panels|var panelSubscriptions' Sources/Workspace.swift | head -30

Repository: manaflow-ai/cmux

Length of output: 410


🏁 Script executed:

# Search for calls to configureTerminalPanel to verify it's invoked per terminal
rg 'configureTerminalPanel\(' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 404


🏁 Script executed:

# Check methods that add new terminals to the workspace
rg -B 5 -A 10 'func newTerminalSplit|func newTerminalSurface|func createReplacement' Sources/Workspace.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 2540


🏁 Script executed:

# Verify the exact flow: when a terminal is added, is configureTerminalPanel called?
rg -B 3 -A 3 'configureTerminalPanel.*terminalPanel' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 842


🏁 Script executed:

# Check if there are any other tmux-related subscriptions or if this is the only place
rg 'tmuxControlMode|tmuxActive' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 345


Aggregate tmux state across all terminal panels, not just the last one.

tmuxControlModeSubscription gets overwritten on every configureTerminalPanel(_:) call (which occurs once per added terminal), so this workspace ends up tracking only the most recently configured terminal. In a multi-terminal workspace, that breaks the documented "any terminal panel in this workspace" behavior and leaves earlier panel subscriptions to be garbage-collected without contributing further updates. Additionally, panel removal paths do not clean up this subscription, unlike panelSubscriptions. Store one subscription per terminal (as a dictionary), and recompute the workspace flag from the aggregate state of all live terminals instead of assigning a single panel's value.

🐛 Suggested direction
-    private var tmuxControlModeSubscription: AnyCancellable?
+    private var tmuxControlModeSubscriptions: [UUID: AnyCancellable] = [:]
+
+    private func recomputeTmuxControlModeActive() {
+        tmuxControlModeActive = panels.values.contains { ($0 as? TerminalPanel)?.tmuxActive == true }
+    }
-        let subscription = terminalPanel.$tmuxActive
+        tmuxControlModeSubscriptions[terminalPanel.id] = terminalPanel.$tmuxActive
             .removeDuplicates()
             .receive(on: DispatchQueue.main)
-            .sink { [weak self] active in
-                self?.tmuxControlModeActive = active
+            .sink { [weak self] _ in
+                self?.recomputeTmuxControlModeActive()
             }
-        // Store the subscription (replacing any prior one)
-        tmuxControlModeSubscription = subscription
+        recomputeTmuxControlModeActive()

Also remove the terminal's cancellable and call recomputeTmuxControlModeActive() from the existing panel teardown paths.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Workspace.swift` around lines 5542 - 5544, The workspace currently
overwrites tmuxControlModeSubscription each time configureTerminalPanel(_:) is
called, so tmuxControlModeActive only reflects the last configured terminal;
instead change tmuxControlModeSubscription from a single AnyCancellable? to a
dictionary keyed by terminal identifier (e.g. [TerminalPanelID: AnyCancellable])
and, when configuring a terminal in configureTerminalPanel(_:), store that
terminal's cancellable into the dictionary; on terminal teardown/remove (the
same places that update panelSubscriptions) remove and cancel that terminal's
cancellable and then call recomputeTmuxControlModeActive(); implement
recomputeTmuxControlModeActive() to set the `@Published` tmuxControlModeActive by
OR-ing the tmux control mode state across all live terminals (reading each
terminal's current state or the latest published value), ensuring
panelSubscriptions logic remains unchanged except for also cleaning up the new
per-terminal cancellable.

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 5 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/Workspace.swift">

<violation number="1" location="Sources/Workspace.swift:6055">
P2: tmuxControlModeActive only reflects the last configured TerminalPanel. It can flip to false while another panel is still in control mode and won’t update when earlier panels change. Track tmuxActive per panel and derive `any` across them instead of overwriting a single subscription.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Sources/Workspace.swift
.removeDuplicates()
.receive(on: DispatchQueue.main)
.sink { [weak self] active in
self?.tmuxControlModeActive = active

@cubic-dev-ai cubic-dev-ai Bot Apr 1, 2026 •

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: tmuxControlModeActive only reflects the last configured TerminalPanel. It can flip to false while another panel is still in control mode and won’t update when earlier panels change. Track tmuxActive per panel and derive any across them instead of overwriting a single subscription.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Workspace.swift, line 6055:

<comment>tmuxControlModeActive only reflects the last configured TerminalPanel. It can flip to false while another panel is still in control mode and won’t update when earlier panels change. Track tmuxActive per panel and derive `any` across them instead of overwriting a single subscription.</comment>

<file context>
@@ -6043,6 +6046,16 @@ final class Workspace: Identifiable, ObservableObject {
+            .removeDuplicates()
+            .receive(on: DispatchQueue.main)
+            .sink { [weak self] active in
+                self?.tmuxControlModeActive = active
+            }
+        // Store the subscription (replacing any prior one)
</file context>
Fix with Cubic

@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: 53c1b55d54

ℹ️ 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".

bc9be90a21997a4e5f06bf15ae2ec0f937c2dc42 6b83b66768e8bba871a3753ae8ffbaabd03370b306c429cd86c9cdcc8db82589
41e796064e89eacabdf3a6729475e250a5518e7a 135302bbdf3e83b200f0165ff2a32cdf13017219e6c8ffca17b672edfbfae395
f9030b5c5232db69ba8625bb53d51ce735b80d51 6c439d731d97bd35a3289f54478af3ac01e30ba74ec2672490e0c98f95262b55
4c170d120d5bc87d039fab75c1f2cfeed1951108 56e1644d1e9020edd2c41e326df03d00387538371fc5498a4febcf9d17b4d7de

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 Pin checksum for the updated ghostty submodule SHA

scripts/download-prebuilt-ghosttykit.sh looks up the checksum by the current ghostty submodule HEAD, but this commit moves the gitlink to f7bf18e56f7ae2fa3b02a108b34d6f17fd12226b while adding a checksum entry for 4c170d120d5bc87d039fab75c1f2cfeed1951108. In this state, ./scripts/setup.sh (or direct GhosttyKit download) will fail with “Missing pinned GhosttyKit checksum for ghostty …”, blocking dependency setup and builds for this revision.

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.

♻️ Duplicate comments (1)
Sources/GhosttyTerminalView.swift (1)

2863-2905: ⚠️ Potential issue | 🔴 Critical

Guard tmux state callbacks against stale runtime surfaces.

After the async hop on Line 2866, Line 2880 re-reads terminalSurface.surface instead of validating the ghostty_surface_t that raised the action. If that runtime surface is torn down or rebound before the closure runs, this can flip tmuxActive, post .ghosttyTmuxStateChanged, and call ghostty_surface_tmux_* / ghostty_surface_refresh on the wrong or freed surface pointer.

🐛 Suggested guard
         case GHOSTTY_ACTION_TMUX_STATE:
             guard let terminalSurface = surfaceView.terminalSurface else { return true }
             let tmuxState = action.action.tmux_state
+            let sourceSurface = target.target.surface
             DispatchQueue.main.async {
+                let liveSurface = MainActor.assumeIsolated {
+                    terminalSurface.liveSurfaceForGhosttyAccess(reason: "tmux.state")
+                }
+                guard let liveSurface, liveSurface == sourceSurface else { return }
                 switch tmuxState {
                 case GHOSTTY_TMUX_STATE_ENTERED:
                     terminalSurface.tmuxActive = true
                     `#if` DEBUG
                     dlog("tmux.entered tab=\(surfaceView.tabId?.uuidString.prefix(5) ?? "nil") surface=\(terminalSurface.id.uuidString.prefix(5))")
                     `#endif`
                 case GHOSTTY_TMUX_STATE_EXITED:
                     terminalSurface.tmuxActive = false
                     `#if` DEBUG
                     dlog("tmux.exited tab=\(surfaceView.tabId?.uuidString.prefix(5) ?? "nil") surface=\(terminalSurface.id.uuidString.prefix(5))")
                     `#endif`
                 case GHOSTTY_TMUX_STATE_WINDOWS_CHANGED:
                     // Auto-set the renderer to the first pane so we can see tmux output
-                    if let surface = terminalSurface.surface {
-                        let paneCount = ghostty_surface_tmux_pane_count(surface)
-                        if paneCount > 0 {
-                            var paneId: UInt = 0
-                            let idCount = ghostty_surface_tmux_pane_ids(surface, &paneId, 1)
-                            if idCount > 0 {
-                                let success = ghostty_surface_tmux_set_active_pane(surface, paneId)
-                                if success {
-                                    // Force the renderer to redraw with the new pane terminal
-                                    ghostty_surface_refresh(surface)
-                                }
-                                `#if` DEBUG
-                                dlog("tmux.windowsChanged panes=\(paneCount) activePaneId=\(paneId) setActive=\(success)")
-                                `#endif`
-                            }
+                    let paneCount = ghostty_surface_tmux_pane_count(liveSurface)
+                    if paneCount > 0 {
+                        var paneId: UInt = 0
+                        let idCount = ghostty_surface_tmux_pane_ids(liveSurface, &paneId, 1)
+                        if idCount > 0 {
+                            let success = ghostty_surface_tmux_set_active_pane(liveSurface, paneId)
+                            if success {
+                                // Force the renderer to redraw with the new pane terminal
+                                ghostty_surface_refresh(liveSurface)
+                            }
+                            `#if` DEBUG
+                            dlog("tmux.windowsChanged panes=\(paneCount) activePaneId=\(paneId) setActive=\(success)")
+                            `#endif`
                         }
                     }
                 default:
                     break
                 }

Based on learnings: in Sources/GhosttyTerminalView.swift, the surface may be torn down or reparented, so callers must re-read and validate self.surface before Ghostty API access.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 2863 - 2905, In the
GHOSTTY_ACTION_TMUX_STATE handling, capture the runtime surface pointer once
(from terminalSurface.surface) before DispatchQueue.main.async and inside the
dispatched closure re-read terminalSurface.surface and compare it to the
captured pointer; only proceed to set terminalSurface.tmuxActive, call
ghostty_surface_tmux_pane_count/ghostty_surface_tmux_pane_ids/ghostty_surface_tmux_set_active_pane/ghostty_surface_refresh
and post .ghosttyTmuxStateChanged if the current surface matches the captured
pointer (otherwise skip), ensuring you reference the same terminalSurface and
surface pointer when toggling tmuxActive and invoking Ghostty APIs to avoid
operating on a stale or rebound surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2863-2905: In the GHOSTTY_ACTION_TMUX_STATE handling, capture the
runtime surface pointer once (from terminalSurface.surface) before
DispatchQueue.main.async and inside the dispatched closure re-read
terminalSurface.surface and compare it to the captured pointer; only proceed to
set terminalSurface.tmuxActive, call
ghostty_surface_tmux_pane_count/ghostty_surface_tmux_pane_ids/ghostty_surface_tmux_set_active_pane/ghostty_surface_refresh
and post .ghosttyTmuxStateChanged if the current surface matches the captured
pointer (otherwise skip), ensuring you reference the same terminalSurface and
surface pointer when toggling tmuxActive and invoking Ghostty APIs to avoid
operating on a stale or rebound surface.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b53ba037-696b-427b-981b-cd38047b85b7

📥 Commits

Reviewing files that changed from the base of the PR and between df05fc7 and f9376ac.

📒 Files selected for processing (1)
  • Sources/GhosttyTerminalView.swift

Remove set_active_pane pointer swap (caused use-after-free when viewer
recreated panes during layout changes). The stream handler now decodes
%output and writes directly to the surface terminal.

@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: f6241a8c39

ℹ️ 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 thread Sources/Workspace.swift
Comment on lines +6057 to +6058
// Store the subscription (replacing any prior one)
tmuxControlModeSubscription = subscription

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 Reset tmux workspace state when tracked panel disappears

configureTerminalPanel stores tmux observation in a single tmuxControlModeSubscription, but panel teardown only removes entries from panelSubscriptions and never cancels/recomputes this tmux subscription. If the currently tracked terminal is closed while tmuxActive is true (for example, closing a pane during tmux -CC), the workspace can stay stuck in tmux-control-mode state because no later false update is guaranteed from a removed panel. Please track subscriptions per terminal panel (or recompute from live panels on close) so tmuxControlModeActive reflects current workspace state.

Useful? React with 👍 / 👎.

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

This branch was successfully deployed

1 active deployment
Preview — f6241a8c Deployed Apr 2, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants