Repository navigation
Conversation
When cmd+clicking an absolute file path or file:// URL in the terminal, open it in a side-by-side markdown panel instead of the system default editor. This leverages the existing MarkdownPanel with live file watching. Changes: - Add .internalFile case to TerminalOpenURLTarget - Route absolute paths and file:// URLs to .internalFile in resolveTerminalOpenURLTarget() - Handle .internalFile in link click handler by opening a newMarkdownSplit beside the source terminal panel - Fall back to NSWorkspace.shared.open() if workspace lookup fails Closes manaflow-ai#1283
|
@Tim-Feng is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds tmux control-mode surfaces and per-pane input routing to GhosttyNSView/GhosttyTerminalView, extends ghostty C API for tmux/pane/preedit I/O, treats absolute/file URLs as internal files (opening them in markdown panels when possible), and installs shell wrappers to auto-inject tmux -CC for interactive sessions. Changes
Sequence DiagramsequenceDiagram
participant Terminal as PTY / tmux
participant Ghostty as Ghostty surface
participant NSView as GhosttyNSView / GhosttyTerminalView
participant App as cmux App
participant Workspace as Workspace / Panel Manager
participant System as NSWorkspace
Terminal->>Ghostty: tmux pane output / windows_changed / link metadata
Ghostty->>NSView: emit action (GHOSTTY_ACTION_TMUX_WINDOWS_CHANGED / %output / OPEN_URL)
NSView->>NSView: register/create per-pane surfaces & update pane state
NSView->>App: resolveTerminalOpenURLTarget(path/url)
alt internal file
App->>Workspace: attempt newMarkdownSplit(using source workspace/panel IDs)
Workspace-->>App: success or failure
alt opened
Workspace->>NSView: render file in markdown panel
else fallback
App->>System: NSWorkspace.shared.open(fileURL)
end
else external URL
App->>System: NSWorkspace.shared.open(URL)
end
Note right of NSView: Input routing (keyboard/mouse/IME/paste) -> active pane surface via ghostty_surface_tmux_send_keys / preedit APIs
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2339-2354: The code currently discards the optional result of
resolved.workspace.newMarkdownSplit(...) and always returns true; change it to
check the returned MarkdownPanel? from newMarkdownSplit in performOnMain and if
it is nil call NSWorkspace.shared.open(URL(fileURLWithPath: path)) and return
its appropriate boolean (or true after opening) so failures fall back to opening
the file; update the block around performOnMain, AppDelegate.shared,
workspaceContainingPanel, and newMarkdownSplit to mirror the
nil-check/error-handling used in TerminalController.swift (check the optional
result and handle the nil branch by calling NSWorkspace.shared.open(...)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fba12e8b-81d3-4744-b378-8a1d2e90158d
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
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/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:2348">
P2: Validate that the path exists, is a readable file (not a directory) before opening a markdown split. Otherwise cmd‑clicking a missing path now opens a “File unavailable” markdown panel instead of falling back to the system handler.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…own panel Addresses review feedback: cmd+clicking a missing or directory path now falls back to the system handler instead of showing "File unavailable" in a markdown panel.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/GhosttyTerminalView.swift (1)
2360-2365:⚠️ Potential issue | 🟠 MajorHandle markdown split creation failure before returning success.
On Line 2360, the optional return from
resolved.workspace.newMarkdownSplit(...)is ignored, and the handler returns success unconditionally. If split creation fails, this branch silently swallows the click instead of falling back toNSWorkspace.shared.open(...).Suggested fix
- _ = resolved.workspace.newMarkdownSplit( - from: sourcePanelId, - orientation: .horizontal, - filePath: path - ) - return true + if resolved.workspace.newMarkdownSplit( + from: sourcePanelId, + orientation: .horizontal, + filePath: path + ) != nil { + return true + } + return NSWorkspace.shared.open(URL(fileURLWithPath: path))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 2360 - 2365, The current branch calls resolved.workspace.newMarkdownSplit(from:orientation:filePath:) and ignores its optional result, always returning true; update the handler to check the returned optional from newMarkdownSplit (the call in GhosttyTerminalView where newMarkdownSplit is invoked), and if it returns non-nil return true, but if it returns nil then fall back to opening the file via NSWorkspace.shared.open(URL(fileURLWithPath: path)) (or the existing fallback used elsewhere) and return the outcome accordingly so failures don't silently swallow the click.
🤖 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 2360-2365: The current branch calls
resolved.workspace.newMarkdownSplit(from:orientation:filePath:) and ignores its
optional result, always returning true; update the handler to check the returned
optional from newMarkdownSplit (the call in GhosttyTerminalView where
newMarkdownSplit is invoked), and if it returns non-nil return true, but if it
returns nil then fall back to opening the file via
NSWorkspace.shared.open(URL(fileURLWithPath: path)) (or the existing fallback
used elsewhere) and return the outcome accordingly so failures don't silently
swallow the click.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 164cd981-0f31-4045-98dc-f8d4a933329d
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
Addresses CodeRabbit review: if newMarkdownSplit returns nil, open the file via NSWorkspace instead of silently swallowing the click.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
2319-2326:⚠️ Potential issue | 🟠 MajorHandle
.internalFilebefore the browser-link preference check.Lines 2319-2326 return to
NSWorkspace.shared.open(...)for every target whenopenTerminalLinksInCmuxBrowser()is off, so the new markdown-panel flow never runs for file-path clicks. Users who disabled in-app browser links keep the old external-editor behavior for the exact path this PR is trying to change.🔧 Suggested fix
- if !BrowserLinkOpenSettings.openTerminalLinksInCmuxBrowser() { + if case .internalFile = target { + // Keep going: local-file routing should not depend on the browser-link setting. + } else if !BrowserLinkOpenSettings.openTerminalLinksInCmuxBrowser() { `#if` DEBUG dlog("link.openURL cmuxBrowser=disabled, opening externally url=\(target.url)") `#endif` return performOnMain { NSWorkspace.shared.open(target.url) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 2319 - 2326, Handle targets with .internalFile before checking BrowserLinkOpenSettings.openTerminalLinksInCmuxBrowser(): move or add a guard that detects when target.type == .internalFile (or equivalent enum/case) and routes those to the new markdown-panel/internal-file flow instead of returning early to NSWorkspace.shared.open; specifically update the logic around BrowserLinkOpenSettings.openTerminalLinksInCmuxBrowser(), target.url, performOnMain and NSWorkspace.shared.open so that .internalFile targets bypass the "openTerminalLinksInCmuxBrowser() == false" early-return and continue into the markdown-panel handling code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 364-371: The enum case .internalFile should store a full URL
(change case internalFile(String) to internalFile(URL)) and the url computed
property should return that URL directly (similar to .embeddedBrowser and
.external), not construct a local file URL from a path; update any
parsing/factory logic that currently classifies file:// URLs to only produce
.internalFile for file URLs whose host is nil/empty or "localhost" and route
file URLs with a non-local host to .external(url); also update call sites that
previously expected a String path to use the stored URL and, where a filesystem
path is needed, use url.path.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2319-2326: Handle targets with .internalFile before checking
BrowserLinkOpenSettings.openTerminalLinksInCmuxBrowser(): move or add a guard
that detects when target.type == .internalFile (or equivalent enum/case) and
routes those to the new markdown-panel/internal-file flow instead of returning
early to NSWorkspace.shared.open; specifically update the logic around
BrowserLinkOpenSettings.openTerminalLinksInCmuxBrowser(), target.url,
performOnMain and NSWorkspace.shared.open so that .internalFile targets bypass
the "openTerminalLinksInCmuxBrowser() == false" early-return and continue into
the markdown-panel handling code path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16cd50b4-fba4-4487-bb49-e2927fb9f263
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
1. Store full URL in .internalFile instead of String path
- Preserves host info for network file:// URLs
- Only treats local file:// (nil/empty/localhost host) as internal
- Remote file:// URLs route to .external
2. Bypass browser-link preference for .internalFile targets
- Users who disabled in-app browser links still get markdown panel
for local file paths (this is file routing, not browser routing)
This was referenced Mar 17, 2026
Phase 1: minimal bridge (expose tmux viewer events to C API) Phase 2: full tmux GUI integration (pane-to-surface mapping) Key finding: Ghostty already has 4245 lines of tmux control mode implementation, but events are not exposed through ghostty.h. The .windows action in stream_handler.zig is TODO. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wire up tmux control mode events from Ghostty to cmux Swift layer and add a pane text query API for link detection under tmux. Ghostty submodule (7 files, +137 lines): - tmux_state action (enter/exit) fired on control mode lifecycle - ghostty_surface_tmux_pane_text(surface, pane_id, result) query API - VT parser fix: tmux_dcs flag keeps DCS 1000p passthrough alive despite raw ESC/UTF-8 bytes in %output data (tmux-specific, does not affect XTGETTCAP/DECRQSS) - Control parser: skip stray bytes in idle state (tmux 3.6+) - ABI sync: add missing GHOSTTY_ACTION_COPY_TITLE_TO_CLIPBOARD C header ghostty.h: - GHOSTTY_ACTION_TMUX_STATE enum + action union entry - ghostty_action_tmux_state_e (ENTER/EXIT) - GHOSTTY_TMUX_PANE_ID_ANY convenience constant - ghostty_surface_tmux_pane_text() declaration Swift GhosttyTerminalView.swift: - GhosttyNSView.tmuxControlMode property - GHOSTTY_ACTION_TMUX_STATE handler - Debug: immediate + delayed pane text query on enter Verified: tmux -CC new-session in cmux DEV, session stable, pane text query returns viewport content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add handleTmuxCmdClick() and openTmuxClickedPath() to intercept cmd+click in tmux control mode, query pane text via the bridge API, and open matched file paths in the markdown panel. Also updates ghostty submodule (dumpString unwrap=false for accurate row-based line mapping). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Each tmux pane now renders as a native Ghostty surface with interactive keyboard input. The host surface's I/O thread routes tmux %output to registered pane surfaces, and keyboard events are forwarded through Manual I/O → send-keys back to tmux. Key changes: - Swift: handleTmuxWindowsChanged creates Manual I/O surfaces per pane, with io_write_cb classifying input as literal text or tmux key names - Swift: keyDown redirects to pane surface when tmuxControlMode is active, preserving full IME/interpretKeyEvents flow - C header: add tmux pane query, register/unregister, send-keys APIs - Ghostty submodule: %output routing, VT parser UTF-8 fix, octal unescape Known limitations (follow-up): - IME preedit not visible on pane surface (blind typing works) - Mouse selection not forwarded to pane surface - Initial sync is text-only MVP (no colors/cursor/modes) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add inputSurface property: returns pane surface in tmux mode so keyboard input, IME preedit, and IME cursor positioning all target the correct surface - Redirect syncPreedit to pane surface via inputSurface - Redirect firstRect(forCharacterRange:) to pane view coordinate space so IME candidate window appears at correct position - Commit preedit text as literal input when switching input methods mid-composition (captures keyboard layout change during marked text) - Track markedTextCursorOffset from setMarkedText selectedRange - Use guard var for surface in keyDown to allow tmux mode redirection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update ghostty submodule: Terminal.clone, replaceTerminal, preedit cursor bar rendering - Swift: syncPreedit uses ghostty_surface_preedit_with_cursor to pass IME cursor offset for bar cursor display - C header: add ghostty_surface_preedit_with_cursor declaration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add tmux() wrapper function to both zsh and bash shell integration scripts. When the user runs `tmux attach`, `tmux new-session`, or bare `tmux` in an interactive cmux terminal, the wrapper automatically inserts -CC to enable native pane rendering via tmux control mode. Non-client commands (ls, send-keys, kill-server, etc.) pass through unchanged. Global flags that consume a value (-L, -S, -f, -c) are properly skipped during subcommand detection. The `a` alias for `attach` is also recognized. Escape hatches: - `command tmux ...` bypasses the wrapper - `CMUX_TMUX_AUTO_CC=0` disables globally - Already inside tmux ($TMUX set) → no rewrite - Already has -CC/-C → no rewrite - Non-interactive or non-tty → no rewrite Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
hmm... this PR looks good |
Phase 2 tmux pane rendering improvements: - Fix $TMUX env leak: cmux terminals inherited $TMUX from the launcher, causing the shell wrapper to skip -CC injection. Now check CMUX_SOCKET_PATH to distinguish cmux terminals from real tmux sessions. Also unset TMUX/TMUX_PANE in reload.sh OPEN_CLEAN_ENV. - cmd+click opens files: share host callback context with pane surface (config.userdata) so OPEN_URL actions route to the Swift handler. Mouse events (mouseDown/mouseMoved/mouseUp) route to pane surface for native link detection. - Paste in tmux mode: bypass Ghostty's async clipboard flow (which sends results to host surface due to shared userdata). Read NSPasteboard directly and send via send-keys -l. Multi-line safe: split by newlines, send Enter between lines. - Arrow keys: support SS3/application mode (ESC O A/B/C/D) in addition to CSI mode (ESC [ A/B/C/D). - First-key guard: drop key events when tmuxControlMode=true but pane surfaces aren't ready yet, preventing keys from going to host. - Space key: send as tmux key name (send-keys Space) instead of send-keys -l to avoid quoting issues. - Shell wrapper: use precmd hook to install tmux() function after .zshrc (avoids oh-my-zsh override). - Refactor: extract activeTmuxPane helper to centralize pane routing (currently returns first pane, ready for multi-pane support). Known issues: - Pane surface space rendering: tmux pane has correct content but pane surface display drops spaces (terminal state context issue) - Initial sync disabled: Terminal.clone viewport position wrong - Multi-pane routing: activeTmuxPane returns first pane only Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ection Without this, drag events go to the host surface and text selection in tmux pane surfaces doesn't work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dering Fixes tmux pane surface displaying leaked title text (e.g. "echohello world" instead of "hello world"). The ESC k ... ST sequence from tmux/screen is now silently absorbed by the VT parser. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tim-Feng
changed the title
Tim-Feng
changed the title
There was a problem hiding this comment.
4 issues found across 9 files (changes from recent commits).
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="Resources/shell-integration/cmux-bash-integration.bash">
<violation number="1" location="Resources/shell-integration/cmux-bash-integration.bash:535">
P2: tmux subcommand detection misses `-T` (a value-taking global flag), which can misparse arguments and break `-CC` auto-injection.</violation>
</file>
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:4208">
P1: Tmux input routing uses `tmuxPaneSurfaces.values.first` as the active pane, so in multi-pane tmux sessions keyboard/paste/mouse/IME can target the wrong pane.</violation>
<violation number="2" location="Sources/GhosttyTerminalView.swift:5796">
P2: Pane state is removed before unregister ACK, so ACK handler cannot free pane surface/write context, causing leaks on pane removal.</violation>
</file>
<file name="Resources/shell-integration/cmux-zsh-integration.zsh">
<violation number="1" location="Resources/shell-integration/cmux-zsh-integration.zsh:591">
P2: tmux subcommand detection misses valid global options with arguments (e.g. `-T`), causing auto `-CC` injection to be skipped for attach/new flows.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| for arg in "$@"; do | ||
| if (( skip_next )); then skip_next=0; continue; fi | ||
| case "$arg" in | ||
| -L|-S|-f|-c) skip_next=1; continue ;; # these flags consume the next arg |
There was a problem hiding this comment.
P2: tmux subcommand detection misses -T (a value-taking global flag), which can misparse arguments and break -CC auto-injection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Resources/shell-integration/cmux-bash-integration.bash, line 535:
<comment>tmux subcommand detection misses `-T` (a value-taking global flag), which can misparse arguments and break `-CC` auto-injection.</comment>
<file context>
@@ -509,4 +509,41 @@ _cmux_fix_path() {
+ for arg in "$@"; do
+ if (( skip_next )); then skip_next=0; continue; fi
+ case "$arg" in
+ -L|-S|-f|-c) skip_next=1; continue ;; # these flags consume the next arg
+ -*) continue ;;
+ *) subcmd="$arg"; break ;;
</file context>
Suggested change
| -L|-S|-f|-c) skip_next=1; continue ;; # these flags consume the next arg | |
| -L|-S|-f|-c|-T) skip_next=1; continue ;; # these flags consume the next arg |
| tmuxPendingUnregister[paneId] = state.regId | ||
| ghostty_surface_tmux_unregister_pane(hostSurface, paneId, state.regId) | ||
| state.view.removeFromSuperview() | ||
| tmuxPaneSurfaces.removeValue(forKey: paneId) |
There was a problem hiding this comment.
P2: Pane state is removed before unregister ACK, so ACK handler cannot free pane surface/write context, causing leaks on pane removal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 5796:
<comment>Pane state is removed before unregister ACK, so ACK handler cannot free pane surface/write context, causing leaks on pane removal.</comment>
<file context>
@@ -5454,16 +5601,526 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ tmuxPendingUnregister[paneId] = state.regId
+ ghostty_surface_tmux_unregister_pane(hostSurface, paneId, state.regId)
+ state.view.removeFromSuperview()
+ tmuxPaneSurfaces.removeValue(forKey: paneId)
+ #if DEBUG
+ dlog("tmux.pane remove pane=\(paneId) regId=\(state.regId)")
</file context>
| for arg in "$@"; do | ||
| if (( skip_next )); then skip_next=0; continue; fi | ||
| case "$arg" in | ||
| -L|-S|-f|-c) skip_next=1; continue ;; |
There was a problem hiding this comment.
P2: tmux subcommand detection misses valid global options with arguments (e.g. -T), causing auto -CC injection to be skipped for attach/new flows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Resources/shell-integration/cmux-zsh-integration.zsh, line 591:
<comment>tmux subcommand detection misses valid global options with arguments (e.g. `-T`), causing auto `-CC` injection to be skipped for attach/new flows.</comment>
<file context>
@@ -567,8 +567,46 @@ _cmux_zshexit() {
+ for arg in "$@"; do
+ if (( skip_next )); then skip_next=0; continue; fi
+ case "$arg" in
+ -L|-S|-f|-c) skip_next=1; continue ;;
+ -*) continue ;;
+ *) subcmd="$arg"; break ;;
</file context>
Suggested change
| -L|-S|-f|-c) skip_next=1; continue ;; | |
| -L|-S|-f|-c|-T) skip_next=1; continue ;; |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
docs/phase2-review.md (1)
37-37: Refactor: Use unique subheadings to improve navigation.The document uses repeated "## Verdict", "## Recommendation", and "## Bottom line" subheadings within each main section. While this provides structural consistency, it creates ambiguous references and markdown linting warnings (MD024).
Consider using unique subheadings that include the topic context, or use level-3 headings (###) for the repeated substructure:
-## Verdict +### Verdict: Thread-safe but lifetime riskThis applies to all sections (lines 37, 65, 84, 93, 121, 140, 148, 171, 189, 199).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/phase2-review.md` at line 37, The document repeats identical second-level headings ("## Verdict", "## Recommendation", "## Bottom line") across multiple sections causing ambiguous references and MD024 warnings; update each repeated heading to be unique by appending the section context (e.g., "## Verdict — [Section Topic]" or switch to third-level headings like "### Verdict: [Section Topic]") for all occurrences noted (lines referenced in comment) so each subheading is distinct and navigable while preserving the original content under functions or sections named by their current headings.
🤖 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: The parent repo's submodule pointer references commit
8408eec2a7ca4aa0887c071e91e75d8d845b3021 which is not present on the fork's
remote; push that commit to the fork's origin main branch (manaflow-ai/ghostty)
so the commit becomes reachable, then update the submodule pointer in the parent
repo to the pushed commit SHA and commit that change; finally update
docs/ghostty-fork.md with any notes about the fork, merge/conflict details, and
the fact that this commit was pushed to the fork before bumping the submodule
pointer.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 5790-5799: The loop currently removes entries from
tmuxPaneSurfaces and only stores regId in tmuxPendingUnregister, which causes
handleTmuxPaneUnregistered(...) to lack the surface and writeContext to free;
instead, when scheduling an unregister in the for loop (where
ghostty_surface_tmux_unregister_pane is called), move the entire state object
into tmuxPendingUnregister[paneId] (preserve state.regId, view, writeContext,
etc.), do NOT call state.view.removeFromSuperview() nor
tmuxPaneSurfaces.removeValue(forKey:) there, and let
handleTmuxPaneUnregistered(paneId, ...) own removing the view, freeing
writeContext and removing from tmuxPaneSurfaces once the ack arrives; update any
code that expects tmuxPendingUnregister to contain only regId to handle the full
state structure.
- Around line 5969-5975: The pane view frame is updated but the underlying
ghostty_surface_t isn’t resized, so after computing the new CGRect via
tmuxPaneFrame(paneInfo) and assigning it to state.view.frame, also compute the
new pixel dimensions from state.view.bounds.size multiplied by the display scale
(e.g. state.view.window?.screen.scale or UIScreen.main.scale) and call
ghostty_surface_set_size(...) on the surface associated with that pane (use the
same tmuxPaneSurfaces[paneInfo.pane_id] entry — e.g. state.surface or
equivalent) to set the pixel width/height; only call the resize when the pixel
size actually changes to avoid unnecessary I/O.
- Around line 5802-5810: tmuxPaneContainer and its overlay NSViews are blocking
pointer events to GhosttyNSView (so GhosttyNSView.mouseDown/mouseMoved etc.
never run); change the container/overlay creation (tmuxPaneContainer and the
views added around lines where GhosttyPane container is created) to be
pointer-transparent by using the same passthrough pattern as
GhosttyFlashOverlayView (override hitTest to return nil) or otherwise forward
events to underlying views so tmux routing logic still receives mouse events;
update the custom overlay view class (or subclass the container) and use that
when instantiating tmuxPaneContainer and the other overlay views referenced in
the nearby block so they don’t intercept hit-testing.
- Around line 4202-4218: activeTmuxPane currently returns
tmuxPaneSurfaces.values.first which incorrectly maps inputSurface to an
arbitrary pane; update the code to track and resolve an explicit active pane ID
(e.g., store a tmuxActivePaneID property) and have activeTmuxPane fetch
tmuxPaneSurfaces[tmuxActivePaneID] (or resolve the pane from the event/mouse
location when appropriate). Ensure handleTmuxWindowsChanged, mouse/focus/hover
handlers, and any paste/IME entry points update tmuxActivePaneID (or compute
from event coordinates) so inputSurface always returns the focused/hovered pane
surface instead of values.first. Use the existing symbols activeTmuxPane,
inputSurface, tmuxPaneSurfaces, and handleTmuxWindowsChanged when making these
changes.
- Around line 4481-4513: The tmux branch currently only reads
NSPasteboard.general.string(forType: .string) which loses rich-text, file-URL
and image-only paste behaviors handled by GhosttyPasteboardHelper; update the
activeTmuxPane branch (around activeTmuxPane, ghostty_surface_tmux_send_keys,
and the fallback performBindingAction("paste_from_clipboard")) to obtain the
clipboard payload via the existing GhosttyPasteboardHelper API (use its method
that returns the best pasteable string(s) or file/url fallback instead of raw
.string), then keep the existing line-splitting/send-keys loop so file URLs and
rich-text fallbacks are preserved for tmux pastes. Ensure empty checks and early
return behavior remain the same.
- Around line 5641-5652: The code is creating fullText with String(cString: ptr)
which assumes a NUL-terminated C string; instead use the returned length field
on ghostty_text_s to build Data and decode UTF-8. Replace the String(cString:
ptr) usage in the ghostty_surface_tmux_pane_text handling (the textResult /
ghostty_text_s block) with Data(bytes: ptr, count: Int(textResult.text_len)) and
then String(decoding: thatData, as: UTF8.self) to produce fullText, keeping the
existing defer that calls ghostty_surface_free_text(surface, &mutable).
---
Nitpick comments:
In `@docs/phase2-review.md`:
- Line 37: The document repeats identical second-level headings ("## Verdict",
"## Recommendation", "## Bottom line") across multiple sections causing
ambiguous references and MD024 warnings; update each repeated heading to be
unique by appending the section context (e.g., "## Verdict — [Section Topic]" or
switch to third-level headings like "### Verdict: [Section Topic]") for all
occurrences noted (lines referenced in comment) so each subheading is distinct
and navigable while preserving the original content under functions or sections
named by their current headings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f06919b9-c6af-4359-aa17-71819db0db7b
📒 Files selected for processing (9)
Resources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshSources/GhosttyTerminalView.swiftdocs/phase2-review.mddocs/phase2-step1-4-review.mddocs/tmux-integration-plan.mdghosttyghostty.hscripts/reload.sh
✅ Files skipped from review due to trivial changes (1)
- docs/tmux-integration-plan.md
| @@ -1 +1 @@ | |||
| Subproject commit bc9be90a21997a4e5f06bf15ae2ec0f937c2dc42 | |||
| Subproject commit 8408eec2a7ca4aa0887c071e91e75d8d845b3021 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target_commit="8408eec2a7ca4aa0887c071e91e75d8d845b3021"
# Confirm current submodule state
git submodule status ghostty
git -C ghostty rev-parse HEAD
# Verify target commit is published on remote main
git -C ghostty fetch origin main --quiet
git -C ghostty merge-base --is-ancestor "$target_commit" origin/main
# Verify fork-maintenance docs mention/update for this bump
rg -n "8408eec2a7ca4aa0887c071e91e75d8d845b3021|8408eec2|ghostty-fork|conflict" docs/ghostty-fork.mdRepository: manaflow-ai/cmux
Length of output: 341
Push the ghostty commit to the fork's origin/main branch before updating the submodule pointer.
The commit 8408eec2a7ca4aa0887c071e91e75d8d845b3021 is not reachable on the remote fork, which will cause clone failures. Ensure this commit is pushed to manaflow-ai/ghostty main branch, then update the parent pointer. Also update docs/ghostty-fork.md with any fork/conflict notes for this change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ghostty` at line 1, The parent repo's submodule pointer references commit
8408eec2a7ca4aa0887c071e91e75d8d845b3021 which is not present on the fork's
remote; push that commit to the fork's origin main branch (manaflow-ai/ghostty)
so the commit becomes reachable, then update the submodule pointer in the parent
repo to the pushed commit SHA and commit that change; finally update
docs/ghostty-fork.md with any notes about the fork, merge/conflict details, and
the fact that this commit was pushed to the fork before bumping the submodule
pointer.
Comment on lines
+4481
to
4513
| // In tmux mode, Ghostty's async clipboard flow would send the result | ||
| // back to the host surface (shared userdata). Instead, read the | ||
| // clipboard directly and route through the pane's send-keys path. | ||
| if let paneState = activeTmuxPane { | ||
| guard let str = NSPasteboard.general.string(forType: .string), | ||
| !str.isEmpty else { return } | ||
| let hostSurface = paneState.writeContext.pointee.hostSurface | ||
| let paneId = paneState.writeContext.pointee.paneId | ||
| // Split by newlines to avoid raw newlines breaking the tmux | ||
| // command stream. Each line is sent as literal text, with | ||
| // explicit Enter key events between lines. | ||
| let lines = str.components(separatedBy: .newlines) | ||
| for (i, line) in lines.enumerated() { | ||
| if !line.isEmpty, let data = line.data(using: .utf8) { | ||
| data.withUnsafeBytes { rawBuf in | ||
| let ptr = rawBuf.baseAddress!.assumingMemoryBound(to: UInt8.self) | ||
| ghostty_surface_tmux_send_keys(hostSurface, paneId, ptr, data.count, 0) | ||
| } | ||
| } | ||
| // Send Enter between lines (not after the last line) | ||
| if i < lines.count - 1 { | ||
| "Enter".withCString { cstr in | ||
| ghostty_surface_tmux_send_keys( | ||
| hostSurface, paneId, | ||
| UnsafePointer<UInt8>(OpaquePointer(cstr)), | ||
| strlen(cstr), 1 // key_type=1 (key name) | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| return | ||
| } | ||
| _ = performBindingAction("paste_from_clipboard") |
There was a problem hiding this comment.
Tmux paste drops the existing pasteboard decoding behavior.
This branch only reads .string, so file URLs, rich-text fallback text, and clipboard-only images stop pasting in tmux even though normal terminal paste supports them through GhosttyPasteboardHelper.
Possible fix
- guard let str = NSPasteboard.general.string(forType: .string),
- !str.isEmpty else { return }
+ let pasteboard = NSPasteboard.general
+ let str = GhosttyPasteboardHelper.stringContents(from: pasteboard)
+ ?? GhosttyPasteboardHelper.saveClipboardImageIfNeeded(
+ from: pasteboard,
+ assumeNoText: true
+ )
+ guard let str, !str.isEmpty else { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 4481 - 4513, The tmux branch
currently only reads NSPasteboard.general.string(forType: .string) which loses
rich-text, file-URL and image-only paste behaviors handled by
GhosttyPasteboardHelper; update the activeTmuxPane branch (around
activeTmuxPane, ghostty_surface_tmux_send_keys, and the fallback
performBindingAction("paste_from_clipboard")) to obtain the clipboard payload
via the existing GhosttyPasteboardHelper API (use its method that returns the
best pasteable string(s) or file/url fallback instead of raw .string), then keep
the existing line-splitting/send-keys loop so file URLs and rich-text fallbacks
are preserved for tmux pastes. Ensure empty checks and early return behavior
remain the same.
Comment on lines
+5641
to
+5652
| var textResult = ghostty_text_s() | ||
| guard ghostty_surface_tmux_pane_text(surface, UInt.max, &textResult), | ||
| let ptr = textResult.text else { | ||
| #if DEBUG | ||
| dlog("tmux.cmdClick pane text query failed") | ||
| #endif | ||
| return false | ||
| } | ||
| defer { var mutable = textResult; ghostty_surface_free_text(surface, &mutable) } | ||
|
|
||
| let fullText = String(cString: ptr) | ||
| let lines = fullText.components(separatedBy: "\n") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the ghostty_text_s contract and compare local decoding sites.
mapfile -t headers < <(fd 'ghostty\.h$' || true)
for header in "${headers[@]}"; do
printf '=== %s ===\n' "$header"
sed -n '/typedef struct ghostty_text_s/,/} ghostty_text_s;/p' "$header"
done
sed -n '4597,4608p;5641,5652p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 116
🏁 Script executed:
# Find ghostty.h header file
fd 'ghostty\.h$'Repository: manaflow-ai/cmux
Length of output: 69
🏁 Script executed:
# Once we have the header location, examine the ghostty_text_s struct
# Also look at the code in GhosttyTerminalView.swift at the mentioned lines
rg 'typedef struct ghostty_text_s' -A 10Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check the actual code at lines 5641-5652
sed -n '5641,5652p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 552
🏁 Script executed:
# Search for other uses of ghostty_text_s to see decode patterns
rg 'ghostty_text_s' Sources/GhosttyTerminalView.swift -B 2 -A 5 | head -100Repository: manaflow-ai/cmux
Length of output: 1013
🏁 Script executed:
# Read the ghostty.h file to see ghostty_text_s struct definition
cat ghostty.h | grep -A 10 'typedef struct ghostty_text_s'Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Search for proper usage of text_len with ghostty_text_s decoding
rg 'text\.text_len|textResult\.text_len' -B 3 -A 3Repository: manaflow-ai/cmux
Length of output: 1138
🏁 Script executed:
# Look for String decoding patterns with ghostty_text_s in the file
rg 'String.*text\.text\)|Data.*text\.text' Sources/GhosttyTerminalView.swift -B 2 -A 2Repository: manaflow-ai/cmux
Length of output: 301
Decode pane text with text_len, not String(cString:).
The code at lines 5641-5652 uses String(cString: ptr) which assumes NUL termination. Other decode sites in the codebase (GhosttyTerminalView.swift and TerminalController.swift) consistently use Data(bytes: ptr, count: Int(text.text_len)) followed by String(decoding: _, as: UTF8.self). The text_len field must be used to avoid truncation or buffer overread.
Fix
- let fullText = String(cString: ptr)
+ let fullText = String(
+ decoding: Data(bytes: ptr, count: Int(textResult.text_len)),
+ as: UTF8.self
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 5641 - 5652, The code is
creating fullText with String(cString: ptr) which assumes a NUL-terminated C
string; instead use the returned length field on ghostty_text_s to build Data
and decode UTF-8. Replace the String(cString: ptr) usage in the
ghostty_surface_tmux_pane_text handling (the textResult / ghostty_text_s block)
with Data(bytes: ptr, count: Int(textResult.text_len)) and then String(decoding:
thatData, as: UTF8.self) to produce fullText, keeping the existing defer that
calls ghostty_surface_free_text(surface, &mutable).
Comment on lines
+5790
to
+5799
| // Remove stale panes (two-phase: unregister first, destroy on ack) | ||
| for paneId in toRemove { | ||
| guard let state = tmuxPaneSurfaces[paneId] else { continue } | ||
| tmuxPendingUnregister[paneId] = state.regId | ||
| ghostty_surface_tmux_unregister_pane(hostSurface, paneId, state.regId) | ||
| state.view.removeFromSuperview() | ||
| tmuxPaneSurfaces.removeValue(forKey: paneId) | ||
| #if DEBUG | ||
| dlog("tmux.pane remove pane=\(paneId) regId=\(state.regId)") | ||
| #endif |
There was a problem hiding this comment.
Keep removed panes alive until the unregister ack arrives.
Removing tmuxPaneSurfaces[paneId] here means handleTmuxPaneUnregistered(...) no longer has the surface or writeContext to free, so every topology change leaks them. Keep the full state in a pending-unregister structure and let the ack handler own destruction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 5790 - 5799, The loop
currently removes entries from tmuxPaneSurfaces and only stores regId in
tmuxPendingUnregister, which causes handleTmuxPaneUnregistered(...) to lack the
surface and writeContext to free; instead, when scheduling an unregister in the
for loop (where ghostty_surface_tmux_unregister_pane is called), move the entire
state object into tmuxPendingUnregister[paneId] (preserve state.regId, view,
writeContext, etc.), do NOT call state.view.removeFromSuperview() nor
tmuxPaneSurfaces.removeValue(forKey:) there, and let
handleTmuxPaneUnregistered(paneId, ...) own removing the view, freeing
writeContext and removing from tmuxPaneSurfaces once the ack arrives; update any
code that expects tmuxPendingUnregister to contain only regId to handle the full
state structure.
Comment on lines
+5802
to
+5810
| // Ensure container exists — add above the scroll view so it's | ||
| // not hidden behind the host terminal's Metal rendering layer. | ||
| if tmuxPaneContainer == nil { | ||
| let container = NSView(frame: bounds) | ||
| container.wantsLayer = true | ||
| container.autoresizingMask = [.width, .height] | ||
| // Walk up: GhosttyNSView → documentView → clipView → scrollView → GhosttySurfaceScrollView | ||
| if let parentView = enclosingScrollView?.superview { | ||
| parentView.addSubview(container, positioned: .above, relativeTo: nil) |
There was a problem hiding this comment.
Make the tmux overlay views pointer-transparent.
These views are inserted above the hosted terminal as plain NSViews. They become the topmost hit-test targets, so GhosttyNSView.mouseDown/mouseMoved/... will not see pointer events and the manual tmux routing below cannot run. Use a passthrough view here (for example the same hitTest -> nil pattern as GhosttyFlashOverlayView) or forward events explicitly.
Also applies to: 5841-5850
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 5802 - 5810,
tmuxPaneContainer and its overlay NSViews are blocking pointer events to
GhosttyNSView (so GhosttyNSView.mouseDown/mouseMoved etc. never run); change the
container/overlay creation (tmuxPaneContainer and the views added around lines
where GhosttyPane container is created) to be pointer-transparent by using the
same passthrough pattern as GhosttyFlashOverlayView (override hitTest to return
nil) or otherwise forward events to underlying views so tmux routing logic still
receives mouse events; update the custom overlay view class (or subclass the
container) and use that when instantiating tmuxPaneContainer and the other
overlay views referenced in the nearby block so they don’t intercept
hit-testing.
Comment on lines
+5969
to
+5975
| // Update layout for existing panes | ||
| for i in 0..<Int(written) { | ||
| let paneInfo = panes[i] | ||
| if let state = tmuxPaneSurfaces[paneInfo.pane_id], !toAdd.contains(paneInfo.pane_id) { | ||
| state.view.frame = tmuxPaneFrame(paneInfo) | ||
| } | ||
| } |
There was a problem hiding this comment.
Resize the pane surface when the pane frame changes.
Updating only state.view.frame leaves the backing ghostty_surface_t at its old pixel size. These manual-I/O pane surfaces need the same ghostty_surface_set_size(...) maintenance as the normal terminal path, or rendering and hit-testing drift after tmux layout changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 5969 - 5975, The pane view
frame is updated but the underlying ghostty_surface_t isn’t resized, so after
computing the new CGRect via tmuxPaneFrame(paneInfo) and assigning it to
state.view.frame, also compute the new pixel dimensions from
state.view.bounds.size multiplied by the display scale (e.g.
state.view.window?.screen.scale or UIScreen.main.scale) and call
ghostty_surface_set_size(...) on the surface associated with that pane (use the
same tmuxPaneSurfaces[paneInfo.pane_id] entry — e.g. state.surface or
equivalent) to set the pixel width/height; only call the resize when the pixel
size actually changes to avoid unnecessary I/O.
The tmux status bar cannot be exposed by leaving a gap in the pane container. Ghostty's tmux control mode intercepts all output via the viewer; the host terminal does not reliably render tmux chrome/status bar as a separate visible layer. Revert container frame to full parent bounds. A native tmux status indicator can be added as a future enhancement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
Sources/GhosttyTerminalView.swift (6)
4481-4511:⚠️ Potential issue | 🟠 MajorUse
GhosttyPasteboardHelperin the tmux paste path too.This branch only reads
.string, so file URLs, rich-text fallback text, and image-only clipboard content stop pasting in tmux even though the normal terminal path supports them.Possible fix
- guard let str = NSPasteboard.general.string(forType: .string), - !str.isEmpty else { return } + let pasteboard = NSPasteboard.general + let str = GhosttyPasteboardHelper.stringContents(from: pasteboard) + ?? GhosttyPasteboardHelper.saveClipboardImageIfNeeded( + from: pasteboard, + assumeNoText: true + ) + guard let str, !str.isEmpty else { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 4481 - 4511, The tmux branch reads only NSPasteboard.general.string(forType: .string), which drops file URLs, rich-text fallbacks and image-only clipboards; replace that direct read with the existing GhosttyPasteboardHelper pasteboard-reading API (use GhosttyPasteboardHelper to obtain the pasteboard content/string(s) and any fallback text or image-derived text) before splitting into lines and sending via ghostty_surface_tmux_send_keys; keep using activeTmuxPane, hostSurface, paneId and the existing loop/Enter logic but source the text from GhosttyPasteboardHelper so all clipboard types supported by the normal terminal paste path also work in the tmux paste path.
5641-5652:⚠️ Potential issue | 🔴 CriticalDecode tmux pane text with
text_len, notString(cString:).
ghostty_text_sis length-delimited. Line 5651 assumes NUL termination, which can truncate pane content or read past the returned buffer.Possible fix
- let fullText = String(cString: ptr) + let fullText = String( + decoding: Data(bytes: ptr, count: Int(textResult.text_len)), + as: UTF8.self + )#!/bin/bash set -euo pipefail fd 'ghostty.h$' -x sh -c ' printf "=== %s ===\n" "$1" sed -n "/typedef struct ghostty_text_s/,/} ghostty_text_s;/p" "$1" ' sh {} sed -n '4597,4608p;5641,5652p' Sources/GhosttyTerminalView.swift🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 5641 - 5652, The code incorrectly decodes the tmux pane buffer using String(cString:) which assumes NUL-termination; instead read the length-delimited ghostty_text_s buffer using its text_len field returned by ghostty_surface_tmux_pane_text and build the String from the raw bytes (respecting text_len) before splitting into lines; update the usage around ghostty_text_s, ghostty_surface_tmux_pane_text, ghostty_surface_free_text and the fullText/lines construction so you create the String from the pointer + text_len rather than String(cString:).
4202-4218:⚠️ Potential issue | 🟠 MajorStop routing
inputSurfacethroughtmuxPaneSurfaces.values.first.This still picks an arbitrary pane. Once there is more than one tmux pane, keyboard, paste, IME, and hover/click routing can all land in the wrong surface. Track an explicit active pane ID and resolve
inputSurfacefrom that instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 4202 - 4218, Replace the arbitrary selection of a tmux pane via tmuxPaneSurfaces.values.first used by activeTmuxPane/inputSurface with an explicit tracked active pane identifier: add a stored property (e.g., activeTmuxPaneID or activeTmuxPaneKey) that is updated wherever pane focus/hover/click changes, then change activeTmuxPane to look up tmuxPaneSurfaces[activeTmuxPaneID] when tmuxControlMode is true (falling back to nil), and have inputSurface return that pane.surface when present; update any event handlers that change focus to set the new active pane ID instead of relying on values.first.
5967-5972:⚠️ Potential issue | 🟠 MajorResize the backing pane surface when the pane frame changes.
Updating only
state.view.frameleaves the manual-I/O pane surface at its old pixel size. After tmux layout changes, rendering and hit-testing drift until the pane is recreated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 5967 - 5972, When updating existing panes in the loop (tmuxPaneSurfaces[paneInfo.pane_id]), also detect when the view frame actually changed (compare state.view.frame to tmuxPaneFrame(paneInfo)), set the new frame, and then resize the pane's backing surface so its pixel buffer matches the new view size (e.g. call the surface/resizing API on the state object after changing state.view.frame). Ensure you reference tmuxPaneSurfaces, paneInfo.pane_id, state.view.frame and tmuxPaneFrame(paneInfo) so the backing surface is updated whenever the layout changes.
5802-5809:⚠️ Potential issue | 🟠 MajorMake the tmux container and pane host views pointer-transparent.
These views sit above the terminal portal. As plain
NSViewhit-test targets, they intercept mouse events beforeGhosttyNSView.mouseDown/mouseMoved/...can run, which breaks the manual tmux routing below. Use a passthrough view here (hitTest -> nil) or forward events explicitly.Also applies to: 5844-5848
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 5802 - 5809, The container and tmux pane host views (tmuxPaneContainer and the pane-host view created later) are intercepting mouse events; make them pointer-transparent by using a passthrough NSView implementation (e.g., a PassthroughView subclass whose hitTest(_:) returns nil) instead of plain NSView so events fall through to GhosttyNSView.mouseDown/mouseMoved/etc.; update the allocation sites where you create `let container = NSView(frame: ...)` (and the corresponding pane-host creation at the 5844–5848 block) to instantiate the passthrough view class and ensure autoresizingMask and addSubview usage remain the same.
5790-5797:⚠️ Potential issue | 🟠 MajorDon’t drop pane state before the unregister ack.
Removing
tmuxPaneSurfaces[paneId]here leaveshandleTmuxPaneUnregistered(...)with only a regId, so the surface andwriteContextnever get freed on topology changes. Move the full state into a pending-unregister map and let the ack handler own teardown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 5790 - 5797, Currently you remove the pane state from tmuxPaneSurfaces and only store regId in tmuxPendingUnregister, which prevents handleTmuxPaneUnregistered from freeing the surface and writeContext; instead, move the full state (not just state.regId) into tmuxPendingUnregister[paneId], call ghostty_surface_tmux_unregister_pane(hostSurface, paneId, state.regId) as before, and do not call state.view.removeFromSuperview() or tmuxPaneSurfaces.removeValue(forKey: paneId) here; update handleTmuxPaneUnregistered to look up and tear down the full state from tmuxPendingUnregister (removing the view, freeing writeContext, and clearing the pending entry) so the ack handler owns the teardown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 5605-5620: The tmux branch currently returns immediately after
sending the mouse event to the pane surface, which prevents the cmd+click
file-path fallback (handleTmuxCmdClick) from running and the path-matcher from
capturing paths with spaces; update the mouseDown handling for activeTmuxPane
(paneState, paneSurface, ghostty_surface_mouse_pos,
ghostty_surface_mouse_button, modsFromEvent) so that after forwarding the mouse
event you only return if the pane surface handled a command-click link;
otherwise, detect a command-click (event.modifierFlags.contains(.command)) and
call handleTmuxCmdClick(...) as a fallback before returning; also replace the
current matcher logic used by handleTmuxCmdClick with a more permissive pattern
that allows spaces in file paths (or supports quoted/tilde paths) so paths like
"~/My File.md" are matched fully.
---
Duplicate comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4481-4511: The tmux branch reads only
NSPasteboard.general.string(forType: .string), which drops file URLs, rich-text
fallbacks and image-only clipboards; replace that direct read with the existing
GhosttyPasteboardHelper pasteboard-reading API (use GhosttyPasteboardHelper to
obtain the pasteboard content/string(s) and any fallback text or image-derived
text) before splitting into lines and sending via
ghostty_surface_tmux_send_keys; keep using activeTmuxPane, hostSurface, paneId
and the existing loop/Enter logic but source the text from
GhosttyPasteboardHelper so all clipboard types supported by the normal terminal
paste path also work in the tmux paste path.
- Around line 5641-5652: The code incorrectly decodes the tmux pane buffer using
String(cString:) which assumes NUL-termination; instead read the
length-delimited ghostty_text_s buffer using its text_len field returned by
ghostty_surface_tmux_pane_text and build the String from the raw bytes
(respecting text_len) before splitting into lines; update the usage around
ghostty_text_s, ghostty_surface_tmux_pane_text, ghostty_surface_free_text and
the fullText/lines construction so you create the String from the pointer +
text_len rather than String(cString:).
- Around line 4202-4218: Replace the arbitrary selection of a tmux pane via
tmuxPaneSurfaces.values.first used by activeTmuxPane/inputSurface with an
explicit tracked active pane identifier: add a stored property (e.g.,
activeTmuxPaneID or activeTmuxPaneKey) that is updated wherever pane
focus/hover/click changes, then change activeTmuxPane to look up
tmuxPaneSurfaces[activeTmuxPaneID] when tmuxControlMode is true (falling back to
nil), and have inputSurface return that pane.surface when present; update any
event handlers that change focus to set the new active pane ID instead of
relying on values.first.
- Around line 5967-5972: When updating existing panes in the loop
(tmuxPaneSurfaces[paneInfo.pane_id]), also detect when the view frame actually
changed (compare state.view.frame to tmuxPaneFrame(paneInfo)), set the new
frame, and then resize the pane's backing surface so its pixel buffer matches
the new view size (e.g. call the surface/resizing API on the state object after
changing state.view.frame). Ensure you reference tmuxPaneSurfaces,
paneInfo.pane_id, state.view.frame and tmuxPaneFrame(paneInfo) so the backing
surface is updated whenever the layout changes.
- Around line 5802-5809: The container and tmux pane host views
(tmuxPaneContainer and the pane-host view created later) are intercepting mouse
events; make them pointer-transparent by using a passthrough NSView
implementation (e.g., a PassthroughView subclass whose hitTest(_:) returns nil)
instead of plain NSView so events fall through to
GhosttyNSView.mouseDown/mouseMoved/etc.; update the allocation sites where you
create `let container = NSView(frame: ...)` (and the corresponding pane-host
creation at the 5844–5848 block) to instantiate the passthrough view class and
ensure autoresizingMask and addSubview usage remain the same.
- Around line 5790-5797: Currently you remove the pane state from
tmuxPaneSurfaces and only store regId in tmuxPendingUnregister, which prevents
handleTmuxPaneUnregistered from freeing the surface and writeContext; instead,
move the full state (not just state.regId) into tmuxPendingUnregister[paneId],
call ghostty_surface_tmux_unregister_pane(hostSurface, paneId, state.regId) as
before, and do not call state.view.removeFromSuperview() or
tmuxPaneSurfaces.removeValue(forKey: paneId) here; update
handleTmuxPaneUnregistered to look up and tear down the full state from
tmuxPendingUnregister (removing the view, freeing writeContext, and clearing the
pending entry) so the ack handler owns the teardown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fdfd9414-57a1-47ef-b9b2-13a15ebf86ea
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
Comment on lines
+5605
to
+5620
| // In tmux mode, route mouse events to the pane surface so Ghostty's | ||
| // native link detection works on the pane's real Terminal content. | ||
| #if DEBUG | ||
| dlog("tmux.mouseDown check tmuxControlMode=\(tmuxControlMode) paneCount=\(tmuxPaneSurfaces.count)") | ||
| #endif | ||
| if let paneState = activeTmuxPane { | ||
| let paneSurface = paneState.surface | ||
| let point = paneState.view.convert(event.locationInWindow, from: nil) | ||
| let y = paneState.view.bounds.height - point.y | ||
| #if DEBUG | ||
| dlog("tmux.mouseDown routed to pane regId=\(paneState.regId) point=(\(String(format: "%.0f", point.x)),\(String(format: "%.0f", y))) cmd=\(event.modifierFlags.contains(.command))") | ||
| #endif | ||
| ghostty_surface_mouse_pos(paneSurface, point.x, y, modsFromEvent(event)) | ||
| _ = ghostty_surface_mouse_button(paneSurface, GHOSTTY_MOUSE_PRESS, GHOSTTY_MOUSE_LEFT, modsFromEvent(event)) | ||
| return | ||
| } |
There was a problem hiding this comment.
The tmux file-path cmd+click fallback is never reached.
The tmux mouseDown branch returns right after forwarding the click to the pane surface, so handleTmuxCmdClick(...) below never runs for plain file paths. Even after wiring it in, the current matcher still stops at spaces, so common paths like ~/My File.md would truncate.
Also applies to: 5627-5670
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 5605 - 5620, The tmux branch
currently returns immediately after sending the mouse event to the pane surface,
which prevents the cmd+click file-path fallback (handleTmuxCmdClick) from
running and the path-matcher from capturing paths with spaces; update the
mouseDown handling for activeTmuxPane (paneState, paneSurface,
ghostty_surface_mouse_pos, ghostty_surface_mouse_button, modsFromEvent) so that
after forwarding the mouse event you only return if the pane surface handled a
command-click link; otherwise, detect a command-click
(event.modifierFlags.contains(.command)) and call handleTmuxCmdClick(...) as a
fallback before returning; also replace the current matcher logic used by
handleTmuxCmdClick with a more permissive pattern that allows spaces in file
paths (or supports quoted/tilde paths) so paths like "~/My File.md" are matched
fully.
Add a green status bar at the bottom of tmux pane surfaces showing
session name, window list, pane title, and time — matching tmux's
default status-left/status-right format.
Implementation:
- NSView with green background + left/right NSTextFields
- Queries tmux display-message with #{T:status-left/right} format
- Targets specific pane ID (-t %paneId) for correct session context
- Auto-refreshes every 15 seconds via Timer
- Pane surface frame adjusted to reserve space above status bar
Known limitations (TODO):
- Query uses external tmux process (should use control channel)
- No -L/-S socket parameter (breaks non-default server)
- paneId fixed at creation time (stale on window switch)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Prefix key state machine: Ctrl+B enters pending mode, next key resolves to tmux command (loaded dynamically from list-keys -T prefix) - Ctrl+B D detach works: %exit parsed by control.zig, tmuxExit() cleans up pane surfaces, parser reset to ground state - ghostty_surface_tmux_command() C API for raw tmux commands - Active pane tracking + select-pane sync on click - tmux status bar with session/window info (15s auto-refresh) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
3 issues found across 3 files (changes from recent commits).
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/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:5012">
P2: Keyboard prefix pane-switch commands do not synchronize `tmuxActivePaneId`, so subsequent key/IME input can be routed to the wrong tmux pane.</violation>
<violation number="2" location="Sources/GhosttyTerminalView.swift:5960">
P2: Status bar polling is permanently bound to the initial pane ID, so after pane/window changes it can query a stale pane and show incorrect/empty status.</violation>
<violation number="3" location="Sources/GhosttyTerminalView.swift:6227">
P2: tmux status/prefix queries spawn an unbound external `tmux` client instead of using the active control-mode session context, risking wrong/empty bindings and status data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| #if DEBUG | ||
| dlog("tmux.prefix EXEC key=\(key) cmd=\(command)") | ||
| #endif | ||
| tmuxSendCommand(command) |
There was a problem hiding this comment.
P2: Keyboard prefix pane-switch commands do not synchronize tmuxActivePaneId, so subsequent key/IME input can be routed to the wrong tmux pane.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 5012:
<comment>Keyboard prefix pane-switch commands do not synchronize `tmuxActivePaneId`, so subsequent key/IME input can be routed to the wrong tmux pane.</comment>
<file context>
@@ -4957,6 +4988,44 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ #if DEBUG
+ dlog("tmux.prefix EXEC key=\(key) cmd=\(command)")
+ #endif
+ tmuxSendCommand(command)
+ } else {
+ #if DEBUG
</file context>
Instead of a blank pane surface that requires pressing Enter, capture the tmux pane's visible content on creation using `capture-pane -p -e` and feed it to the pane surface via ghostty_surface_process_output. Safety: - Captures regId before async dispatch, verifies pane still alive on completion (prevents use-after-free if pane is destroyed during query) - Uses raw stdout (no trimming) to preserve meaningful whitespace - Dispatches process_output on main thread after lifetime check Known limitation: query uses external tmux process (should use control channel in the future, same as status bar and prefix config). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:6300">
P2: `tmuxQueryRaw` can deadlock by waiting for process exit before reading stdout, and it is now used by initial pane capture (`capture-pane -p -e`) which can produce large output.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+6300
to
+6301
| process.waitUntilExit() | ||
| let data = pipe.fileHandleForReading.readDataToEndOfFile() |
There was a problem hiding this comment.
P2: tmuxQueryRaw can deadlock by waiting for process exit before reading stdout, and it is now used by initial pane capture (capture-pane -p -e) which can produce large output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 6300:
<comment>`tmuxQueryRaw` can deadlock by waiting for process exit before reading stdout, and it is now used by initial pane capture (`capture-pane -p -e`) which can produce large output.</comment>
<file context>
@@ -6248,6 +6254,57 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ process.standardError = FileHandle.nullDevice
+ do {
+ try process.run()
+ process.waitUntilExit()
+ let data = pipe.fileHandleForReading.readDataToEndOfFile()
+ return String(data: data, encoding: .utf8) ?? ""
</file context>
Suggested change
| process.waitUntilExit() | |
| let data = pipe.fileHandleForReading.readDataToEndOfFile() | |
| let data = pipe.fileHandleForReading.readDataToEndOfFile() | |
| process.waitUntilExit() |
Fix blank pane on tmux attach by syncing tmux client geometry and using capture-pane with proper row truncation. - Send refresh-client -C to sync tmux client size with pane surface - tmuxDesiredClientSize() / tmuxCurrentClientSize() helpers - tmuxInitialSync: truncate capture-pane to pane surface rows, keeping bottom rows (suffix) so prompt stays visible - Cursor position adjusted for dropped top rows Known issue: cursor may be offset by one row after initial sync. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
…ial sync - Add opaque layer-backed background to tmux pane container so the host terminal's Metal layer (e.g. "Last login" text) doesn't bleed through gaps in the pane surface rendering. - Simplify tmuxInitialSync back to basic capture-pane replay without retry/padding/truncation. The geometry sync via refresh-client -C handles size matching; complex alignment logic was causing more problems than it solved. - Remove tmuxDesiredClientSize/tmuxCurrentClientSize helpers and didInitialSync/tmuxRequestedClientSize state (no longer needed). Known limitation: re-attach after filling screen with Enter may show content shifted up due to capture-pane returning more rows than pane surface has (tmux defaults to 80x24 before refresh-client takes effect). This requires Terminal.clone for a proper fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:5890">
P2: Tmux pane container caches background color at creation and is never refreshed on later theme/background updates, causing stale/mismatched background in tmux mode.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ed padding - Hide host terminal rendering via alphaValue=0 (not isHidden) to prevent "Last login" text bleed-through while keeping GhosttyNSView in the event chain for mouse handling (selection, cmd+click). - Initial sync padding: use cursor position to calculate minimal top padding instead of blindly padding to fill surfaceRows. This prevents content from being pushed off-screen. - Retry-based initial sync waits for tmux geometry to converge with pane surface rows before replaying capture-pane content. Known limitation: re-attach after filling screen with Enter may show content at wrong position (tmux geometry convergence timing issue). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:5887">
P1: Host view alpha can remain stuck at 0 because restoration is tied only to tmux cleanup, while some windows-changed paths return without cleanup.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Hide host terminal rendering but keep the view in the event | ||
| // chain for mouse handling. alphaValue=0 makes the Metal layer | ||
| // invisible without setting isHidden (which breaks hit-testing). | ||
| self.alphaValue = 0 |
There was a problem hiding this comment.
P1: Host view alpha can remain stuck at 0 because restoration is tied only to tmux cleanup, while some windows-changed paths return without cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 5887:
<comment>Host view alpha can remain stuck at 0 because restoration is tied only to tmux cleanup, while some windows-changed paths return without cleanup.</comment>
<file context>
@@ -5880,15 +5880,11 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ // Hide host terminal rendering but keep the view in the event
+ // chain for mouse handling. alphaValue=0 makes the Metal layer
+ // invisible without setting isHidden (which breaks hit-testing).
+ self.alphaValue = 0
// Walk up: GhosttyNSView → documentView → clipView → scrollView → GhosttySurfaceScrollView
if let parentView = enclosingScrollView?.superview {
</file context>
Author
Hi, you may want to try "Tidey", rewrite on iTerm2, it's cmux+editor https://github.com/Tim-Feng/Tidey |
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
Changes by phase
Phase 0: cmd+click → markdown panel
resolveTerminalOpenURLTarget()detects local file pathsPhase 1: tmux control mode bridge (Ghostty fork)
tmux_stateaction +ghostty_surface_tmux_pane_text()query API%outputoctal unescapePhase 2: native pane rendering
-CC(precmd hook; fixes leaked$TMUXenv var)ghostty_surface_new()with Manual I/O mode%outputrouting to pane surface viaprocessOutput()send-keysliteral/key-name classification, SS3 arrow keyssend-keys -l(multi-line safe: splits by newline + Enter)config.userdata) so OPEN_URL actions route correctlyESC k ... ST(tmux/screen set-title) to prevent title text leaking to screenactiveTmuxPanehelper centralizes pane routing logicGhostty fork changes (
tmux-control-mode-bridgebranch)parse_table.zig: ESC k → sos_pm_apc_stringParser.zig: tmux_dcs override for 0xA0-0xFF bytes + ESC k testcontrol.zig:unescapeTmuxOctal()+ testsstream_handler.zig: %output routing, pane register/unregisterembedded.zig:ghostty_surface_tmux_send_keys(), pane query/register APIsTermio.zig:replaceTerminal()Terminal.zig:clone()+restoreScreenState()generic.zig+State.zig: preedit bar cursor renderingKnown issues
Terminal.clone()viewport position is wrong; pane starts empty (user must press Enter to see prompt)activeTmuxPanecurrently returns only the first paneTest plan
echo /path/to/fileoutput displays correctly (with spaces)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes