fix(ui): treat Cmd/Ctrl-held mouse events as BossTerm UI, not TUI

[kshivang]

Summary

Sibling fix to #260 — same file, same mouse-reporting early-return pattern, different handlers.

In a pane running a TUI that enables mouse reporting (claude, vim with set mouse=a, htop, nvim, …):

• Cmd-hover on a URL did not draw the underline. Move handler short-circuits to the TUI forward at ProperTerminal.kt:1142, so hoveredHyperlink never gets set — and the renderer at TerminalCanvasRenderer.kt:833 requires hoveredHyperlink != null && isModifierPressed.
• Cmd-click on a URL did not open it via BossTerm. Press handler at :808 forwards the click to the TUI, so the Cmd-click branch at :1042 is never reached. Some TUIs implement their own URL opener on the forwarded click; many — including some Claude Code surfaces — do not.

Fix mirrors the existing shift bypass for text selection: when the user is holding Cmd/Ctrl (tracked via isModifierPressed, updated by the .onPreviewKeyEvent at :1382-1395), skip the mouse-reporting forward in Press/Move/Release so the hyperlink hover/click handlers run locally. Scroll keeps current forwarding so Ctrl+wheel (TUI font-zoom, pager input) still works.

Test plan

• ./gradlew :compose-ui:compileKotlinDesktop — clean.
• ./gradlew :embedded-example:run → run claude → Cmd-hover a URL → underline appears → Cmd-click → browser opens.
• Plain shell in same pane: Cmd-hover/click on URL still works (regression check).
• Tabbed mode (:bossterm-app:run): vim with set mouse=a, hover/click URLs in a buffer — same behavior.
• Non-link Cmd-click in mouse-reporting TUI: doesn't reach the TUI (acceptable — Cmd isn't normally a TUI mouse modifier; this is symmetric with how shift is already treated).
• Ctrl+wheel still reaches the TUI (no change to Scroll handler).

Note

Lightly depends on #260's focus-restore for the keyboard-canvas-focused state that updates isModifierPressed. If #260 lands first, this is mergeable as-is. If this lands first, both still work because clicking into a TUI pane will focus the canvas via the existing (un-fixed) path most of the time.

Generated with Claude Code

[claude]

Review

Small, focused fix that mirrors the existing shift-bypass pattern for hyperlink interaction in mouse-reporting TUIs. The intent is right and the comments do a good job of explaining the why and pointing at the symmetric handlers. A few things worth considering before merging.

1. isModifierPressed is keyboard-focus-dependent — a PointerEvent read would be more robust

isModifierPressed is updated only by .onPreviewKeyEvent at ProperTerminal.kt:1396-1409, so it only reflects reality while the terminal canvas has keyboard focus. The PR description already calls this out as a soft dependency on #260's focus-restore. But the codebase already has a more robust way to read Cmd/Ctrl that doesn't depend on focus at all — PointerEvent.toMouseModifierFlags() in ComposeMouseEvent.kt:48-64 reads awtEvent.isMetaDown / awtEvent.isControlDown straight off the native AWT event, the same way isShiftPressed() already works.

Concretely you could add an isCtrlOrMetaPressed() extension next to isShiftPressed() and use that in the three guards:

fun PointerEvent.isCtrlOrMetaPressed(): Boolean {
    val flags = toMouseModifierFlags()
    return (flags and MouseButtonModifierFlags.MOUSE_BUTTON_CTRL_FLAG) != 0 ||
           (flags and MouseButtonModifierFlags.MOUSE_BUTTON_META_FLAG) != 0
}

Benefits:

• Works regardless of whether the canvas had keyboard focus when Cmd was pressed (alt-tab back to BossTerm with Cmd held, click into a sibling Compose element then back, embedder-hosted scenarios, etc.).
• Removes the soft dependency on fix(ui): restore terminal focus on click into mouse-reporting TUIs #260 entirely.
• Symmetric with the existing shiftPressed = event.isShiftPressed() pattern one line above each guard, instead of mixing two different sources of truth for "is modifier held."

isModifierPressed would still be the right source for the hyperlink-click branch at :1047 and the renderer prop at :1637, since those run inside the canvas-focused path. But for the mouse-reporting bypass gates, the pointer event is closer to ground truth.

2. Press/Release pairing edge case

The Release-handler comment says the bypass exists "so the TUI never sees an unpaired button-up after a Cmd+click we handled locally" — but the symmetric failure can still happen if the user's modifier state changes between press and release:

• Press without Cmd → release with Cmd: TUI sees press, bypass swallows release → TUI is stuck thinking the button is held.
• Press with Cmd → release without Cmd: bypass swallows press, TUI sees release → TUI gets an unpaired button-up.

