From 92967e97a0814882370ece9bff7d30c7f52e0baa Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 16 Jul 2026 14:24:39 +0000 Subject: [PATCH] fix(tui): preserved plan review text selection Decoupled fullscreen alternate-screen rendering from terminal mouse capture. Disabled pointer tracking for Plan Review so terminals retain native selection. Fixes #5711 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../src/modes/interactive-mode.ts | 1 + .../test/interactive-mode-plan-review.test.ts | 26 ++++++++++- packages/tui/CHANGELOG.md | 4 ++ packages/tui/src/tui.ts | 46 +++++++++++-------- packages/tui/test/render-regressions.test.ts | 42 +++++++++++++++++ 6 files changed, 101 insertions(+), 22 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0e753ced7..1cfa0c844 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Plan Review capturing mouse drags as pointer events, preventing native terminal text selection ([#5711](https://github.com/can1357/oh-my-pi/issues/5711)). + ## [17.0.1] - 2026-07-16 ### Changed diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 27bb59275..159a7c1f0 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -2551,6 +2551,7 @@ export class InteractiveMode implements InteractiveModeContext { maxHeight: "100%", margin: 0, fullscreen: true, + mouseTracking: false, }); this.ui.setFocus(overlay); this.ui.requestRender(); diff --git a/packages/coding-agent/test/interactive-mode-plan-review.test.ts b/packages/coding-agent/test/interactive-mode-plan-review.test.ts index fb50cdcca..55efffa73 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -10,7 +10,7 @@ import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config import { resolveLocalUrlToPath } from "@oh-my-pi/pi-coding-agent/internal-urls"; import { AssistantMessageComponent } from "@oh-my-pi/pi-coding-agent/modes/components/assistant-message"; import type { HookSelectorSlider } from "@oh-my-pi/pi-coding-agent/modes/components/hook-selector"; -import type { PlanReviewOverlay } from "@oh-my-pi/pi-coding-agent/modes/components/plan-review-overlay"; +import { PlanReviewOverlay } from "@oh-my-pi/pi-coding-agent/modes/components/plan-review-overlay"; import { InteractiveMode } from "@oh-my-pi/pi-coding-agent/modes/interactive-mode"; import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; @@ -19,7 +19,7 @@ import { SILENT_ABORT_MARKER, USER_INTERRUPT_LABEL } from "@oh-my-pi/pi-coding-a import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { AUTO_THINKING } from "@oh-my-pi/pi-coding-agent/thinking"; import * as clipboard from "@oh-my-pi/pi-coding-agent/utils/clipboard"; -import { setKeybindings, Text } from "@oh-my-pi/pi-tui"; +import { type OverlayHandle, type OverlayOptions, setKeybindings, Text } from "@oh-my-pi/pi-tui"; import { formatNumber, TempDir } from "@oh-my-pi/pi-utils"; /** @@ -333,6 +333,28 @@ describe("InteractiveMode plan review rendering", () => { } }); + it("leaves terminal mouse tracking disabled while Plan Review is open", async () => { + let capturedOverlay: PlanReviewOverlay | undefined; + let capturedOptions: OverlayOptions | undefined; + const overlayHandle: OverlayHandle = { + hide: vi.fn(), + setHidden: vi.fn(), + isHidden: vi.fn(() => false), + }; + vi.spyOn(mode.ui, "showOverlay").mockImplementation((component, options) => { + if (!(component instanceof PlanReviewOverlay)) throw new Error("Expected Plan Review overlay"); + capturedOverlay = component; + capturedOptions = options; + return overlayHandle; + }); + + const choice = mode.showPlanReview("# Plan\n\nSelectable body", "Plan mode - next step", ["Approve"]); + + expect(capturedOptions).toMatchObject({ fullscreen: true, mouseTracking: false }); + capturedOverlay?.handleInput("\x1b"); + await expect(choice).resolves.toBeUndefined(); + }); + it("copies the overlay's current edited plan markdown from the real plan review overlay", async () => { let capturedOverlay: PlanReviewOverlay | undefined; const overlayHandle = { hide: vi.fn() }; diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 2b3c229e7..307bd4e88 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added a fullscreen overlay mouse-tracking opt-out so selection-first dialogs can preserve native terminal text selection ([#5711](https://github.com/can1357/oh-my-pi/issues/5711)). + ## [17.0.1] - 2026-07-16 ### Added diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index b938aa2d9..3ebf6e2a7 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -80,11 +80,11 @@ const CURSOR_BEGIN = `${HIDE_CURSOR}${SYNC_OUTPUT_BEGIN}`; const CURSOR_BEGIN_NO_SYNC = HIDE_CURSOR; const CURSOR_END = SYNC_OUTPUT_END; const CURSOR_END_NO_SYNC = ""; -// Mouse reporting, enabled only for the lifetime of a fullscreen overlay so the -// rest of the app keeps the terminal's native text selection. 1000h = button -// click tracking, 1003h = any-motion tracking so overlays can light up hover -// targets (the pointer moving with no button held), 1006h = SGR extended -// coordinates so columns/rows past 223 are reported. +// Mouse reporting is scoped to fullscreen overlays that opt into pointer +// interaction. 1000h = button click tracking, 1003h = any-motion tracking for +// hover targets, and 1006h = SGR extended coordinates past column/row 223. +// Selection-first overlays leave these modes disabled so the terminal retains +// native text selection. const MOUSE_TRACKING_ON = "\x1b[?1000h\x1b[?1003h\x1b[?1006h"; const MOUSE_TRACKING_OFF = "\x1b[?1006l\x1b[?1003l\x1b[?1000l"; @@ -454,6 +454,11 @@ export interface OverlayOptions { * unchanged and still draw over the transcript on the normal screen. */ fullscreen?: boolean; + /** + * Enable terminal mouse reporting while fullscreen. Defaults on; disable it + * when native terminal text selection takes precedence over pointer events. + */ + mouseTracking?: boolean; } /** @@ -1086,6 +1091,7 @@ export class TUI extends Container { // untouched, so exiting reconciles cleanly against the terminal-restored // normal screen. #altPreviousLines is the last alt frame, for repaint-skip. #altActive = false; + #altMouseTrackingActive = false; #altPreviousLines: string[] = []; #altEnterWidth = 0; #altEnterHeight = 0; @@ -1742,11 +1748,12 @@ export class TUI extends Container { stop(): void { if (this.#altActive || this.#pendingAltExit) { - const exitSequence = - this.#pendingAltExit || `${MOUSE_TRACKING_OFF}${this.#keyboardEnhancementExit()}\x1b[?1049l`; + const mouseExit = this.#altMouseTrackingActive ? MOUSE_TRACKING_OFF : ""; + const exitSequence = this.#pendingAltExit || `${mouseExit}${this.#keyboardEnhancementExit()}\x1b[?1049l`; this.terminal.write(exitSequence); setAltScreenActive(false); this.#altActive = false; + this.#altMouseTrackingActive = false; this.#altPreviousLines = []; this.#pendingAltExit = ""; } @@ -2705,24 +2712,29 @@ export class TUI extends Container { // requests it, borrow the terminal's alternate buffer and paint only the // modal there; the normal screen and all accounting stay untouched. let deferredAltExit = this.#pendingAltExit; - const wantAlt = this.#wantsAltScreen(); + const topOverlay = this.#getTopmostVisibleOverlay(); + const wantAlt = topOverlay?.options?.fullscreen === true; + const wantMouseTracking = wantAlt && topOverlay.options?.mouseTracking !== false; if (wantAlt && !this.#altActive) { // Enhanced keyboard modes can be buffer-local: re-push the active // modified-key reporting sequence on the freshly entered alternate // screen, or Esc/modified keys revert to legacy encoding inside // fullscreen overlays (Ghostty/kitty/iTerm2). - this.terminal.write(`\x1b[?1049h${this.#keyboardEnhancementEnter()}${MOUSE_TRACKING_ON}`); + const mouseEnter = wantMouseTracking ? MOUSE_TRACKING_ON : ""; + this.terminal.write(`\x1b[?1049h${this.#keyboardEnhancementEnter()}${mouseEnter}`); setAltScreenActive(true); this.terminal.hideCursor(); this.#forgetHardwareCursorState(); this.#recordHardwareCursorHidden(); this.#altActive = true; + this.#altMouseTrackingActive = wantMouseTracking; this.#altPreviousLines = []; this.#altEnterWidth = width; this.#altEnterHeight = height; } else if (!wantAlt && this.#altActive) { + const mouseExit = this.#altMouseTrackingActive ? MOUSE_TRACKING_OFF : ""; const enhancementExit = this.#keyboardEnhancementExit(); - const exitSequence = `${MOUSE_TRACKING_OFF}${enhancementExit}\x1b[?1049l`; + const exitSequence = `${mouseExit}${enhancementExit}\x1b[?1049l`; // Session replacement can finish while a fullscreen selector is still // covering the old normal buffer. Keep the overlay visible until the // replacement is ready, then fuse the buffer restore into that full paint; @@ -2734,6 +2746,7 @@ export class TUI extends Container { setAltScreenActive(false); this.#forgetHardwareCursorState(); this.#altActive = false; + this.#altMouseTrackingActive = false; this.#altPreviousLines = []; // A resize while on the alt buffer reflowed the terminal's saved // normal screen; it no longer matches our accounting, so force the @@ -2741,6 +2754,9 @@ export class TUI extends Container { if (width !== this.#altEnterWidth || height !== this.#altEnterHeight) { this.#resizeEventPending = true; } + } else if (wantMouseTracking !== this.#altMouseTrackingActive) { + this.terminal.write(wantMouseTracking ? MOUSE_TRACKING_ON : MOUSE_TRACKING_OFF); + this.#altMouseTrackingActive = wantMouseTracking; } if (this.#altActive) { this.#componentRenderTargets.clear(); @@ -3701,16 +3717,6 @@ export class TUI extends Container { this.terminal.write(buffer); } - /** Topmost visible overlay requests the alternate-screen buffer. */ - #wantsAltScreen(): boolean { - for (let i = this.overlayStack.length - 1; i >= 0; i--) { - const entry = this.overlayStack[i]!; - if (!this.#isOverlayVisible(entry)) continue; - return entry.options?.fullscreen === true; - } - return false; - } - /** * Compose and paint a single fullscreen overlay frame on the alt buffer. * Cursor markers are stripped (the modal draws its own in-band caret and diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index ffa2d5da1..364b1b660 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -3310,6 +3310,48 @@ describe("TUI terminal-state regressions", () => { } }); + it("leaves native text selection available for selection-first fullscreen overlays", async () => { + const term = new VirtualTerminal(40, 8, 200); + const writes = captureWrites(term); + const tui = new TUI(term); + tui.addChild(new MutableLinesComponent(rows("base-", 8))); + + try { + tui.start(); + await settle(term); + + const showFrom = writes.length; + const handle = tui.showOverlay(new MutableLinesComponent(["SELECTABLE PLAN TEXT"]), { + anchor: "bottom-center", + width: "100%", + maxHeight: "100%", + margin: 0, + fullscreen: true, + mouseTracking: false, + }); + await settle(term); + + const modalWrites = writes.slice(showFrom).join(""); + expect(modalWrites).toContain("\x1b[?1049h"); + expect(modalWrites).not.toContain("\x1b[?1000h"); + expect(modalWrites).not.toContain("\x1b[?1003h"); + expect(modalWrites).not.toContain("\x1b[?1006h"); + expect(visible(term).some(line => line.includes("SELECTABLE PLAN TEXT"))).toBeTrue(); + + const hideFrom = writes.length; + handle.hide(); + await settle(term); + + const hideWrites = writes.slice(hideFrom).join(""); + expect(hideWrites).toContain("\x1b[?1049l"); + expect(hideWrites).not.toContain("\x1b[?1000l"); + expect(hideWrites).not.toContain("\x1b[?1003l"); + expect(hideWrites).not.toContain("\x1b[?1006l"); + } finally { + tui.stop(); + } + }); + it("falls back to kittyEnableSequence for legacy custom terminals", async () => { const term = new LegacyKeyboardVirtualTerminal(40, 8, 200); const writes = captureWrites(term);