Cancel tmux copy-mode before paste-buffer in TmuxBackend.sendText

[dgershman]

Closes #486

Summary

Crow's bundled crow-tmux.conf keeps mouse on so wheel scrollback works through the cockpit (#452). The side effect is that tmux's default WheelUpPane puts the pane into copy-mode, where paste-buffer does not deliver content into the underlying shell. Programmatic sends — Manager paste, auto-respond prompts, quick actions — silently fail whenever the user happened to scroll first.

This PR fixes the send path: pre-cancel copy-mode before paste-buffer via tmux if-shell -F '#{pane_in_mode}' 'send-keys -X cancel'. The if-shell guard keeps it a no-op in the common case (pane not in a mode).

Changes

• TmuxController.cancelCopyModeIfActive(target:) — new helper wrapping the if-shell guard so the intent reads locally.
• TmuxBackend.sendText — calls the helper after loadBufferFromStdin and before pasteBuffer. Doc comment extended to document why.
• crow-tmux.conf — explicit WheelUpPane rebind using copy-mode -e. Matches tmux ≥3.0 defaults but pinning it insulates Crow from upstream changes and minimizes time-in-copy-mode (the -e auto-exits when wheel-down scrolls past the bottom of history).
• Tests — two new real-tmux integration tests in TmuxBackendTests:
  • sendTextCancelsCopyModeBeforePaste forces the pane into copy-mode via a side-channel controller, then asserts pane_in_mode flips back to 0 after sendText.
  • sendTextOnNormalPaneIsUnaffectedByCancelGuard sanity-checks the if-shell no-op path so a normal-state pane doesn't fail.

Deferred (intentional)

The keystroke-trap leg of #486 — auto-exit copy-mode on a printable keyDown in GhosttySurfaceView — is not addressed here. It needs a tmux control-mode %mode-changed parser the codebase doesn't have yet; building that infrastructure is orthogonal to the paste fix this PR closes. I'll file a follow-up issue against this PR.

The user-typing trap is still present in this PR (scroll up, then start typing → keystrokes eaten until Escape). The Manager-paste / auto-respond / quick-action paste bug — the symptom the ticket title actually names — is fixed.

Test plan

• make ghostty then arch -arm64 swift test --package-path Packages/CrowTerminal → all 63 tests in 9 suites pass.
• Manual: launch Crow Manager, mouse-wheel-up to enter copy-mode visually, run crow send --session 00000000-0000-0000-0000-000000000000 --terminal <uuid> "test\n" from another shell — text lands in the prompt and submits.
• Manual: trigger an auto-respond or Refine quick action into a scrolled-up working session — prompt arrives.
• Regression: wheel scrollback, drag-select + copy-pipe-no-clear, double/triple-click word/line copy still work as defined by Ghostty terminal: text selection clears on mouse-up, blocking copy/paste #445 / Replace 'mouse off' with 'mouse on + copy-pipe-no-clear' in crow-tmux.conf (#446 broke wheel scrollback) #452.

🐦‍⬛ Generated with Claude Code, orchestrated by Crow

[dhilgaertner]

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

