From 1af88d0d9d2337f19c925b304b545c27f8444f25 Mon Sep 17 00:00:00 2001 From: daiimus Date: Sun, 8 Mar 2026 19:08:41 -0700 Subject: [PATCH 1/6] =?UTF-8?q?fix(tmux):=20fix=204=20TmuxSessionManager?= =?UTF-8?q?=20bugs=20=E2=80=94=20paste-buffer=20double=20%,=20focus=20rout?= =?UTF-8?q?ing,=20split=20tree=20search,=20paste=20escaping?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Finding #3: Remove extra "%" prefix in paste-buffer command. focusedPaneId already contains the "%" prefix (e.g. "%0"), so "paste-buffer -t %\(paneId)" produced "paste-buffer -t %%0" which tmux rejects. - Finding #4: handleFocusedPaneChanged now calls setFocusedPane when the window matches focusedWindowId. Previously log-only, meaning pane focus changes from tmux (e.g. select-pane) did not route input to the correct pane via send-keys. - Finding #14: findSplitContainingWithSizeHelper now checks both children of a split, not just leftmostPaneId. The old code missed panes that were a right child or not the leftmost leaf in a subtree. - Finding #24: pasteTmuxBuffer now escapes "$" and backtick characters in clipboard content. These are interpreted by tmux as format variables and shell expansion respectively, causing injection in set-buffer. --- Geistty/Sources/SSH/TmuxSessionManager.swift | 33 +++++++++++++++----- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/Geistty/Sources/SSH/TmuxSessionManager.swift b/Geistty/Sources/SSH/TmuxSessionManager.swift index 9f9d08f..35f49e1 100644 --- a/Geistty/Sources/SSH/TmuxSessionManager.swift +++ b/Geistty/Sources/SSH/TmuxSessionManager.swift @@ -1470,10 +1470,17 @@ class TmuxSessionManager: ObservableObject { ) -> (direction: TmuxSplitTree.Direction, ratio: Double, totalSize: Int)? { guard case .split(let split) = node else { return nil } - if split.left.leftmostPaneId == paneId { - // Found the split - calculate total size based on direction - let totalSize = split.direction == .horizontal ? totalCols : totalRows - return (split.direction, split.ratio, totalSize) + // Check if the target pane is a direct child of this split (on either side). + // The old code only checked split.left.leftmostPaneId, which missed panes + // that are a right child or not the leftmost leaf in a subtree. + if split.left.contains(paneId: paneId) || split.right.contains(paneId: paneId) { + // Verify that the pane is an immediate child (leaf), not deeper in a subtree. + // If it's deeper, we need to recurse to find the innermost containing split. + let isDirectChild = split.left.isPane(paneId) || split.right.isPane(paneId) + if isDirectChild { + let totalSize = split.direction == .horizontal ? totalCols : totalRows + return (split.direction, split.ratio, totalSize) + } } // Recurse into children with adjusted sizes @@ -1610,9 +1617,16 @@ class TmuxSessionManager: ObservableObject { /// Handle a tmux `%window-pane-changed` notification from the Zig viewer. /// Fired when the focused pane within a window changes (e.g., via select-pane). - /// Currently log-only; future work may update pane focus indicators. + /// If the window matches our focused window, update input routing to the new pane + /// so that keystrokes are sent to the correct pane via send-keys. func handleFocusedPaneChanged(windowId: UInt32, paneId: UInt32) { logger.info("tmux focused pane changed: @\(windowId) %\(paneId)") + + // Only update focus if this is our currently focused window. + // focusedWindowId has "@" prefix (e.g. "@0"), so format windowId to match. + if "@\(windowId)" == focusedWindowId { + setFocusedPane("%\(paneId)") + } } /// Handle a tmux `%subscription-changed` notification from the Zig viewer. @@ -1682,9 +1696,13 @@ class TmuxSessionManager: ObservableObject { // - Newlines and carriage returns must be escaped to prevent breaking // the tmux control-mode command framing (one command per line) // - Use -- to prevent content starting with - from being parsed as flags + // Also escape $ (tmux format variables) and backtick (shell expansion) + // to prevent injection when the content is passed to set-buffer. let escaped = clipboardContent .replacingOccurrences(of: "\\", with: "\\\\") .replacingOccurrences(of: "\"", with: "\\\"") + .replacingOccurrences(of: "$", with: "\\$") + .replacingOccurrences(of: "`", with: "\\`") .replacingOccurrences(of: "\n", with: "\\n") .replacingOccurrences(of: "\r", with: "\\r") @@ -1698,8 +1716,9 @@ class TmuxSessionManager: ObservableObject { logger.warning("set-buffer failed: \(content)") return } - // Buffer set successfully — now paste it into the focused pane - self.sendCommandFireAndForget("paste-buffer -t %\(paneId)") + // Buffer set successfully — now paste it into the focused pane. + // paneId already has the "%" prefix (e.g. "%0"), so don't add another. + self.sendCommandFireAndForget("paste-buffer -t \(paneId)") logger.info("Pasted clipboard to tmux pane %\(paneId) (\(clipboardContent.count) chars)") } From 9c5fb2bd0ff3d6fc2c52c8d13288c520c86091f3 Mon Sep 17 00:00:00 2001 From: daiimus Date: Sun, 8 Mar 2026 19:45:45 -0700 Subject: [PATCH 2/6] fix(tmux): remove double % prefix in paste log message The log message used "%\(paneId)" but paneId already contains the "%" prefix (e.g. "%0"), producing "%%0" in logs. The command on the previous line was already fixed, but the log line was missed. --- Geistty/Sources/SSH/TmuxSessionManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Geistty/Sources/SSH/TmuxSessionManager.swift b/Geistty/Sources/SSH/TmuxSessionManager.swift index 35f49e1..16608b4 100644 --- a/Geistty/Sources/SSH/TmuxSessionManager.swift +++ b/Geistty/Sources/SSH/TmuxSessionManager.swift @@ -1719,7 +1719,7 @@ class TmuxSessionManager: ObservableObject { // Buffer set successfully — now paste it into the focused pane. // paneId already has the "%" prefix (e.g. "%0"), so don't add another. self.sendCommandFireAndForget("paste-buffer -t \(paneId)") - logger.info("Pasted clipboard to tmux pane %\(paneId) (\(clipboardContent.count) chars)") + logger.info("Pasted clipboard to tmux pane \(paneId) (\(clipboardContent.count) chars)") } if !surface.sendTmuxCommand("set-buffer -- \"\(escaped)\"") { From f486a8ae79f4ec7264486ea71dbfd5f820735a34 Mon Sep 17 00:00:00 2001 From: daiimus Date: Sun, 8 Mar 2026 20:09:51 -0700 Subject: [PATCH 3/6] fix(tmux): use % prefix in paste test focusedPaneId to match production In production, focusedPaneId always includes the "%" prefix (e.g. "%5"). Tests were passing bare numeric IDs (e.g. "5"), which masked the paste-buffer double-% bug fix from the previous commit. With the fix removing the extra % from the command string, the % must come from focusedPaneId itself. --- Geistty/GeisttyTests/TmuxSessionManagerTests.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift index ca1ec80..cb4b0bc 100644 --- a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift +++ b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift @@ -4042,8 +4042,8 @@ extension TmuxSessionManagerTests { // Set clipboard content UIPasteboard.general.string = "clipboard text" - // Set focused pane - mgr.setFocusedPane("5") + // Set focused pane (with % prefix, as in production) + mgr.setFocusedPane("%5") mgr.pasteTmuxBuffer() @@ -4096,7 +4096,7 @@ extension TmuxSessionManagerTests { defer { UIPasteboard.general.string = savedClipboard } UIPasteboard.general.string = "line with \\backslash and \"quotes\"" - mgr.setFocusedPane("0") + mgr.setFocusedPane("%0") mgr.pasteTmuxBuffer() @@ -4121,7 +4121,7 @@ extension TmuxSessionManagerTests { defer { UIPasteboard.general.string = savedClipboard } UIPasteboard.general.string = "line1\nline2\rline3" - mgr.setFocusedPane("0") + mgr.setFocusedPane("%0") mgr.pasteTmuxBuffer() @@ -4170,7 +4170,7 @@ extension TmuxSessionManagerTests { defer { UIPasteboard.general.string = savedClipboard } UIPasteboard.general.string = "something" - mgr.setFocusedPane("0") + mgr.setFocusedPane("%0") mgr.pasteTmuxBuffer() // Simulate error on set-buffer From 437221407c84486021e4aa47881cfb7872c9ff18 Mon Sep 17 00:00:00 2001 From: daiimus Date: Sun, 8 Mar 2026 20:38:51 -0700 Subject: [PATCH 4/6] fix(tmux): address Copilot round 3 review on TmuxSessionManager - Fix comment: $ is not for tmux format variables (those use #{}). Reworded to "defense-in-depth against potential expansion in edge cases" - Add testPasteTmuxBufferEscapesDollarAndBacktick: verifies $ and backtick are escaped in set-buffer command - Add testHandleFocusedPaneChangedUpdatesFocusForMatchingWindow: sets focusedWindowId to "@1", calls handleFocusedPaneChanged(windowId: 1, paneId: 5), asserts focusedPaneId becomes "%5" - Add testHandleFocusedPaneChangedIgnoresNonMatchingWindow: verifies focusedPaneId unchanged when windowId does not match focusedWindowId - Add setFocusedWindowIdForTesting() DEBUG helper for test access --- .../TmuxSessionManagerTests.swift | 71 ++++++++++++++++++- Geistty/Sources/SSH/TmuxSessionManager.swift | 9 ++- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift index cb4b0bc..6311940 100644 --- a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift +++ b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift @@ -4130,8 +4130,35 @@ extension TmuxSessionManagerTests { "Newlines should be escaped: got \(cmd)") XCTAssertTrue(cmd.contains("\\r"), "Carriage returns should be escaped: got \(cmd)") - XCTAssertFalse(cmd.contains("\n"), - "Raw newlines must not appear in command: got \(cmd)") + XCTAssertFalse(cmd.contains("\n"), + "Raw newlines must not appear in command: got \(cmd)") + } + + /// Dollar signs and backticks must be escaped as defense-in-depth + /// against potential expansion when passed to set-buffer. + @MainActor + func testPasteTmuxBufferEscapesDollarAndBacktick() { + let mgr = TmuxSessionManager() + let mock = MockTmuxSurface() + #if DEBUG + mgr.tmuxQuerySurfaceOverride = mock + #endif + + let savedClipboard = UIPasteboard.general.string + defer { UIPasteboard.general.string = savedClipboard } + + UIPasteboard.general.string = "price is $100 and `command`" + mgr.setFocusedPane("%0") + + mgr.pasteTmuxBuffer() + + let cmd = mock.sendTmuxCommandCalls.first ?? "" + XCTAssertTrue(cmd.contains("\\$100"), + "Dollar signs should be escaped: got \(cmd)") + XCTAssertTrue(cmd.contains("\\`command\\`"), + "Backticks should be escaped: got \(cmd)") + XCTAssertFalse(cmd.contains("$100") && !cmd.contains("\\$100"), + "Unescaped dollar sign must not appear: got \(cmd)") } /// pasteTmuxBuffer should bail out when no pane is focused. @@ -4873,6 +4900,46 @@ extension TmuxSessionManagerTests { mgr.handleFocusedPaneChanged(windowId: UInt32.max, paneId: UInt32.max) } + /// When the window matches focusedWindowId, handleFocusedPaneChanged + /// should update focusedPaneId via setFocusedPane. + @MainActor + func testHandleFocusedPaneChangedUpdatesFocusForMatchingWindow() { + let mgr = TmuxSessionManager() + let mock = MockTmuxSurface() + #if DEBUG + mgr.tmuxQuerySurfaceOverride = mock + #endif + + // Set focusedWindowId to "@1" so the handler recognizes window 1 + mgr.setFocusedWindowIdForTesting("@1") + mgr.setFocusedPane("%0") // initial focus + + // Trigger pane change for matching window + mgr.handleFocusedPaneChanged(windowId: 1, paneId: 5) + XCTAssertEqual(mgr.focusedPaneId, "%5", + "focusedPaneId should be updated to %5 for matching window") + } + + /// When the window does NOT match focusedWindowId, handleFocusedPaneChanged + /// should leave focusedPaneId unchanged. + @MainActor + func testHandleFocusedPaneChangedIgnoresNonMatchingWindow() { + let mgr = TmuxSessionManager() + let mock = MockTmuxSurface() + #if DEBUG + mgr.tmuxQuerySurfaceOverride = mock + #endif + + // Set focusedWindowId to "@1" + mgr.setFocusedWindowIdForTesting("@1") + mgr.setFocusedPane("%0") // initial focus + + // Trigger pane change for a different window + mgr.handleFocusedPaneChanged(windowId: 2, paneId: 9) + XCTAssertEqual(mgr.focusedPaneId, "%0", + "focusedPaneId should remain %0 for non-matching window") + } + @MainActor func testHandleSubscriptionChangedDoesNotCrash() { let mgr = TmuxSessionManager() diff --git a/Geistty/Sources/SSH/TmuxSessionManager.swift b/Geistty/Sources/SSH/TmuxSessionManager.swift index 16608b4..a55a186 100644 --- a/Geistty/Sources/SSH/TmuxSessionManager.swift +++ b/Geistty/Sources/SSH/TmuxSessionManager.swift @@ -143,6 +143,11 @@ class TmuxSessionManager: ObservableObject { // ARCHIVED: Session 127 — needsReattach + needsReattachForTesting removed // (destroy-and-recreate approach). See docs/archive/REATTACH_PRESERVED_SURFACES_FEB_2026.swift + + /// Test-only: set focusedWindowId to test window-matching logic + func setFocusedWindowIdForTesting(_ windowId: String) { + focusedWindowId = windowId + } #endif /// Cell size from the primary surface (for calculating terminal dimensions) @@ -1696,8 +1701,8 @@ class TmuxSessionManager: ObservableObject { // - Newlines and carriage returns must be escaped to prevent breaking // the tmux control-mode command framing (one command per line) // - Use -- to prevent content starting with - from being parsed as flags - // Also escape $ (tmux format variables) and backtick (shell expansion) - // to prevent injection when the content is passed to set-buffer. + // Also escape $ and backtick as defense-in-depth against potential + // expansion in edge cases when the content is passed to set-buffer. let escaped = clipboardContent .replacingOccurrences(of: "\\", with: "\\\\") .replacingOccurrences(of: "\"", with: "\\\"") From 1fefdb5e4e48718ca1d2f9b0ab2a3c8d7865dc0d Mon Sep 17 00:00:00 2001 From: daiimus Date: Sun, 8 Mar 2026 20:57:28 -0700 Subject: [PATCH 5/6] fix(test): fix indentation and add right-child pane resize test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix inconsistent indentation (9 spaces → 8) on XCTAssertFalse call. Add testSyncSplitRatioRightChildPaneSendsResizePane which validates finding #14: the right child of a horizontal split can be correctly found and resized. The old code only checked split.left.leftmostPaneId, missing right children entirely. --- .../TmuxSessionManagerTests.swift | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift index 6311940..5de1588 100644 --- a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift +++ b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift @@ -3796,6 +3796,50 @@ extension TmuxSessionManagerTests { XCTAssertTrue(cmd.contains("11\n"), "resize-pane should target 11 rows (23 * 0.5), got: \(cmd)") } + + /// syncSplitRatioToTmux should correctly find and resize the RIGHT child pane. + /// This validates finding #14: the old code only checked split.left.leftmostPaneId, + /// which would miss panes that are the right child of a split. + @MainActor + func testSyncSplitRatioRightChildPaneSendsResizePane() { + let (mgr, log) = managerWithCommandLog() + let mock = MockTmuxSurface() + let layout = horizontalSplitLayout(paneA: 0, paneB: 1, totalCols: 80, rows: 24) + mock.stubbedWindows = [TmuxWindowInfo(id: 0, width: 80, height: 24, name: "bash")] + mock.stubbedWindowLayouts = [layout] + mock.stubbedActiveWindowId = 0 + mock.stubbedPaneIds = [0, 1] + #if DEBUG + mgr.tmuxQuerySurfaceOverride = mock + #endif + + mgr.controlModeActivated() + mgr.viewerBecameReady() + mgr.configureSurfaceManagement( + factory: { _ in nil }, + inputHandler: { _, _ in }, + resizeHandler: { _, _ in } + ) + mgr.handleTmuxStateChanged(windowCount: 1, paneCount: 2) + + // Set lastRefreshSize AFTER controlModeActivated (which resets it to nil) + #if DEBUG + mgr.setLastRefreshSizeForTesting(cols: 80, rows: 24) + #endif + + log.commands.removeAll() + // Resize pane 1 (the RIGHT child) — this was broken before finding #14 fix + mgr.syncSplitRatioToTmux(forPaneId: 1, ratio: 0.6) + + // Should send a resize-pane -x command targeting pane 1 + XCTAssertEqual(log.commands.count, 1, "Exactly one resize command should be sent for right-child pane") + let cmd = log.commands.first ?? "" + XCTAssertTrue(cmd.hasPrefix("resize-pane -t %1 -x "), + "Command should target pane 1 with -x flag, got: \(cmd)") + // Expected: available = 80 - 1 (divider) = 79, new size = max(1, Int(79 * 0.6)) = 47 + XCTAssertTrue(cmd.contains("47\n"), + "resize-pane should target 47 columns (79 * 0.6), got: \(cmd)") + } } // MARK: - Pending Input Display Tests @@ -4130,8 +4174,8 @@ extension TmuxSessionManagerTests { "Newlines should be escaped: got \(cmd)") XCTAssertTrue(cmd.contains("\\r"), "Carriage returns should be escaped: got \(cmd)") - XCTAssertFalse(cmd.contains("\n"), - "Raw newlines must not appear in command: got \(cmd)") + XCTAssertFalse(cmd.contains("\n"), + "Raw newlines must not appear in command: got \(cmd)") } /// Dollar signs and backticks must be escaped as defense-in-depth From 6f6d91151b4aea57c3593cbe5004f98b4a5f30f8 Mon Sep 17 00:00:00 2001 From: daiimus Date: Sun, 8 Mar 2026 21:09:46 -0700 Subject: [PATCH 6/6] fix(test): replace tautological dollar-sign assertion with regex The old assertion (contains("$100") && !contains("\\$100")) was always false because the escaped form is a substring of the unescaped pattern. Use a negative lookbehind regex to properly detect any unescaped dollar sign. --- Geistty/GeisttyTests/TmuxSessionManagerTests.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift index 5de1588..f4a002a 100644 --- a/Geistty/GeisttyTests/TmuxSessionManagerTests.swift +++ b/Geistty/GeisttyTests/TmuxSessionManagerTests.swift @@ -4201,8 +4201,10 @@ extension TmuxSessionManagerTests { "Dollar signs should be escaped: got \(cmd)") XCTAssertTrue(cmd.contains("\\`command\\`"), "Backticks should be escaped: got \(cmd)") - XCTAssertFalse(cmd.contains("$100") && !cmd.contains("\\$100"), - "Unescaped dollar sign must not appear: got \(cmd)") + let unescapedDollarRange = cmd.range(of: #"(?