Reading the modifier from the pointer event (point 1) narrows this somewhat — AWT keeps isMetaDown/isControlDown accurate per-event — but the underlying issue is using a current modifier read on release to decide what to do about a past press. A more durable fix would be to remember on press whether the event was forwarded (e.g. lastPressWasForwarded: Boolean) and mirror that exact decision on release. Probably not worth blocking on for this PR since the window where a user toggles Cmd mid-click is narrow, but worth a comment noting it as a known limitation (or a short follow-up).

3. Minor: duplicated guard

The condition settings.enableMouseReporting && isRemoteMouseAction(shiftPressed) && !isModifierPressed (plus the event.button != PointerButton.Secondary term in Press) now appears in three handlers. Pulling it into a shouldForwardMouseToTui(event, shiftPressed, excludeSecondary: Boolean = false) helper would make future changes (like adding a fourth modifier exemption) less error-prone. Optional.

Other notes

• Scope: correctly leaves Scroll untouched — Ctrl+wheel for TUI font-zoom / pager input continues to work. Good call-out in the comment at :1151-1152.
• Style: comments are well-written, conventionally placed, and explain why not what — consistent with CLAUDE.md guidance.
• Tests: no automated coverage added, but the existing handlers in this file aren't unit-tested either, and the manual test plan is comprehensive. Worth checking the un-ticked items (plain shell regression, tabbed mode, Ctrl+wheel) before merge.
• Security / perf: nothing of concern. Three extra boolean reads on the hot path; negligible.

TL;DR

Fix is correct and behaves as advertised for the common case. The main improvement to consider is reading the Cmd/Ctrl state from the PointerEvent (via the existing AWT-backed helper) instead of the keyboard-tracked isModifierPressed — that removes the focus dependency, makes this PR independent of #260, and partially mitigates the press/release pairing edge case.

[claude]

Code review

Solid, focused fix that mirrors the existing shift bypass and tracks Press/Release pairing correctly. The two pointer-state invariants here — (a) don't strand half a sequence at the TUI when modifiers toggle, (b) don't double-forward — both look right. A few notes:

Press/Release pairing — looks correct

• lastPressForwardedToTui is reset eagerly on every Press edge (line 838) before the forwarding check, so a stale true from a missed Release can't leak into a later press. Good defensive design.
• Right-click + null-button paths never set the flag to true, so Release won't try to forward what Press didn't. Consistent.
• Drag mirrors the Press decision via lastPressForwardedToTui, so toggling Cmd mid-drag stays consistent with how the drag started. The comment at ProperTerminal.kt:1166-1169 calls this out explicitly — appreciated.

Potential concerns

1. Mode change between Press and Release. Release still gates on isRemoteMouseAction(shiftPressed) in addition to lastPressForwardedToTui. If the TUI disables mouse reporting between a forwarded Press and its Release (rare but possible — e.g., the TUI exits or toggles set mouse=), isRemoteMouseAction returns false and the Release won't be forwarded, stranding a button-down at the TUI. This isn't introduced by this PR (pre-existing behavior with the prior unconditional check), but the new lastPressForwardedToTui gives you a cleaner signal that could subsume the mode check on Release. Consider: if (lastPressForwardedToTui) alone on Release would be more robust. Not blocking — fine to leave as-is.

2. Hover state staleness when Cmd is released mid-hover. When user hovers with Cmd held, hoveredHyperlink is set. If they release Cmd while still hovering the same cell, the next Move event reads isCtrlOrMetaPressed() == false and forwards to the TUI early-return, so the hover-detection block at :1199-1225 doesn't run. hoveredHyperlink stays set. The renderer at TerminalCanvasRenderer.kt:833 gates the underline on isModifierPressed, so visually the underline disappears — correct. But the hoverConsumers exit callback (:1208-1210) won't fire until the user moves to a non-link cell (or back to a TUI-forwarded path on a non-link). If any consumer cares about Cmd-state changes (e.g., a tooltip), it would feel stale. Probably not a real bug today, just worth noting.

3. isCtrlOrMetaPressed() helper. Cleanly defined, doc comment is excellent (calls out the focus-independence vs. isModifierPressed). One nit: the implementation calls toMouseModifierFlags() once and bit-ands twice — could short-circuit on Ctrl first to skip the second bitmask, but the perf delta is invisible. Fine as is.

Style / conventions

• Comments are verbose but explain why, which matches CLAUDE.md guidance ("Only add one when the WHY is non-obvious") and the existing density in this file. No complaint.
• The reference "open via line ~1042" in the Press-handler comment (:828) is actually around :1060 now. Line-number refs in comments rot — consider naming the handler conceptually ("the Cmd-click branch below") instead of pinning a number. Minor.

Performance

No concern. One extra modifier check per pointer event, all from already-extracted AWT modifier flags.

Security

N/A — pure UI input routing, no new external surface.

Tests

No automated tests added, which is reasonable given how hard Compose pointer-event flows are to unit-test. The manual test plan in the PR description is well-structured; the unchecked regression items (plain shell URL click, tabbed mode + vim, non-link Cmd-click in TUI, Ctrl+wheel) are the right things to verify before merge.

