From e5e29219fe968ab885c75ceaddd87de519bd0eb5 Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Wed, 24 Jun 2026 12:54:15 -0700 Subject: [PATCH 1/4] feat(tui): add shared SGR mouse-routing helpers and SelectList.routeMouse Add routeSgrMouseInput() and routeSelectListMouse() to centralize the repeated SGR parse + SelectList hit-test/hover/wheel/click patterns, and make SelectList implement MouseRoutable via routeMouse(). The structural SelectListMouseTarget interface keeps mouse.ts free of component imports. Extend the emergency-restore tests to assert mouse-off sequences (1006l/1003l/1000l) are emitted while keeping the alt-screen 1049 gating. --- packages/tui/CHANGELOG.md | 3 + packages/tui/src/components/select-list.ts | 7 +- packages/tui/src/mouse.ts | 50 ++++++++++ .../test/emergency-restore-altscreen.test.ts | 21 ++++- packages/tui/test/mouse.test.ts | 92 ++++++++++++++++++- packages/tui/test/select-list.test.ts | 65 +++++++++++++ 6 files changed, 233 insertions(+), 5 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index eb30b7f9c..94b62433f 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -15,6 +15,9 @@ - Fixed `@`-path autocomplete failing on Windows for paths outside the cwd. Windows absolute paths (e.g. `C:\\Users\\...`) were not detected as absolute — only `/` was checked — so they were incorrectly joined with the base directory, producing invalid search paths and empty suggestions. Path-join calls also introduced backslashes into suggestion values, breaking round-trip insertion. Absolute path detection now uses `path.isAbsolute()` (handles drive letters) and suggestion paths are normalized to forward slashes (valid on all platforms). - Fixed settings rows crashing native text truncation when a malformed config value reaches the renderer as a non-string ([#3338](https://github.com/can1357/oh-my-pi/issues/3338)). - Fixed desktop notifications being silently lost under tmux on the common stack of tmux + kitty/ghostty/wezterm/iTerm2. `TERMINAL_ID` resolves to the inner terminal (whose markers leak into the tmux session env), which maps to `NotifyProtocol.Osc9` / `NotifyProtocol.Osc99`, and `sendNotification()` wrote that raw OSC straight to stdout — tmux dropped it on the floor and `monitor-bell` / `monitor-activity` never fired, so a backgrounded omp pane had no way to flag completion or `ask` blockage. Under `TMUX`, OSC-protocol notifications are now wrapped in tmux's `\x1bPtmux;…\x1b\\` DCS passthrough envelope (so users with `set -g allow-passthrough on` still get the real toast on the outer terminal) and followed by a `\x07` BEL (so `set -g monitor-bell on` reliably flags the window otherwise). The OSC 99 capability probe in `terminal.ts` is wrapped the same way so rich notifications keep working across tmux. `NotifyProtocol.Bell` paths are unchanged. ([#3395](https://github.com/can1357/oh-my-pi/issues/3395)) +### Added + +- Added shared SGR mouse input routing helpers and `SelectList.routeMouse()` support for fullscreen overlay hit-testing. ## [16.1.10] - 2026-06-21 diff --git a/packages/tui/src/components/select-list.ts b/packages/tui/src/components/select-list.ts index 10aff5309..06b151a42 100644 --- a/packages/tui/src/components/select-list.ts +++ b/packages/tui/src/components/select-list.ts @@ -2,6 +2,7 @@ import { popLoopPhase, pushLoopPhase } from "@oh-my-pi/pi-utils"; import { fuzzyFilter } from "../fuzzy"; import { getKeybindings } from "../keybindings"; import { extractPrintableText } from "../keys"; +import { type MouseRoutable, routeSelectListMouse, type SgrMouseEvent } from "../mouse"; import type { SymbolTheme } from "../symbols"; import type { Component } from "../tui"; import { Ellipsis, padding, replaceTabs, truncateToWidth, visibleWidth, wrapTextWithAnsi } from "../utils"; @@ -80,7 +81,7 @@ type SelectItemLayout = spacing: ""; }; -export class SelectList implements Component { +export class SelectList implements Component, MouseRoutable { #filteredItems: ReadonlyArray; #filterQuery = ""; #selectedIndex: number = 0; @@ -139,6 +140,10 @@ export class SelectList implements Component { this.onSelect?.(item); } + routeMouse(event: SgrMouseEvent, line: number, _col: number): void { + routeSelectListMouse(this, event, line); + } + invalidate(): void { // No cached state to invalidate currently } diff --git a/packages/tui/src/mouse.ts b/packages/tui/src/mouse.ts index e3b43bef5..c2efa8df2 100644 --- a/packages/tui/src/mouse.ts +++ b/packages/tui/src/mouse.ts @@ -44,6 +44,56 @@ export function parseSgrMouse(data: string): SgrMouseEvent | null { return { button, col, row, release, wheel, motion, leftClick }; } +/** Handler invoked with a decoded SGR event; returning `false` reports unhandled. */ +export type SgrMouseHandler = (event: SgrMouseEvent) => boolean | undefined; + +/** + * Decode an SGR mouse report and forward it to `handler`. Returns `false` when + * `data` is not an SGR mouse report (or fails to parse), so callers can fall + * through to other input handling. Centralizes the repeated + * `data.startsWith("\x1b[<")` + `parseSgrMouse()` pattern. + */ +export function routeSgrMouseInput(data: string, handler: SgrMouseHandler): boolean { + if (!data.startsWith("\x1b[<")) return false; + const event = parseSgrMouse(data); + if (!event) return false; + return handler(event) !== false; +} + +/** + * Structural view of a SelectList-like target for mouse routing. Declared here + * (rather than importing the component) to keep this core module free of any + * component-to-core import cycle. + */ +export interface SelectListMouseTarget { + handleWheel(delta: -1 | 1): void; + hitTest(line: number): number | undefined; + setHoverIndex(index: number | null): void; + clickItem(index: number): void; +} + +/** + * Route a decoded mouse event against a SelectList-like target at the given + * 0-based frame-local `line`. Centralizes the repeated wheel/hit-test/hover/ + * click pattern. Returns `true` when the event was consumed. + */ +export function routeSelectListMouse(target: SelectListMouseTarget, event: SgrMouseEvent, line: number): boolean { + if (event.wheel !== null) { + target.handleWheel(event.wheel); + return true; + } + const index = target.hitTest(line); + if (event.motion) { + target.setHoverIndex(index ?? null); + return true; + } + if (event.leftClick && index !== undefined) { + target.clickItem(index); + return true; + } + return false; +} + /** * Implemented by components that accept routed mouse events at frame-local * coordinates. Hosts translate screen coordinates to the component's own diff --git a/packages/tui/test/emergency-restore-altscreen.test.ts b/packages/tui/test/emergency-restore-altscreen.test.ts index a93b97eb3..bb9bc33f1 100644 --- a/packages/tui/test/emergency-restore-altscreen.test.ts +++ b/packages/tui/test/emergency-restore-altscreen.test.ts @@ -76,6 +76,9 @@ describe("emergencyTerminalRestore alt-screen gating", () => { const restored = writes.join(""); expect(restored).not.toContain("\x1b[?1049l"); + expect(restored).toContain("\x1b[?1006l"); + expect(restored).toContain("\x1b[?1003l"); + expect(restored).toContain("\x1b[?1000l"); // Still performs the blind restore itself (cursor visibility proves the branch ran). expect(restored).toContain("\x1b[?25h"); }); @@ -87,7 +90,11 @@ describe("emergencyTerminalRestore alt-screen gating", () => { writes.length = 0; emergencyTerminalRestore(); - expect(writes.join("")).toContain("\x1b[?1049l"); + const firstRestore = writes.join(""); + expect(firstRestore).toContain("\x1b[?1049l"); + expect(firstRestore).toContain("\x1b[?1006l"); + expect(firstRestore).toContain("\x1b[?1003l"); + expect(firstRestore).toContain("\x1b[?1000l"); // State was consumed: a second restore must not leave the (now main) buffer again. writes.length = 0; @@ -99,12 +106,20 @@ describe("emergencyTerminalRestore alt-screen gating", () => { const inactive = startCapturedTerminal(); inactive.writes.length = 0; emergencyTerminalRestore(); // activeTerminal set, alt screen never entered - expect(inactive.writes.join("")).not.toContain("\x1b[?1049l"); + const inactiveRestore = inactive.writes.join(""); + expect(inactiveRestore).not.toContain("\x1b[?1049l"); + expect(inactiveRestore).toContain("\x1b[?1006l"); + expect(inactiveRestore).toContain("\x1b[?1003l"); + expect(inactiveRestore).toContain("\x1b[?1000l"); const active = startCapturedTerminal(); setAltScreenActive(true); active.writes.length = 0; emergencyTerminalRestore(); - expect(active.writes.join("")).toContain("\x1b[?1049l"); + const activeRestore = active.writes.join(""); + expect(activeRestore).toContain("\x1b[?1049l"); + expect(activeRestore).toContain("\x1b[?1006l"); + expect(activeRestore).toContain("\x1b[?1003l"); + expect(activeRestore).toContain("\x1b[?1000l"); }); }); diff --git a/packages/tui/test/mouse.test.ts b/packages/tui/test/mouse.test.ts index cc2b2e390..5282f7f01 100644 --- a/packages/tui/test/mouse.test.ts +++ b/packages/tui/test/mouse.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from "bun:test"; -import { parseSgrMouse } from "@oh-my-pi/pi-tui/mouse"; +import { + parseSgrMouse, + routeSelectListMouse, + routeSgrMouseInput, + type SelectListMouseTarget, + type SgrMouseEvent, +} from "@oh-my-pi/pi-tui/mouse"; describe("parseSgrMouse", () => { it("returns null for non-mouse input", () => { @@ -40,3 +46,87 @@ describe("parseSgrMouse", () => { expect(event?.wheel).toBeNull(); }); }); + +describe("routeSgrMouseInput", () => { + it("returns false and does not call the handler for non-mouse input", () => { + let called = false; + const handled = routeSgrMouseInput("a", () => { + called = true; + return true; + }); + expect(handled).toBe(false); + expect(called).toBe(false); + }); + + it("decodes and forwards an SGR mouse report", () => { + let received: SgrMouseEvent | null = null; + const handled = routeSgrMouseInput("\x1b[<0;2;3M", event => { + received = event; + return true; + }); + expect(handled).toBe(true); + expect(received).not.toBeNull(); + const event = received as unknown as SgrMouseEvent; + expect(event.row).toBe(2); + expect(event.col).toBe(1); + expect(event.leftClick).toBe(true); + }); +}); + +describe("routeSelectListMouse", () => { + function makeTarget(hit: number | undefined) { + const calls: string[] = []; + const target: SelectListMouseTarget = { + handleWheel: delta => calls.push(`wheel:${delta}`), + hitTest: () => hit, + setHoverIndex: index => calls.push(`hover:${index}`), + clickItem: index => calls.push(`click:${index}`), + }; + return { target, calls }; + } + + const baseEvent: SgrMouseEvent = { + button: 0, + col: 0, + row: 0, + release: false, + wheel: null, + motion: false, + leftClick: false, + }; + + it("forwards wheel notches", () => { + const { target, calls } = makeTarget(undefined); + const handled = routeSelectListMouse(target, { ...baseEvent, wheel: 1 }, 0); + expect(handled).toBe(true); + expect(calls).toEqual(["wheel:1"]); + }); + + it("hovers the hit-tested row on motion", () => { + const { target, calls } = makeTarget(4); + const handled = routeSelectListMouse(target, { ...baseEvent, motion: true }, 0); + expect(handled).toBe(true); + expect(calls).toEqual(["hover:4"]); + }); + + it("clears hover when motion misses a row", () => { + const { target, calls } = makeTarget(undefined); + const handled = routeSelectListMouse(target, { ...baseEvent, motion: true }, 0); + expect(handled).toBe(true); + expect(calls).toEqual(["hover:null"]); + }); + + it("clicks the hit-tested row", () => { + const { target, calls } = makeTarget(2); + const handled = routeSelectListMouse(target, { ...baseEvent, leftClick: true }, 0); + expect(handled).toBe(true); + expect(calls).toEqual(["click:2"]); + }); + + it("ignores release events", () => { + const { target, calls } = makeTarget(2); + const handled = routeSelectListMouse(target, { ...baseEvent, release: true }, 0); + expect(handled).toBe(false); + expect(calls).toEqual([]); + }); +}); diff --git a/packages/tui/test/select-list.test.ts b/packages/tui/test/select-list.test.ts index 21399880c..f9b2128ed 100644 --- a/packages/tui/test/select-list.test.ts +++ b/packages/tui/test/select-list.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { SelectList } from "@oh-my-pi/pi-tui/components/select-list"; import { KeybindingsManager, setKeybindings, TUI_KEYBINDINGS } from "@oh-my-pi/pi-tui/keybindings"; +import type { SgrMouseEvent } from "@oh-my-pi/pi-tui/mouse"; import { visibleWidth } from "@oh-my-pi/pi-tui/utils"; const testTheme = { @@ -373,3 +374,67 @@ describe("SelectList", () => { }); }); }); + +describe("SelectList.routeMouse", () => { + const hoverTheme = { + ...testTheme, + hovered: (text: string) => `${text}`, + selectedText: (text: string) => `${text}`, + }; + + const baseEvent: SgrMouseEvent = { + button: 0, + col: 0, + row: 0, + release: false, + wheel: null, + motion: false, + leftClick: false, + }; + + function makeList() { + const items = [ + { value: "a", label: "a" }, + { value: "b", label: "b" }, + { value: "c", label: "c" }, + ]; + return new SelectList(items, 5, hoverTheme); + } + + it("advances selection on a wheel notch", () => { + const list = makeList(); + let changed: string | undefined; + list.onSelectionChange = item => { + changed = item.value; + }; + list.render(80); + + list.routeMouse({ ...baseEvent, wheel: 1 }, 0, 0); + + expect(changed).toBe("b"); + }); + + it("hovers the row under the pointer on motion", () => { + const list = makeList(); + list.render(80); + + list.routeMouse({ ...baseEvent, motion: true }, 1, 0); + const rendered = list.render(80).join("\n"); + + expect(rendered).toContain(""); + expect(rendered).not.toContain(""); + }); + + it("confirms the clicked row", () => { + const list = makeList(); + let selected: string | undefined; + list.onSelect = item => { + selected = item.value; + }; + list.render(80); + + list.routeMouse({ ...baseEvent, leftClick: true }, 2, 0); + + expect(selected).toBe("c"); + }); +}); From 3995e629eaa195de6f7a0a5357b5016df57b136a Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Wed, 24 Jun 2026 12:54:30 -0700 Subject: [PATCH 2/4] refactor(coding-agent): reuse shared TUI mouse-routing helpers Migrate fullscreen overlay selectors onto routeSgrMouseInput() and routeSelectListMouse(), removing duplicated SGR parsing and SelectList hit-test boilerplate. Behavior-preserving: wheel step sizes, footer/row offset guards, and consumption semantics are unchanged. Make the inline-picker wrappers (theme/thinking/queue-mode/show-images/ plugin) MouseRoutable, each subtracting their single top-border row before delegating to SelectList.routeMouse(). Name previously magic coordinate offsets (spacerRowsAfterTabs, contentColInset). --- packages/coding-agent/CHANGELOG.md | 3 + .../components/agent-transcript-viewer.ts | 16 +++-- .../modes/components/plan-review-overlay.ts | 70 +++++++++---------- .../src/modes/components/plugin-selector.ts | 7 +- .../modes/components/queue-mode-selector.ts | 7 +- .../src/modes/components/session-selector.ts | 21 +++--- .../src/modes/components/settings-selector.ts | 30 +++----- .../modes/components/show-images-selector.ts | 7 +- .../src/modes/components/theme-selector.ts | 7 +- .../src/modes/components/thinking-selector.ts | 7 +- .../src/modes/setup-wizard/scenes/glyph.ts | 15 +--- .../modes/setup-wizard/scenes/providers.ts | 3 +- .../src/modes/setup-wizard/scenes/theme.ts | 18 ++--- .../modes/setup-wizard/scenes/web-search.ts | 21 +++--- .../src/modes/setup-wizard/wizard-overlay.ts | 11 +-- 15 files changed, 121 insertions(+), 122 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3ba62ff1f..642734b87 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -47,6 +47,9 @@ - Fixed extension `tool_call`/`tool_result` events for hashline `edit` calls to expose `event.input.path` for single-file edits and `event.input.paths` for every parsed target, so planning-mode gates can allow one markdown plan edit but still block multi-file hashline calls that cannot be represented by one path ([#1678](https://github.com/can1357/oh-my-pi/issues/1678)). - Fixed scripted `eval` `agent()` subagents continuing after a successful `yield` when a trailing empty assistant `stop` arrived after the executor's yield-triggered abort. The session's `agent_end` maintenance compared `#assistantEndedWithSuccessfulYield(msg)` against the trailing empty-stop message — not the prior yield-bearing one — so the empty-stop recovery path appended a retry reminder and scheduled `agent.continue()`, reviving the already-yielded child. The yield handler now sets a sticky `#yieldTerminationPending` flag (cleared on the next `prompt()`) that short-circuits empty-stop / unexpected-stop / compaction continuations for the rest of the run, so a successful yield is terminal regardless of trailing stops ([#3389](https://github.com/can1357/oh-my-pi/issues/3389)). - Fixed snapcompact rasterizing transcript frames into requests bound for GitHub Copilot business and enterprise endpoints, which then rejected the session permanently with `400 vision is not supported`. The snapcompact vision gate now also short-circuits whenever `model.provider === "github-copilot"` and the resolved `baseUrl` is not the canonical personal-Copilot host, protecting cached/stale Model specs that still advertise `["text","image"]` on a non-personal endpoint. ([#3387](https://github.com/can1357/oh-my-pi/issues/3387)) +### Changed + +- Reused shared TUI mouse-routing helpers across fullscreen overlay selectors. ## [16.1.16] - 2026-06-23 diff --git a/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts b/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts index 4c280c581..1f3df1bd0 100644 --- a/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts +++ b/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts @@ -15,7 +15,7 @@ */ import * as fs from "node:fs"; import type { AgentTool } from "@oh-my-pi/pi-agent-core"; -import { type Component, Editor, matchesKey, parseSgrMouse, ScrollView, type TUI } from "@oh-my-pi/pi-tui"; +import { type Component, Editor, matchesKey, routeSgrMouseInput, ScrollView, type TUI } from "@oh-my-pi/pi-tui"; import { formatDuration, formatNumber, logger } from "@oh-my-pi/pi-utils"; import type { KeyId } from "../../config/keybindings"; import type { MessageRenderer } from "../../extensibility/extensions/types"; @@ -413,12 +413,14 @@ export class AgentTranscriptViewer implements Component { handleInput(data: string): void { if (data.startsWith("\x1b[<")) { - const event = parseSgrMouse(data); - if (event?.wheel != null) { - this.#scrollView.scroll(event.wheel * 3); - this.#syncFollow(); - this.deps.requestRender(); - } + routeSgrMouseInput(data, event => { + if (event.wheel !== null) { + this.#scrollView.scroll(event.wheel * 3); + this.#syncFollow(); + this.deps.requestRender(); + } + return true; + }); return; } diff --git a/packages/coding-agent/src/modes/components/plan-review-overlay.ts b/packages/coding-agent/src/modes/components/plan-review-overlay.ts index 2757dc6e1..11f7a96d2 100644 --- a/packages/coding-agent/src/modes/components/plan-review-overlay.ts +++ b/packages/coding-agent/src/modes/components/plan-review-overlay.ts @@ -23,7 +23,7 @@ import { Markdown, type MarkdownTheme, matchesKey, - parseSgrMouse, + routeSgrMouseInput, ScrollView, truncateToWidth, visibleWidth, @@ -333,42 +333,42 @@ export class PlanReviewOverlay implements Component { * the body. */ #handleMouse(data: string): boolean { - const event = parseSgrMouse(data); - if (!event) return false; - if (event.wheel !== null) { - // Scroll wheel: three rows per notch. - this.#scrollView.scroll(event.wheel * 3); - return true; - } - if (event.release) return true; - if (event.motion) { - // Motion (hover or drag): light up the option row under the pointer so a - // mouse user gets the same affordance the keyboard cursor gives. Any - // non-option row clears the highlight. - this.#setHoveredOption(this.#optionClickRows.get(event.row)); - return true; - } - if (!event.leftClick) return true; - const optionIndex = this.#optionClickRows.get(event.row); - if (optionIndex !== undefined) { - if (!this.#disabled.has(optionIndex)) { - this.#focus = "actions"; - this.#selectedIndex = optionIndex; - this.#confirmSelection(); + return routeSgrMouseInput(data, event => { + if (event.wheel !== null) { + // Scroll wheel: three rows per notch. + this.#scrollView.scroll(event.wheel * 3); + return true; + } + if (event.release) return true; + if (event.motion) { + // Motion (hover or drag): light up the option row under the pointer so a + // mouse user gets the same affordance the keyboard cursor gives. Any + // non-option row clears the highlight. + this.#setHoveredOption(this.#optionClickRows.get(event.row)); + return true; + } + if (!event.leftClick) return true; + const optionIndex = this.#optionClickRows.get(event.row); + if (optionIndex !== undefined) { + if (!this.#disabled.has(optionIndex)) { + this.#focus = "actions"; + this.#selectedIndex = optionIndex; + this.#confirmSelection(); + } + return true; + } + const tocPos = this.#tocClickRows.get(event.row); + if (tocPos !== undefined && event.col < this.#sidebarClickMaxCol) { + this.#focus = "toc"; + this.#tocCursor = tocPos; + this.#scrubBodyToToc(); + return true; + } + if (this.#bodyClickRows.has(event.row)) { + this.#setFocus("body"); } return true; - } - const tocPos = this.#tocClickRows.get(event.row); - if (tocPos !== undefined && event.col < this.#sidebarClickMaxCol) { - this.#focus = "toc"; - this.#tocCursor = tocPos; - this.#scrubBodyToToc(); - return true; - } - if (this.#bodyClickRows.has(event.row)) { - this.#setFocus("body"); - } - return true; + }); } /** Set the hovered option from a hit-tested row, ignoring disabled rows and diff --git a/packages/coding-agent/src/modes/components/plugin-selector.ts b/packages/coding-agent/src/modes/components/plugin-selector.ts index aae2fd274..68613193f 100644 --- a/packages/coding-agent/src/modes/components/plugin-selector.ts +++ b/packages/coding-agent/src/modes/components/plugin-selector.ts @@ -4,7 +4,7 @@ * Shows available plugins from all configured marketplaces in a SelectList. * Selecting a plugin triggers installation. Esc cancels. */ -import { Container, type SelectItem, SelectList } from "@oh-my-pi/pi-tui"; +import { Container, type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; import { getSelectListTheme } from "../theme/theme"; import { DynamicBorder } from "./dynamic-border"; @@ -82,6 +82,11 @@ export class PluginSelectorComponent extends Container { getSelectList(): SelectList { return this.#selectList; } + + routeMouse(event: SgrMouseEvent, line: number, col: number): void { + const topBorderRows = 1; + this.#selectList.routeMouse(event, line - topBorderRows, col); + } } function splitPluginId(id: string): [string, string, "user" | "project" | undefined] | [null, null, null] { diff --git a/packages/coding-agent/src/modes/components/queue-mode-selector.ts b/packages/coding-agent/src/modes/components/queue-mode-selector.ts index c9a575427..d283a88d7 100644 --- a/packages/coding-agent/src/modes/components/queue-mode-selector.ts +++ b/packages/coding-agent/src/modes/components/queue-mode-selector.ts @@ -1,4 +1,4 @@ -import { Container, type SelectItem, SelectList } from "@oh-my-pi/pi-tui"; +import { Container, type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; import { getSelectListTheme } from "../../modes/theme/theme"; import { DynamicBorder } from "./dynamic-border"; @@ -53,4 +53,9 @@ export class QueueModeSelectorComponent extends Container { getSelectList(): SelectList { return this.#selectList; } + + routeMouse(event: SgrMouseEvent, line: number, col: number): void { + const topBorderRows = 1; + this.#selectList.routeMouse(event, line - topBorderRows, col); + } } diff --git a/packages/coding-agent/src/modes/components/session-selector.ts b/packages/coding-agent/src/modes/components/session-selector.ts index 01fe5447c..94db7750d 100644 --- a/packages/coding-agent/src/modes/components/session-selector.ts +++ b/packages/coding-agent/src/modes/components/session-selector.ts @@ -5,8 +5,8 @@ import { Input, matchesKey, padding, - parseSgrMouse, replaceTabs, + routeSgrMouseInput, ScrollView, Spacer, Text, @@ -726,15 +726,16 @@ export class SessionSelectorComponent extends Container { */ #handleMouse(data: string): void { if (this.#confirmationDialog) return; - const event = parseSgrMouse(data); - if (!event) return; - if (event.wheel !== null) { - this.#sessionList.handleWheel(event.wheel); - return; - } - if (!event.leftClick || event.row >= this.#footerStart) return; - const index = this.#sessionList.hitTestSession(event.row - this.#listLineOffset); - if (index !== undefined) this.#sessionList.selectAndConfirm(index); + routeSgrMouseInput(data, event => { + if (event.wheel !== null) { + this.#sessionList.handleWheel(event.wheel); + return true; + } + if (!event.leftClick || event.row >= this.#footerStart) return true; + const index = this.#sessionList.hitTestSession(event.row - this.#listLineOffset); + if (index !== undefined) this.#sessionList.selectAndConfirm(index); + return true; + }); } getSessionList(): SessionList { diff --git a/packages/coding-agent/src/modes/components/settings-selector.ts b/packages/coding-agent/src/modes/components/settings-selector.ts index 35930fde9..f65027eea 100644 --- a/packages/coding-agent/src/modes/components/settings-selector.ts +++ b/packages/coding-agent/src/modes/components/settings-selector.ts @@ -10,7 +10,8 @@ import { type ImageBudget, Input, matchesKey, - parseSgrMouse, + routeSelectListMouse, + routeSgrMouseInput, type SelectItem, SelectList, type SettingItem, @@ -89,7 +90,6 @@ class SelectSubmenu extends Container { #previewText: Text | null = null; #previewUpdateRequestId: number = 0; #selectListLineOffset = 0; - #selectListLineCount = 0; constructor( title: string, @@ -187,7 +187,6 @@ class SelectSubmenu extends Container { const childLines = child.render(Math.max(1, width)); if (child === this.#selectList) { this.#selectListLineOffset = lines.length; - this.#selectListLineCount = childLines.length; } lines.push(...childLines); } @@ -196,20 +195,7 @@ class SelectSubmenu extends Container { /** Mouse routed from the host: wheel steps, hover lights, click confirms. */ routeMouse(event: SgrMouseEvent, line: number, _col: number): void { - if (event.wheel !== null) { - this.#selectList.handleWheel(event.wheel); - return; - } - const listLine = line - this.#selectListLineOffset; - const within = listLine >= 0 && listLine < this.#selectListLineCount; - const index = within ? this.#selectList.hitTest(listLine) : undefined; - if (event.motion) { - this.#selectList.setHoverIndex(index ?? null); - return; - } - if (event.leftClick && index !== undefined) { - this.#selectList.clickItem(index); - } + routeSelectListMouse(this.#selectList, event, line - this.#selectListLineOffset); } handleInput(data: string): void { @@ -458,12 +444,14 @@ export class SettingsSelectorComponent implements Component { * activates it (toggle / open submenu). */ #handleMouse(data: string): boolean { - const event = parseSgrMouse(data); - if (!event) return false; + return routeSgrMouseInput(data, event => this.#routeMouseEvent(event)); + } + #routeMouseEvent(event: SgrMouseEvent): boolean { const list = this.#searchList ?? this.#currentList; - // row() insets content by two columns (border + space). - const innerCol = event.col - 2; + // row() insets content by the border column plus a space. + const contentColInset = 2; + const innerCol = event.col - contentColInset; const contentLine = event.row - this.#contentRowStart; // An open submenu owns the pointer: wheel, hover, and clicks route into diff --git a/packages/coding-agent/src/modes/components/show-images-selector.ts b/packages/coding-agent/src/modes/components/show-images-selector.ts index 6b8469c8d..63da97a8b 100644 --- a/packages/coding-agent/src/modes/components/show-images-selector.ts +++ b/packages/coding-agent/src/modes/components/show-images-selector.ts @@ -1,4 +1,4 @@ -import { Container, type SelectItem, SelectList } from "@oh-my-pi/pi-tui"; +import { Container, type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; import { getSelectListTheme } from "../../modes/theme/theme"; import { DynamicBorder } from "./dynamic-border"; @@ -42,4 +42,9 @@ export class ShowImagesSelectorComponent extends Container { getSelectList(): SelectList { return this.#selectList; } + + routeMouse(event: SgrMouseEvent, line: number, col: number): void { + const topBorderRows = 1; + this.#selectList.routeMouse(event, line - topBorderRows, col); + } } diff --git a/packages/coding-agent/src/modes/components/theme-selector.ts b/packages/coding-agent/src/modes/components/theme-selector.ts index d0a09990b..905e549b2 100644 --- a/packages/coding-agent/src/modes/components/theme-selector.ts +++ b/packages/coding-agent/src/modes/components/theme-selector.ts @@ -1,4 +1,4 @@ -import { Container, type SelectItem, SelectList } from "@oh-my-pi/pi-tui"; +import { Container, type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; import { getSelectListTheme } from "../../modes/theme/theme"; import { DynamicBorder } from "./dynamic-border"; @@ -60,4 +60,9 @@ export class ThemeSelectorComponent extends Container { getSelectList(): SelectList { return this.#selectList; } + + routeMouse(event: SgrMouseEvent, line: number, col: number): void { + const topBorderRows = 1; + this.#selectList.routeMouse(event, line - topBorderRows, col); + } } diff --git a/packages/coding-agent/src/modes/components/thinking-selector.ts b/packages/coding-agent/src/modes/components/thinking-selector.ts index 9c26dc94b..d2b7c2236 100644 --- a/packages/coding-agent/src/modes/components/thinking-selector.ts +++ b/packages/coding-agent/src/modes/components/thinking-selector.ts @@ -1,5 +1,5 @@ import type { Effort } from "@oh-my-pi/pi-ai"; -import { Container, type SelectItem, SelectList } from "@oh-my-pi/pi-tui"; +import { Container, type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; import { getSelectListTheme } from "../../modes/theme/theme"; import { getThinkingLevelMetadata } from "../../thinking"; import { DynamicBorder } from "./dynamic-border"; @@ -49,4 +49,9 @@ export class ThinkingSelectorComponent extends Container { getSelectList(): SelectList { return this.#selectList; } + + routeMouse(event: SgrMouseEvent, line: number, col: number): void { + const topBorderRows = 1; + this.#selectList.routeMouse(event, line - topBorderRows, col); + } } diff --git a/packages/coding-agent/src/modes/setup-wizard/scenes/glyph.ts b/packages/coding-agent/src/modes/setup-wizard/scenes/glyph.ts index 460cfd363..ac7682971 100644 --- a/packages/coding-agent/src/modes/setup-wizard/scenes/glyph.ts +++ b/packages/coding-agent/src/modes/setup-wizard/scenes/glyph.ts @@ -1,4 +1,4 @@ -import { type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; +import { routeSelectListMouse, type SelectItem, SelectList, type SgrMouseEvent } from "@oh-my-pi/pi-tui"; import { getSelectListTheme, type SymbolPreset, setSymbolPreset, theme } from "../../theme/theme"; import type { SetupScene, SetupSceneController, SetupSceneHost } from "./types"; @@ -65,18 +65,7 @@ class GlyphSceneController implements SetupSceneController { /** Wheel moves the highlight (live preview); hover lights the row under the pointer; click confirms it. */ routeMouse(event: SgrMouseEvent, line: number, _col: number): void { if (this.#committing) return; - if (event.wheel !== null) { - this.#selectList.handleWheel(event.wheel); - return; - } - const index = this.#selectList.hitTest(line - this.#listRowStart); - if (event.motion) { - this.#selectList.setHoverIndex(index ?? null); - return; - } - if (event.leftClick && index !== undefined) { - this.#selectList.clickItem(index); - } + routeSelectListMouse(this.#selectList, event, line - this.#listRowStart); } render(width: number): readonly string[] { diff --git a/packages/coding-agent/src/modes/setup-wizard/scenes/providers.ts b/packages/coding-agent/src/modes/setup-wizard/scenes/providers.ts index c77251cd7..4b282e816 100644 --- a/packages/coding-agent/src/modes/setup-wizard/scenes/providers.ts +++ b/packages/coding-agent/src/modes/setup-wizard/scenes/providers.ts @@ -74,7 +74,8 @@ class ProvidersSceneController implements SetupSceneController { return; } if (event.motion) this.#tabBar.setHoverTab(null); - const bodyLine = line - this.#tabRowCount - 1; + const spacerRowsAfterTabs = 1; + const bodyLine = line - this.#tabRowCount - spacerRowsAfterTabs; if (tab.routeMouse) { tab.routeMouse(event, bodyLine, col); return; diff --git a/packages/coding-agent/src/modes/setup-wizard/scenes/theme.ts b/packages/coding-agent/src/modes/setup-wizard/scenes/theme.ts index 2a05d1fc3..ae55f761b 100644 --- a/packages/coding-agent/src/modes/setup-wizard/scenes/theme.ts +++ b/packages/coding-agent/src/modes/setup-wizard/scenes/theme.ts @@ -1,5 +1,6 @@ import { padding, + routeSelectListMouse, type SelectItem, SelectList, type SgrMouseEvent, @@ -128,18 +129,11 @@ class ThemeSceneController implements SetupSceneController { /** Wheel moves the highlight (live preview); hover lights the row under the pointer; click confirms it. */ routeMouse(event: SgrMouseEvent, line: number, _col: number): void { - if (event.wheel !== null) { - this.#selectList.handleWheel(event.wheel); - return; - } - const index = this.#listRowStart >= 0 ? this.#selectList.hitTest(line - this.#listRowStart) : undefined; - if (event.motion) { - this.#selectList.setHoverIndex(index ?? null); - return; - } - if (event.leftClick && index !== undefined) { - this.#selectList.clickItem(index); - } + // Mirror the pre-helper flow: wheel/motion are always processed, but a + // hidden list (#listRowStart < 0, e.g. while loading all themes) must + // never hit-test a row — route through a line that resolves to undefined. + const listLine = this.#listRowStart >= 0 ? line - this.#listRowStart : Number.NEGATIVE_INFINITY; + routeSelectListMouse(this.#selectList, event, listLine); } render(width: number): readonly string[] { diff --git a/packages/coding-agent/src/modes/setup-wizard/scenes/web-search.ts b/packages/coding-agent/src/modes/setup-wizard/scenes/web-search.ts index 9016b56f7..40f7b175a 100644 --- a/packages/coding-agent/src/modes/setup-wizard/scenes/web-search.ts +++ b/packages/coding-agent/src/modes/setup-wizard/scenes/web-search.ts @@ -1,4 +1,10 @@ -import { type SelectItem, SelectList, type SgrMouseEvent, truncateToWidth } from "@oh-my-pi/pi-tui"; +import { + routeSelectListMouse, + type SelectItem, + SelectList, + type SgrMouseEvent, + truncateToWidth, +} from "@oh-my-pi/pi-tui"; import { SETTINGS_SCHEMA } from "../../../config/settings-schema"; import { getSearchProvider, setPreferredSearchProvider } from "../../../web/search/provider"; import { isSearchProviderPreference, type SearchProviderId } from "../../../web/search/types"; @@ -59,18 +65,7 @@ export class WebSearchTab implements SetupTab { /** Wheel moves the highlight; hover lights the row under the pointer; click confirms it. */ routeMouse(event: SgrMouseEvent, line: number, _col: number): void { - if (event.wheel !== null) { - this.#list.handleWheel(event.wheel); - return; - } - const index = this.#list.hitTest(line - this.#listRowStart); - if (event.motion) { - this.#list.setHoverIndex(index ?? null); - return; - } - if (event.leftClick && index !== undefined) { - this.#list.clickItem(index); - } + routeSelectListMouse(this.#list, event, line - this.#listRowStart); } invalidate(): void { diff --git a/packages/coding-agent/src/modes/setup-wizard/wizard-overlay.ts b/packages/coding-agent/src/modes/setup-wizard/wizard-overlay.ts index 938e6fbe3..0e65e45f0 100644 --- a/packages/coding-agent/src/modes/setup-wizard/wizard-overlay.ts +++ b/packages/coding-agent/src/modes/setup-wizard/wizard-overlay.ts @@ -3,7 +3,8 @@ import { matchesKey, type OverlayFocusOwner, padding, - parseSgrMouse, + routeSgrMouseInput, + type SgrMouseEvent, truncateToWidth, visibleWidth, } from "@oh-my-pi/pi-tui"; @@ -104,7 +105,9 @@ export class SetupWizardComponent implements Component, OverlayFocusOwner { handleInput(data: string): void { if (this.#phase === "done") return; if (data.startsWith("\x1b[<")) { - this.#handleMouse(data); + routeSgrMouseInput(data, event => { + this.#routeMouseEvent(event); + }); return; } if (matchesKey(data, "ctrl+c")) { @@ -146,9 +149,7 @@ export class SetupWizardComponent implements Component, OverlayFocusOwner { * advances the splash/outro like Enter. Raw reports never reach scene * keyboard input. */ - #handleMouse(data: string): void { - const event = parseSgrMouse(data); - if (!event) return; + #routeMouseEvent(event: SgrMouseEvent): void { if (this.#phase === "splash" || this.#phase === "outro") { if (!event.leftClick) return; if (this.#phase === "splash") this.#beginScene(); From 16df865a88f4eeb4fc55d4628fe032e6932db4fa Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Wed, 24 Jun 2026 12:54:39 -0700 Subject: [PATCH 3/4] test(coding-agent): cover inline-picker wrapper mouse offset Add regression tests for all five inline-picker wrappers asserting a click on the top border row is inert and a click on the first list row below it confirms the first item, pinning the top-border line offset. --- .../show-images-selector-mouse.test.ts | 52 +++++++++ .../wrapper-selector-mouse-offset.test.ts | 105 ++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 packages/coding-agent/test/modes/components/show-images-selector-mouse.test.ts create mode 100644 packages/coding-agent/test/modes/components/wrapper-selector-mouse-offset.test.ts diff --git a/packages/coding-agent/test/modes/components/show-images-selector-mouse.test.ts b/packages/coding-agent/test/modes/components/show-images-selector-mouse.test.ts new file mode 100644 index 000000000..d3ba4c1ea --- /dev/null +++ b/packages/coding-agent/test/modes/components/show-images-selector-mouse.test.ts @@ -0,0 +1,52 @@ +import { beforeAll, describe, expect, it } from "bun:test"; +import { ShowImagesSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/show-images-selector"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { SgrMouseEvent } from "@oh-my-pi/pi-tui"; + +beforeAll(async () => { + await initTheme(); +}); + +function leftClick(line: number): SgrMouseEvent { + return { button: 0, col: 0, row: line, release: false, wheel: null, motion: false, leftClick: true }; +} + +/** + * The wrapper mounts a single-line top DynamicBorder before its SelectList, so + * routed component-local lines are offset by one. Regression guard for the + * off-by-one that would let a top-border click select the first row. + */ +describe("ShowImagesSelectorComponent.routeMouse offset", () => { + it("ignores a click on the top border row (line 0)", () => { + let selected: boolean | undefined; + const component = new ShowImagesSelectorComponent( + true, + value => { + selected = value; + }, + () => {}, + ); + component.render(80); + + component.routeMouse(leftClick(0), 0, 0); + + expect(selected).toBeUndefined(); + }); + + it("selects the first item when the row below the border is clicked (line 1)", () => { + let selected: boolean | undefined; + const component = new ShowImagesSelectorComponent( + true, + value => { + selected = value; + }, + () => {}, + ); + component.render(80); + + component.routeMouse(leftClick(1), 1, 0); + + // First SelectList row is "Yes" → true. + expect(selected).toBe(true); + }); +}); diff --git a/packages/coding-agent/test/modes/components/wrapper-selector-mouse-offset.test.ts b/packages/coding-agent/test/modes/components/wrapper-selector-mouse-offset.test.ts new file mode 100644 index 000000000..76805d3e9 --- /dev/null +++ b/packages/coding-agent/test/modes/components/wrapper-selector-mouse-offset.test.ts @@ -0,0 +1,105 @@ +import { beforeAll, describe, expect, it } from "bun:test"; +import { Effort } from "@oh-my-pi/pi-ai"; +import { PluginSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/plugin-selector"; +import { QueueModeSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/queue-mode-selector"; +import { ThemeSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/theme-selector"; +import { ThinkingSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/thinking-selector"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { SgrMouseEvent } from "@oh-my-pi/pi-tui"; + +beforeAll(async () => { + await initTheme(); +}); + +function leftClick(line: number): SgrMouseEvent { + return { button: 0, col: 0, row: line, release: false, wheel: null, motion: false, leftClick: true }; +} + +/** + * Every wrapper mounts a single-line top DynamicBorder before its SelectList, + * so routed component-local lines are offset by one. These guard the + * off-by-one that would let a top-border click select the first row. Each case + * asserts line 0 (border) is inert and line 1 (first list row) confirms. + */ +describe("inline-picker wrapper routeMouse offset", () => { + it("ThemeSelectorComponent ignores the border row and selects the first theme below it", () => { + let selected: string | undefined; + const component = new ThemeSelectorComponent( + "alpha", + ["alpha", "beta"], + value => { + selected = value; + }, + () => {}, + () => {}, + ); + component.render(80); + + component.routeMouse(leftClick(0), 0, 0); + expect(selected).toBeUndefined(); + + component.routeMouse(leftClick(1), 1, 0); + expect(selected).toBe("alpha"); + }); + + it("ThinkingSelectorComponent ignores the border row and selects the first level below it", () => { + let selected: Effort | undefined; + const levels = [Effort.Low, Effort.High]; + const component = new ThinkingSelectorComponent( + Effort.Low, + levels, + value => { + selected = value; + }, + () => {}, + ); + component.render(80); + + component.routeMouse(leftClick(0), 0, 0); + expect(selected).toBeUndefined(); + + component.routeMouse(leftClick(1), 1, 0); + expect(selected).toBe(Effort.Low); + }); + + it("QueueModeSelectorComponent ignores the border row and selects the first mode below it", () => { + let selected: "all" | "one-at-a-time" | undefined; + const component = new QueueModeSelectorComponent( + "all", + value => { + selected = value; + }, + () => {}, + ); + component.render(80); + + component.routeMouse(leftClick(0), 0, 0); + expect(selected).toBeUndefined(); + + // First SelectList row is "one-at-a-time" regardless of the preselected mode. + component.routeMouse(leftClick(1), 1, 0); + expect(selected).toBe("one-at-a-time"); + }); + + it("PluginSelectorComponent ignores the border row and selects the first plugin below it", () => { + let selectedName: string | undefined; + const component = new PluginSelectorComponent( + 1, + [{ plugin: { name: "alpha", description: "first" }, marketplace: "shop" }], + new Set(), + { + onSelect: name => { + selectedName = name; + }, + onCancel: () => {}, + }, + ); + component.render(80); + + component.routeMouse(leftClick(0), 0, 0); + expect(selectedName).toBeUndefined(); + + component.routeMouse(leftClick(1), 1, 0); + expect(selectedName).toBe("alpha"); + }); +}); From b9b509b059dccd53650571d42b2a09f515bdb9d8 Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Wed, 24 Jun 2026 14:42:05 -0700 Subject: [PATCH 4/4] test(tui): replace SgrMouseEvent cast with null guard Addresses review feedback: the repo bans type casts in new code. Use a real null guard that throws if the handler never received an event, so the test keeps the handler contract type-checked. --- packages/tui/test/mouse.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/tui/test/mouse.test.ts b/packages/tui/test/mouse.test.ts index 5282f7f01..d74b8e4cf 100644 --- a/packages/tui/test/mouse.test.ts +++ b/packages/tui/test/mouse.test.ts @@ -65,8 +65,8 @@ describe("routeSgrMouseInput", () => { return true; }); expect(handled).toBe(true); - expect(received).not.toBeNull(); - const event = received as unknown as SgrMouseEvent; + if (received === null) throw new Error("expected routeSgrMouseInput to forward an event"); + const event: SgrMouseEvent = received; expect(event.row).toBe(2); expect(event.col).toBe(1); expect(event.leftClick).toBe(true);