• No injection surface in the new if-shell command. target is built only from TmuxBackend.cockpitSessionName (a constant) and an integer windowIndex; even though it's string-interpolated into the send-keys -t <target> -X cancel argument, there is no attacker-controlled input.
• The if-shell -F '#{pane_in_mode}' guard is semantically correct: with -F, tmux treats "0" (the not-in-mode value) as false, so the send-keys -X cancel only fires when the pane is actually in a mode — avoiding the send-keys -X "not in a mode" error in the common path.
• crow-tmux.conf WheelUpPane rebind faithfully mirrors the tmux ≥3.0 default and the existing DoubleClick1Pane/TripleClick1Pane brace-block pattern already in the file (copy-mode -et= == copy-mode -e -t =, consistent with the file's -t = convention).

Concerns:

• None.

Code Quality

Yellow — copy-mode cancel is skipped on the bare-Enter (Enter-only) send path (TmuxBackend.swift:340-358)

cancelCopyModeIfActive(target:) is called inside the if !payload.isEmpty { ... } block. When sendText is invoked with text == \"\n\" (payload empties to "", didPaste stays false), the cancel is skipped and the Enter is delivered via ctrl.sendKeys(target:, keys: [\"Enter\"]). A plain send-keys Enter into a pane that's in copy-mode is routed through the copy-mode key table (default emacs: copy-selection-and-cancel), so it exits copy-mode but never delivers a carriage return to the underlying shell — the same silent-drop failure mode this PR fixes for paste.

This contradicts the new doc comment's claim that "every programmatic send into a pane the user has scrolled ... is silently dropped" is fixed — the Enter-only submit path still is. crow send --terminal <id> \"\n\" (and TmuxBackendTests.swift:225 exercises exactly this bare-\n shape) hits it. Auto-respond / quick actions append text+\n (non-empty payload), so they're covered; pure-Enter submits are not.

Recommendation: hoist cancelCopyModeIfActive(target:) above the paste/Enter split so it runs for any delivery into the pane (it's one cheap if-shell call), or duplicate it before the sendKeys([\"Enter\"]). Then the doc comment's contract holds for all send shapes.

Green — cosmetic: the new WheelUpPane line spells out if-shell while the neighboring MouseDown1Pane/DoubleClick1Pane bindings use the if alias. Harmless; consider matching the surrounding style for grep-consistency.

Tests

• sendTextCancelsCopyModeBeforePaste and sendTextOnNormalPaneIsUnaffectedByCancelGuard are well-scoped real-tmux integration tests covering the in-mode and no-op-guard paths. A test for the bare-Enter-into-copy-mode case would have caught the gap above.
• I did not re-run the suite: a local build needs the GhosttyKit xcframework (make ghostty) and the integration tests require a real tmux. Relying on the PR's stated "all 63 tests pass" for the green run.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

Recommendation: Request Changes — driven by [0 Red, 1 Yellow, 1 Green] findings. The core paste fix is correct and well-tested; addressing the Enter-only path (or narrowing the doc comment's claim) closes it out in this round trip.

🐦‍⬛ Reviewed by Crow via Claude Code

[dhilgaertner]

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

• No command-injection surface. TmuxController.cancelCopyModeIfActive interpolates only target, which is derived from the constant cockpitSessionName ("crow-cockpit") and an integer windowIndex — never from user input. The user's text reaches tmux via loadBufferFromStdin (stdin) and paste-buffer, not via argv, so the payload can't influence command parsing.
• The if-shell -F '#{pane_in_mode}' guard means send-keys -X cancel only fires when the pane is actually in a mode, avoiding the error tmux raises otherwise.

Code Quality

• Correct hoisting of the cancel (TmuxBackend.swift:347). Placing cancelCopyModeIfActive above the if !payload.isEmpty / endsWithNewline split is the key correctness point: it covers the bare-\n path where send-keys Enter would otherwise be eaten by the copy-mode key table (emacs copy-selection-and-cancel / vi cancel) and never reach the shell. The new sendTextBareEnterCancelsCopyMode test locks this in.
• Ordering is safe. Each ctrl.run(...) is a separate synchronous tmux client invocation, so the server has processed the cancel before paste-buffer is issued — no race between cancel and paste.
• crow-tmux.conf WheelUpPane rebind mirrors the existing MouseDown/DoubleClick/TripleClick binding structure and matches the tmux ≥3.0 default; pinning it is reasonable insurance. Note it only takes effect for tmux servers started after this change (config is read at server start / reload), which is expected behavior, not a regression.
• Doc comments are thorough and accurate about the failure mode and the fix.

Green (consider only):

• sendTextCancelsCopyModeBeforePaste asserts pane_in_mode flips back to 0, which proves the cancel ran, but doesn't assert the pasted token actually landed in the pane (e.g. via a capture-pane check). The cancel behavior is the regression being fixed, so this is fine; an end-to-end content assertion would just make the test name's "...BeforePaste" promise airtight. Covered by the manual test plan.
• Minor: the inline comment block at TmuxBackend.sendText (lines ~341-346) has an unbalanced (where ... parenthesis. Cosmetic.
• The keystroke-trap leg of #486 (auto-exit copy-mode on printable keyDown) is intentionally deferred with a clear rationale and a planned follow-up. The paste/auto-respond/quick-action symptom named in the ticket is fixed here.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

Recommendation: Approve — driven by [0 Red, 0 Yellow, 3 Green] findings. Tight, well-tested, well-documented fix with no security or correctness blockers.

Note: I could not run the build/tests locally because GhosttyKit.xcframework is not present in this checkout (requires make ghostty); the review is by inspection. The PR reports all 63 tests passing.

---

🐦‍⬛ Reviewed by Crow via Claude Code