Summary

LGTM. The Press/Release pairing logic is the kind of detail that's easy to get wrong and you got it right on the first cut. The two questions above (mode-change race, hover-consumer staleness) are edge cases worth thinking about but don't block this PR.

[claude]

Code review — PR #261

Solid, well-scoped fix. The pattern mirrors the existing shift-bypass nicely, and the lastPressForwardedToTui latch is the right shape for keeping Press/Release paired across mid-gesture modifier toggles. Comments are unusually good — they explain why, not just what. Compiles clean per the test plan.

A few things worth a closer look before merging.

1. Click handler still uses the focus-gated flag — Cmd-click from an unfocused canvas can no-op

The Press bypass at ProperTerminal.kt:839 correctly uses event.isCtrlOrMetaPressed() (AWT-read, focus-independent), so the TUI no longer eats the click. But the actual URL-open branch at :1060 is unchanged:

if (hoveredHyperlink != null && isModifierPressed) {

isModifierPressed is only updated while the canvas has keyboard focus (the onPreviewKeyEvent at :1429). When a user clicks a hyperlink in a TUI pane while focus lives on a sibling element (sidebar Button, search bar, another pane), the sequence is:

• Move handler bypassed forwarding via event.isCtrlOrMetaPressed() → hoveredHyperlink is set ✓
• Press handler bypassed forwarding via cmdOrCtrlHeld → TUI doesn't get the click ✓
• Line 1060 check: isModifierPressed is still false → URL doesn't open ✗

The press-time scope.launch { delay(50); focusRequester.requestFocus() } from #260 doesn't save this case — it runs after the synchronous click handler. Net effect: first Cmd-click from an unfocused state silently swallows the click (neither TUI nor browser receives it). Second click works because the canvas is now focused.

Suggest using the focus-independent read here too — cmdOrCtrlHeld is already in scope from line 835:

if (hoveredHyperlink != null && (isModifierPressed || cmdOrCtrlHeld)) {

Or simpler: just cmdOrCtrlHeld, since that's what the press bypass already trusts.

2. Same focus-gating problem affects the hover underline

The Move handler now sets hoveredHyperlink regardless of canvas focus, but the renderer underline at TerminalCanvasRenderer.kt:833 still gates on isModifierPressed. So Cmd-hover from an unfocused canvas updates the internal state but doesn't draw the underline — the user gets no visual feedback that the click will work (and per concern #1, the first click may not work anyway).

Plumbing the modifier from the pointer event into the render context (or OR-ing in cmdOrCtrlHeld somewhere visible to render) would close this. Optional — the PR description does acknowledge this dependency on focus-restore behavior.

3. Race when modifier read on Release diverges from Press

The lastPressForwardedToTui latch is the right call here, and the comment at :1328 explains exactly why. One edge to double-check: if the canvas unmounts/recomposes between Press and Release (e.g., pane closes, tab switches mid-click), lastPressForwardedToTui is destroyed with the state. Probably benign — no Release fires either — but worth confirming there's no path where a Release surfaces on a re-created canvas with lastPressForwardedToTui=false and the TUI ends up with an orphan button-down. Skim of the relevant lifecycle suggests it's fine, but a comment noting the assumption would help future readers.

4. Minor

• :828 references "open via line ~1042" — the actual hyperlink-click branch is at :1060 in this diff. The ~ softens it, but line-number references in comments rot fast. Consider replacing with the symbol name (HyperlinkDetector.openUrl branch) or just dropping the line ref.
• The new isCtrlOrMetaPressed() helper in ComposeMouseEvent.kt has no test, consistent with isShiftPressed()/isAltPressed(). Not blocking, but a small unit test against a fake PointerEvent would be cheap insurance and protect against future regressions if the bit math changes.
• Naming nit: isCtrlOrMetaPressed reads ambiguously on macOS (where Ctrl and Meta are very different keys). isHyperlinkModifierPressed or isCmdOrCtrlPressed would match the call-site intent better. The KDoc clarifies, so this is genuinely just a nit.

Security / performance

Nothing concerning. The added work per pointer event is one extra bit-AND read; modifier flags are already computed. No new allocations, no I/O, no untrusted-input handling changes. HyperlinkDetector.openUrl is unchanged.

Test coverage

No automated coverage added or available for this area (the file is composable-heavy and existing mouse-handler logic is untested by convention). The manual test plan in the PR description covers the critical paths; would push the unchecked plain-shell and vim mouse=a cases over the line before merging, since they're the regression-risk cases (#1 above is most likely to surface in plain-shell Cmd-click from sidebar focus).

---

Overall, ship-worthy after concern #1 is addressed — it's the same class of bug as the one being fixed (modifier-aware code path silently no-ops). Concerns #2 and #3 are good-to-haves.