From 8089b7dd774cbee277aec82f403085ab2e8e3fdb Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 7 Jun 2026 02:50:20 +0200 Subject: [PATCH] feat(modes): reworked plan review flow to use an interactive overlay - Replaced the plan review flow to open PlanReviewOverlay for approvals. - Added scrollable markdown plan rendering with prompt, options, and footer in the overlay UI. - Added disabled-option handling so cursor movement and confirmation skip unavailable rows. - Added setPlanContent updates to refresh overlay text and reset scroll position on edits. --- packages/coding-agent/CHANGELOG.md | 1 + .../modes/components/plan-review-overlay.ts | 237 ++++++++++++++++++ .../src/modes/interactive-mode.ts | 100 +++++--- .../test/interactive-mode-plan-review.test.ts | 64 +++-- .../components/plan-review-overlay.test.ts | 195 ++++++++++++++ 5 files changed, 524 insertions(+), 73 deletions(-) create mode 100644 packages/coding-agent/src/modes/components/plan-review-overlay.ts create mode 100644 packages/coding-agent/test/modes/components/plan-review-overlay.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 444f73141..ad39641b8 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changed - Changed the directory grouping for `find`, `search`, `ast_grep`, `ast_edit`, and `lsp` diagnostics from a single flat `# dir/` heading per immediate directory to a multi-level tree that folds the common path prefix into one heading. Previously every group repeated the full directory path — so results rooted outside cwd printed the absolute prefix (e.g. `/Users/me/proj/`) on every heading and nested directories were never collapsed. Now a single-child directory chain folds into one heading (`# packages/pkg/src/`, including an absolute root for out-of-cwd results), subdirectories nest one `#` deeper (`## nested/` → `### child.ts`), and each directory's own files are listed before its subdirectories. TUI hyperlink reconstruction tracks the nested directory stack across the whole output so file and code-frame links keep resolving to the correct absolute paths. +- Changed the plan-mode approval surface from an inline transcript block plus a separate bottom selector into a single fullscreen overlay (like `/copy`). The overlay owns its entire content via `ScrollView`: the plan renders once as Markdown and scrolls inside the outlined box (PageUp/PageDown, g/G), with the approval options and the model-tier slider beneath it. ↑/↓ move the option cursor, ←/→ drive the slider, Enter confirms, the external-editor key opens the plan, and Esc cancels — so a tall plan no longer competes with the selector for vertical space or clips its head on ED3-risk terminals. ### Fixed diff --git a/packages/coding-agent/src/modes/components/plan-review-overlay.ts b/packages/coding-agent/src/modes/components/plan-review-overlay.ts new file mode 100644 index 000000000..59b7e7fb5 --- /dev/null +++ b/packages/coding-agent/src/modes/components/plan-review-overlay.ts @@ -0,0 +1,237 @@ +/** + * Fullscreen plan-review overlay. Replaces the old inline `PlanReviewBlock` + * (which lived in the transcript and competed with the approval selector for + * vertical space, clipping tall plans). The overlay owns its entire content: the + * plan is rendered once via {@link Markdown} and windowed through a + * {@link ScrollView}, while the approval options (plus the optional model-tier + * slider) sit beneath it inside the same outlined box — one self-contained + * surface in the spirit of the `/copy` picker. + * + * Key map mirrors the plan-mode hook selector so muscle memory carries over: + * ↑/↓ (j/k) move the option cursor, ←/→ (h/l) drive the slider, Enter confirms, + * Esc cancels, and the external-editor key opens the plan. PageUp/PageDown and + * g/G scroll the plan body. + */ +import { type Component, Markdown, type MarkdownTheme, matchesKey, ScrollView } from "@oh-my-pi/pi-tui"; +import { getMarkdownTheme, theme } from "../theme/theme"; +import { + matchesAppExternalEditor, + matchesSelectCancel, + matchesSelectDown, + matchesSelectPageDown, + matchesSelectPageUp, + matchesSelectUp, +} from "../utils/keybinding-matchers"; +import type { HookSelectorSlider } from "./hook-selector"; +import { bottomBorder, divider, row, topBorder } from "./overlay-box"; +import { renderSegmentTrack } from "./segment-track"; + +/** Title shown in the overlay's top border. */ +const OVERLAY_TITLE = "Plan Review"; +/** Minimum plan-body rows kept visible even on short terminals. */ +const MIN_BODY_ROWS = 3; + +export interface PlanReviewOverlayCallbacks { + /** Invoked with the chosen option label (never a disabled one). */ + onPick: (label: string) => void; + /** Invoked on Esc / cancel. */ + onCancel: () => void; + /** Invoked when the external-editor key is pressed (overlay stays open). */ + onExternalEditor?: () => void; +} + +export interface PlanReviewOverlayOptions { + /** Prompt rendered above the options (e.g. "Plan mode - next step"). */ + promptTitle?: string; + options: string[]; + /** Indices into `options` that render dimmed and cannot be selected. */ + disabledIndices?: number[]; + /** Footer hint line; falls back to a generic nav hint when omitted. */ + helpText?: string; + /** Initially highlighted option index. */ + initialIndex?: number; + /** Optional model-tier slider rendered between the plan body and options. */ + slider?: HookSelectorSlider; +} + +/** Default footer hint when the caller supplies none. */ +const DEFAULT_HELP = "up/down select enter confirm pgup/pgdn scroll esc cancel"; + +export class PlanReviewOverlay implements Component { + #md: Markdown; + #mdTheme: MarkdownTheme; + #scrollView: ScrollView; + #options: string[]; + #disabled: Set; + #helpText: string; + #promptTitle: string | undefined; + #selectedIndex: number; + #slider: HookSelectorSlider | undefined; + #sliderIndex: number; + + constructor( + planContent: string, + options: PlanReviewOverlayOptions, + private readonly callbacks: PlanReviewOverlayCallbacks, + ) { + this.#mdTheme = getMarkdownTheme(); + this.#md = new Markdown(planContent, 1, 0, this.#mdTheme); + this.#scrollView = new ScrollView([], { + height: MIN_BODY_ROWS, + scrollbar: "auto", + theme: { track: t => theme.fg("dim", t), thumb: t => theme.fg("accent", t) }, + }); + this.#options = options.options; + this.#disabled = new Set( + (options.disabledIndices ?? []).filter(i => Number.isInteger(i) && i >= 0 && i < this.#options.length), + ); + this.#helpText = options.helpText ?? DEFAULT_HELP; + this.#promptTitle = options.promptTitle; + this.#selectedIndex = this.#coerceIndex(options.initialIndex ?? 0); + if (options.slider && options.slider.segments.length > 0) { + this.#slider = options.slider; + this.#sliderIndex = Math.max(0, Math.min(options.slider.index, options.slider.segments.length - 1)); + } else { + this.#sliderIndex = 0; + } + } + + invalidate(): void { + this.#md.invalidate(); + } + + /** Swap the displayed plan (e.g. after an external-editor round-trip) and + * reset the scroll position so the operator starts at the top. */ + setPlanContent(planContent: string): void { + this.#md.setText(planContent); + this.#scrollView.scrollToTop(); + } + + /** Clamp `index` to range, then walk to the nearest enabled option so the + * cursor never rests on a disabled row. */ + #coerceIndex(index: number): number { + const max = this.#options.length - 1; + if (max < 0) return -1; + const clamped = Math.max(0, Math.min(index, max)); + if (!this.#disabled.has(clamped)) return clamped; + for (let i = clamped + 1; i <= max; i++) if (!this.#disabled.has(i)) return i; + for (let i = clamped - 1; i >= 0; i--) if (!this.#disabled.has(i)) return i; + return clamped; + } + + /** Move the option cursor by `delta`, skipping disabled rows, stopping at the + * list edge. */ + #moveSelection(delta: number): void { + const max = this.#options.length - 1; + if (max < 0) return; + let index = this.#selectedIndex; + while (true) { + const next = Math.max(0, Math.min(index + delta, max)); + if (next === index) return; + index = next; + if (!this.#disabled.has(index)) { + this.#selectedIndex = index; + return; + } + } + } + + #moveSlider(delta: number): void { + const slider = this.#slider; + if (!slider) return; + const next = Math.max(0, Math.min(slider.segments.length - 1, this.#sliderIndex + delta)); + if (next === this.#sliderIndex) return; + this.#sliderIndex = next; + slider.onChange?.(next); + } + + handleInput(keyData: string): void { + if (matchesSelectCancel(keyData)) { + this.callbacks.onCancel(); + return; + } + if (matchesSelectUp(keyData) || keyData === "k") { + this.#moveSelection(-1); + } else if (matchesSelectDown(keyData) || keyData === "j") { + this.#moveSelection(1); + } else if (matchesKey(keyData, "left") || (this.#slider && keyData === "h")) { + this.#moveSlider(-1); + } else if (matchesKey(keyData, "right") || (this.#slider && keyData === "l")) { + this.#moveSlider(1); + } else if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { + const index = this.#selectedIndex; + if (index >= 0 && index < this.#options.length && !this.#disabled.has(index)) { + this.callbacks.onPick(this.#options[index]!); + } + } else if (matchesSelectPageUp(keyData) || matchesKey(keyData, "pageUp")) { + this.#scrollView.page(-1); + } else if (matchesSelectPageDown(keyData) || matchesKey(keyData, "pageDown")) { + this.#scrollView.page(1); + } else if (keyData === "g") { + this.#scrollView.scrollToTop(); + } else if (keyData === "G") { + this.#scrollView.scrollToBottom(); + } else if (this.callbacks.onExternalEditor && matchesAppExternalEditor(keyData)) { + this.callbacks.onExternalEditor(); + } + } + + #renderSliderLines(): string[] { + const slider = this.#slider; + if (!slider) return []; + const active = this.#sliderIndex; + const track = renderSegmentTrack(slider.segments, active); + const leftArrow = theme.fg(active > 0 ? "accent" : "dim", "◂"); + const rightArrow = theme.fg(active < slider.segments.length - 1 ? "accent" : "dim", "▸"); + const caption = slider.caption ? `${theme.fg("dim", slider.caption)} ` : ""; + const trackLine = `${caption}${leftArrow} ${track} ${rightArrow}`; + const detail = slider.segments[active]?.detail; + if (!detail) return [trackLine]; + return [trackLine, ` ${theme.fg("dim", "↳")} ${theme.fg("muted", detail)}`]; + } + + #renderOptionLines(): string[] { + return this.#options.map((label, i) => { + const isSelected = i === this.#selectedIndex; + const isDisabled = this.#disabled.has(i); + const cursor = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; + const text = isDisabled + ? theme.fg("dim", label) + : isSelected + ? theme.bold(theme.fg("accent", label)) + : theme.fg("text", label); + return cursor + text; + }); + } + + render(width: number): string[] { + const termHeight = process.stdout.rows || 40; + const innerWidth = Math.max(1, width - 4); + + const planLines = this.#md.render(innerWidth); + const sliderLines = this.#renderSliderLines(); + const optionLines = this.#renderOptionLines(); + const promptLines = this.#promptTitle ? [theme.bold(theme.fg("accent", this.#promptTitle))] : []; + + // Chrome rows around the scrollable body: top border, divider, prompt, + // slider, options, divider, footer, bottom border. + const chrome = 5 + promptLines.length + sliderLines.length + optionLines.length; + const viewport = Math.max(MIN_BODY_ROWS, termHeight - chrome); + + this.#scrollView.setLines(planLines); + this.#scrollView.setHeight(viewport); + const body = this.#scrollView.render(innerWidth); + + return [ + topBorder(width, OVERLAY_TITLE), + ...body.map(line => row(line, width)), + divider(width), + ...promptLines.map(line => row(line, width)), + ...sliderLines.map(line => row(line, width)), + ...optionLines.map(line => row(line, width)), + divider(width), + row(theme.fg("dim", this.#helpText), width), + bottomBorder(width), + ]; + } +} diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 8f2bfa1b1..6abe5cb10 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -20,7 +20,7 @@ import { modelsAreEqual, type UsageReport, } from "@oh-my-pi/pi-ai"; -import type { Component, EditorTheme, SlashCommand } from "@oh-my-pi/pi-tui"; +import type { Component, EditorTheme, OverlayHandle, SlashCommand } from "@oh-my-pi/pi-tui"; import { Container, clearRenderCache, @@ -101,6 +101,7 @@ import type { EvalExecutionComponent } from "./components/eval-execution"; import type { HookEditorComponent } from "./components/hook-editor"; import type { HookInputComponent } from "./components/hook-input"; import type { HookSelectorComponent, HookSelectorSlider } from "./components/hook-selector"; +import { PlanReviewOverlay } from "./components/plan-review-overlay"; import { StatusLineComponent } from "./components/status-line"; import type { ToolExecutionHandle } from "./components/tool-execution"; import { TranscriptContainer } from "./components/transcript-container"; @@ -250,20 +251,6 @@ export interface InteractiveModeOptions { initialMessages?: string[]; } -/** - * Plan-review preview block. Once rendered it is static (a one-shot Markdown of - * the plan file), so even while it sits as the live bottom block beneath the - * approval selector its scrolled-off head is safe to commit to native - * scrollback. Reporting append-only lets an over-tall plan + selector commit the - * plan's head instead of clipping it — without this a plain {@link Container} is - * deferred and a long plan is cut off the top on ED3-risk terminals. - */ -class PlanReviewBlock extends Container { - isTranscriptBlockAppendOnly(): boolean { - return true; - } -} - export class InteractiveMode implements InteractiveModeContext { session: AgentSession; sessionManager: SessionManager; @@ -355,7 +342,8 @@ export class InteractiveMode implements InteractiveModeContext { #planModePreviousModelState: { model: Model; thinkingLevel?: ThinkingLevel } | undefined; #pendingModelSwitch: { model: Model; thinkingLevel?: ThinkingLevel } | undefined; #planModeHasEntered = false; - #planReviewContainer: Container | undefined; + #planReviewOverlay: PlanReviewOverlay | undefined; + #planReviewOverlayHandle: OverlayHandle | undefined; readonly lspServers: LspStartupServerInfo[] | undefined = undefined; mcpManager?: import("../mcp").MCPManager; readonly #toolUiContextSetter: (uiContext: ExtensionUIContext, hasUI: boolean) => void; @@ -1691,22 +1679,60 @@ export class InteractiveMode implements InteractiveModeContext { } } - #renderPlanPreview(planContent: string, options?: { append?: boolean }): void { - const existingContainer = this.#planReviewContainer; - const replaceExisting = options?.append !== true && existingContainer !== undefined; - const planReviewContainer = replaceExisting ? existingContainer : new PlanReviewBlock(); - planReviewContainer.clear(); - planReviewContainer.addChild(new Spacer(1)); - planReviewContainer.addChild(new DynamicBorder()); - planReviewContainer.addChild(new Text(theme.bold(theme.fg("accent", "Plan Review")), 1, 1)); - planReviewContainer.addChild(new Spacer(1)); - planReviewContainer.addChild(new Markdown(planContent, 1, 1, getMarkdownTheme())); - planReviewContainer.addChild(new DynamicBorder()); - if (!replaceExisting) { - this.chatContainer.addChild(planReviewContainer); - } - this.#planReviewContainer = planReviewContainer; + showPlanReview( + planContent: string, + title: string, + options: string[], + dialogOptions?: { + helpText?: string; + disabledIndices?: number[]; + onExternalEditor?: () => void; + initialIndex?: number; + }, + extra?: { slider?: HookSelectorSlider }, + ): Promise { + this.#hidePlanReview(); + const { promise, resolve } = Promise.withResolvers(); + let settled = false; + const finish = (choice: string | undefined): void => { + if (settled) return; + settled = true; + this.#hidePlanReview(); + this.ui.requestRender(); + resolve(choice); + }; + const overlay = new PlanReviewOverlay( + planContent, + { + promptTitle: title, + options, + disabledIndices: dialogOptions?.disabledIndices, + helpText: dialogOptions?.helpText, + initialIndex: dialogOptions?.initialIndex, + slider: extra?.slider, + }, + { + onPick: choice => finish(choice), + onCancel: () => finish(undefined), + onExternalEditor: dialogOptions?.onExternalEditor, + }, + ); + this.#planReviewOverlay = overlay; + this.#planReviewOverlayHandle = this.ui.showOverlay(overlay, { + anchor: "bottom-center", + width: "100%", + maxHeight: "100%", + margin: 0, + }); + this.ui.setFocus(overlay); this.ui.requestRender(); + return promise; + } + + #hidePlanReview(): void { + this.#planReviewOverlayHandle?.hide(); + this.#planReviewOverlayHandle = undefined; + this.#planReviewOverlay = undefined; } #getEditorTerminalPath(): string | null { @@ -1731,9 +1757,9 @@ export class InteractiveMode implements InteractiveModeContext { #getPlanReviewHelpText(): string { const externalEditorKey = this.keybindings.getDisplayString("app.editor.external"); if (!externalEditorKey) { - return "up/down navigate enter select esc cancel"; + return "up/down select enter confirm pgup/pgdn scroll esc cancel"; } - return `up/down navigate enter select ${externalEditorKey.toLowerCase()} open in editor esc cancel`; + return `up/down select enter confirm pgup/pgdn scroll ${externalEditorKey.toLowerCase()} open in editor esc cancel`; } #getPlanApprovalContextUsage(): ContextUsage | undefined { @@ -1794,7 +1820,7 @@ export class InteractiveMode implements InteractiveModeContext { }); if (result !== null) { await Bun.write(resolvedPath, result); - this.#renderPlanPreview(result); + this.#planReviewOverlay?.setPlanContent(result); this.showStatus("Plan updated in external editor."); } } catch (error) { @@ -2223,7 +2249,6 @@ export class InteractiveMode implements InteractiveModeContext { return; } - this.#renderPlanPreview(planContent, { append: true }); const contextUsage = this.#getPlanApprovalContextUsage(); const keepContextLabel = this.#formatKeepContextLabel(contextUsage); const keepContextDisabled = this.#isKeepContextDisabled(contextUsage); @@ -2255,7 +2280,8 @@ export class InteractiveMode implements InteractiveModeContext { : undefined; const helpText = slider ? `${this.#getPlanReviewHelpText()} ◂/▸ model` : this.#getPlanReviewHelpText(); - const choice = await this.showHookSelector( + const choice = await this.showPlanReview( + planContent, "Plan mode - next step", ["Approve and execute", "Approve and compact context", keepContextLabel, "Refine plan"], { @@ -2751,7 +2777,7 @@ export class InteractiveMode implements InteractiveModeContext { this.#omfgController.dispose(); this.#extensionUiController.clearExtensionTerminalInputListeners(); this.clearPinnedError(); - this.#planReviewContainer = undefined; + this.#hidePlanReview(); } handleClearCommand(): Promise { 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 85ae3f784..8c270905d 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -7,7 +7,6 @@ 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 { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { SILENT_ABORT_MARKER } from "@oh-my-pi/pi-coding-agent/session/messages"; -import { Text } from "@oh-my-pi/pi-tui"; import { formatNumber, TempDir } from "@oh-my-pi/pi-utils"; import { ModelRegistry } from "../src/config/model-registry"; import type { HookSelectorSlider } from "../src/modes/components/hook-selector"; @@ -112,7 +111,7 @@ describe("InteractiveMode plan review rendering", () => { resetSettingsForTest(); }); - it("appends each submitted plan review preview to preserve scrollback", async () => { + it("forwards each submitted plan to the review overlay", async () => { const planFilePath = "local://PLAN.md"; const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { getArtifactsDir: () => session.sessionManager.getArtifactsDir(), @@ -122,7 +121,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Refine plan"); + const review = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); await mode.handlePlanApproval({ planFilePath, @@ -131,12 +130,8 @@ describe("InteractiveMode plan review rendering", () => { finalPlanFilePath: "local://PLAN.md", }); - const firstPreview = mode.chatContainer.children.at(-1); - expect(firstPreview).toBeDefined(); - expect(firstPreview!.render(120).join("\n")).toContain("First plan"); + expect(review.mock.calls[0]?.[0]).toContain("First plan"); - const marker = new Text("MARKER", 0, 0); - mode.chatContainer.addChild(marker); await Bun.write(resolvedPlanPath, "# Second plan\n\nbeta"); await mode.handlePlanApproval({ @@ -146,14 +141,9 @@ describe("InteractiveMode plan review rendering", () => { finalPlanFilePath: "local://PLAN.md", }); - const secondPreview = mode.chatContainer.children.at(-1); - expect(secondPreview).toBeDefined(); - expect(secondPreview).not.toBe(firstPreview); - expect(mode.chatContainer.children.at(-2)).toBe(marker); - expect(mode.chatContainer.children.at(-3)).toBe(firstPreview); - expect(firstPreview!.render(120).join("\n")).toContain("First plan"); - expect(firstPreview!.render(120).join("\n")).not.toContain("Second plan"); - expect(secondPreview!.render(120).join("\n")).toContain("Second plan"); + // Each approval shows the current plan in the overlay, not a stale one. + expect(review.mock.calls[1]?.[0]).toContain("Second plan"); + expect(review.mock.calls[1]?.[0]).not.toContain("First plan"); }); it("offers approve-and-keep-context as a distinct plan approval path", async () => { @@ -167,7 +157,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 7320, contextWindow: 10000, percent: 73.2 }); - const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Refine plan"); + const selector = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); await mode.handlePlanApproval({ planFilePath, @@ -177,6 +167,7 @@ describe("InteractiveMode plan review rendering", () => { }); expect(selector).toHaveBeenCalledWith( + expect.any(String), "Plan mode - next step", [ "Approve and execute", @@ -244,7 +235,7 @@ describe("InteractiveMode plan review rendering", () => { percent: (tokens / contextWindow) * 100, }; }); - const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Refine plan"); + const selector = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); await mode.handlePlanApproval({ planFilePath, @@ -254,7 +245,7 @@ describe("InteractiveMode plan review rendering", () => { }); expect(contextSpy).toHaveBeenCalledWith({ contextWindow: executionModel.contextWindow }); - expect(selector.mock.calls[0]?.[1]).toEqual([ + expect(selector.mock.calls[0]?.[2]).toEqual([ "Approve and execute", "Approve and compact context", `Approve and keep context (~${compactNumber(tokens)} / ${compactNumber(executionModel.contextWindow)})`, @@ -273,7 +264,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 9600, contextWindow: 10000, percent: 96 }); - const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Refine plan"); + const selector = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); await mode.handlePlanApproval({ planFilePath, @@ -282,7 +273,7 @@ describe("InteractiveMode plan review rendering", () => { finalPlanFilePath: "local://APPROVED.md", }); - expect(selector.mock.calls[0]?.[2]).toEqual( + expect(selector.mock.calls[0]?.[3]).toEqual( expect.objectContaining({ disabledIndices: [2], }), @@ -300,7 +291,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 9500, contextWindow: 10000, percent: 95 }); - const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Refine plan"); + const selector = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); await mode.handlePlanApproval({ planFilePath, @@ -309,7 +300,7 @@ describe("InteractiveMode plan review rendering", () => { finalPlanFilePath: "local://APPROVED.md", }); - expect(selector.mock.calls[0]?.[2]).toEqual( + expect(selector.mock.calls[0]?.[3]).toEqual( expect.objectContaining({ disabledIndices: undefined, }), @@ -328,7 +319,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModePlanFilePath = planFilePath; // Post-compaction: tokens unknown until the next LLM response. vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: null, contextWindow: 200000, percent: null }); - const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Refine plan"); + const selector = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); await mode.handlePlanApproval({ planFilePath, @@ -338,6 +329,7 @@ describe("InteractiveMode plan review rendering", () => { }); expect(selector).toHaveBeenCalledWith( + expect.any(String), "Plan mode - next step", ["Approve and execute", "Approve and compact context", "Approve and keep context", "Refine plan"], expect.any(Object), @@ -361,7 +353,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: null, contextWindow: 200000, percent: null }); - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and keep context"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and keep context"); const clear = vi.spyOn(mode, "handleClearCommand").mockResolvedValue(); const prompt = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); @@ -390,7 +382,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and execute"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and execute"); const clear = vi.spyOn(mode, "handleClearCommand").mockResolvedValue(); const prompt = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); @@ -443,8 +435,8 @@ describe("InteractiveMode plan review rendering", () => { vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); let observedSegments: string[] = []; - vi.spyOn(mode, "showHookSelector").mockImplementation( - async (_title, _options, _dialogOptions, extra?: { slider?: HookSelectorSlider }) => { + vi.spyOn(mode, "showPlanReview").mockImplementation( + async (_planContent, _title, _options, _dialogOptions, extra?: { slider?: HookSelectorSlider }) => { const slider = extra?.slider; expect(slider).toBeDefined(); observedSegments = slider!.segments.map(segment => segment.label); @@ -480,7 +472,7 @@ describe("InteractiveMode plan review rendering", () => { await mode.handlePlanModeCommand(); - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and execute"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and execute"); vi.spyOn(mode, "handleClearCommand").mockResolvedValue(); vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); @@ -513,7 +505,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and compact context"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and compact context"); const compactSpy = vi.spyOn(mode, "handleCompactCommand").mockResolvedValue("ok"); const markSentSpy = vi.spyOn(session, "markPlanReferenceSent"); const promptSpy = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); @@ -556,7 +548,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and compact context"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and compact context"); vi.spyOn(mode, "handleCompactCommand").mockResolvedValue("cancelled"); const showWarningSpy = vi.spyOn(mode, "showWarning"); const setPlanRefSpy = vi.spyOn(session, "setPlanReferencePath"); @@ -599,7 +591,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and compact context"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and compact context"); vi.spyOn(mode, "handleCompactCommand").mockResolvedValue("failed"); const markSentSpy = vi.spyOn(session, "markPlanReferenceSent"); const promptSpy = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); @@ -632,7 +624,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and compact context"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and compact context"); vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); const setPlanRefSpy = vi.spyOn(session, "setPlanReferencePath"); @@ -685,7 +677,7 @@ describe("InteractiveMode plan review rendering", () => { mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and compact context"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and compact context"); if (compactOutcome === "throw") { vi.spyOn(mode, "handleCompactCommand").mockRejectedValue(throwError ?? new Error("compact boom")); } else { @@ -743,7 +735,7 @@ describe("InteractiveMode plan review rendering", () => { await Bun.write(resolvedPlanPath, "# Plan\n\nBody."); mode.planModeEnabled = true; mode.planModePlanFilePath = planFilePath; - vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and execute"); + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and execute"); const markSpy = vi.spyOn(session, "markPlanCompactAbortPending"); vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); @@ -771,7 +763,7 @@ describe("InteractiveMode plan review rendering", () => { expect(session.getPlanModeState()?.planFilePath).toBe(planFilePath); vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: null, contextWindow: 200000, percent: null }); - const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and keep context"); + const selector = vi.spyOn(mode, "showPlanReview").mockResolvedValue("Approve and keep context"); const showError = vi.spyOn(mode, "showError"); vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); diff --git a/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts b/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts new file mode 100644 index 000000000..49c2cba61 --- /dev/null +++ b/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts @@ -0,0 +1,195 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; +import { stripVTControlCharacters } from "node:util"; +import { KeybindingsManager } from "@oh-my-pi/pi-coding-agent/config/keybindings"; +import type { HookSelectorSlider } from "@oh-my-pi/pi-coding-agent/modes/components/hook-selector"; +import { PlanReviewOverlay } from "@oh-my-pi/pi-coding-agent/modes/components/plan-review-overlay"; +import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { setKeybindings } from "@oh-my-pi/pi-tui"; + +const UP = "\x1b[A"; +const DOWN = "\x1b[B"; +const LEFT = "\x1b[D"; +const RIGHT = "\x1b[C"; +const ENTER = "\r"; +const CANCEL = "\x07"; // ctrl+g, remapped to tui.select.cancel below + +let darkTheme = await getThemeByName("dark"); + +function render(component: PlanReviewOverlay): string { + return stripVTControlCharacters(component.render(80).join("\n")); +} + +const APPROVAL_OPTIONS = [ + "Approve and execute", + "Approve and compact context", + "Approve and keep context", + "Refine plan", +]; + +describe("PlanReviewOverlay", () => { + beforeAll(async () => { + darkTheme = await getThemeByName("dark"); + if (!darkTheme) throw new Error("Failed to load dark theme"); + }); + + beforeEach(() => { + setThemeInstance(darkTheme!); + setKeybindings(KeybindingsManager.inMemory({ "tui.select.cancel": "ctrl+g" })); + }); + + afterEach(() => { + setKeybindings(KeybindingsManager.inMemory()); + vi.restoreAllMocks(); + }); + + it("renders the plan body, prompt, options and footer inside one outlined box", () => { + const overlay = new PlanReviewOverlay( + "# My Plan\n\nstep one then step two", + { promptTitle: "Plan mode - next step", options: APPROVAL_OPTIONS, helpText: "esc cancel" }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const out = render(overlay); + expect(out).toContain("Plan Review"); + expect(out).toContain("My Plan"); + expect(out).toContain("step one then step two"); + expect(out).toContain("Plan mode - next step"); + for (const option of APPROVAL_OPTIONS) expect(out).toContain(option); + expect(out).toContain("esc cancel"); + // Outlined like the /copy overlay. + expect(out).toContain("┌"); + expect(out).toContain("│"); + expect(out).toContain("└"); + }); + + it("confirms the highlighted option on Enter", () => { + const onPick = vi.fn(); + const overlay = new PlanReviewOverlay( + "plan", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick, onCancel: vi.fn() }, + ); + overlay.handleInput(ENTER); + expect(onPick).toHaveBeenCalledTimes(1); + expect(onPick).toHaveBeenCalledWith("Approve and execute"); + }); + + it("moves the option cursor with up/down and confirms the new target", () => { + const onPick = vi.fn(); + const overlay = new PlanReviewOverlay( + "plan", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick, onCancel: vi.fn() }, + ); + overlay.handleInput(DOWN); + overlay.handleInput(ENTER); + expect(onPick).toHaveBeenCalledWith("Approve and compact context"); + + onPick.mockClear(); + overlay.handleInput(UP); + overlay.handleInput(ENTER); + expect(onPick).toHaveBeenCalledWith("Approve and execute"); + }); + + it("skips disabled options and never confirms them", () => { + const onPick = vi.fn(); + // Disable index 2 ("Approve and keep context"). + const overlay = new PlanReviewOverlay( + "plan", + { promptTitle: "next", options: APPROVAL_OPTIONS, disabledIndices: [2] }, + { onPick, onCancel: vi.fn() }, + ); + // 0 -> 1 -> (skip 2) -> 3. + overlay.handleInput(DOWN); + overlay.handleInput(DOWN); + overlay.handleInput(ENTER); + expect(onPick).toHaveBeenCalledTimes(1); + expect(onPick).toHaveBeenCalledWith("Refine plan"); + }); + + it("cancels on the cancel key", () => { + const onCancel = vi.fn(); + const overlay = new PlanReviewOverlay( + "plan", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel }, + ); + overlay.handleInput(CANCEL); + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("drives the model-tier slider with left/right without changing the option cursor", () => { + const changes: number[] = []; + const slider: HookSelectorSlider = { + caption: "continue with", + index: 0, + segments: [{ label: "default" }, { label: "slow", detail: "opus" }], + onChange: index => changes.push(index), + }; + const onPick = vi.fn(); + const overlay = new PlanReviewOverlay( + "plan", + { promptTitle: "next", options: APPROVAL_OPTIONS, slider }, + { onPick, onCancel: vi.fn() }, + ); + overlay.handleInput(RIGHT); + expect(changes).toEqual([1]); + // Clamped at the right edge. + overlay.handleInput(RIGHT); + expect(changes).toEqual([1]); + overlay.handleInput(LEFT); + expect(changes).toEqual([1, 0]); + + // The slider must not have moved the option cursor. + overlay.handleInput(ENTER); + expect(onPick).toHaveBeenCalledWith("Approve and execute"); + }); + + it("invokes the external-editor callback on its key", () => { + setKeybindings(KeybindingsManager.inMemory({ "tui.select.cancel": "ctrl+g", "app.editor.external": "ctrl+e" })); + const onExternalEditor = vi.fn(); + const overlay = new PlanReviewOverlay( + "plan", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn(), onExternalEditor }, + ); + overlay.handleInput("\x05"); // ctrl+e + expect(onExternalEditor).toHaveBeenCalledTimes(1); + }); + + it("scrolls a long plan to bottom and back to top", () => { + const longPlan = Array.from({ length: 200 }, (_, i) => `para ${i}`).join("\n\n"); + const overlay = new PlanReviewOverlay( + longPlan, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const top = render(overlay); + expect(top).toContain("para 0"); + expect(top).not.toContain("para 199"); + + overlay.handleInput("G"); + const bottom = render(overlay); + expect(bottom).toContain("para 199"); + expect(bottom).not.toContain("para 0"); + + overlay.handleInput("g"); + const backToTop = render(overlay); + expect(backToTop).toContain("para 0"); + expect(backToTop).not.toContain("para 199"); + }); + + it("swaps the displayed plan and resets scroll on setPlanContent", () => { + const longPlan = Array.from({ length: 200 }, (_, i) => `para ${i}`).join("\n\n"); + const overlay = new PlanReviewOverlay( + longPlan, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + overlay.handleInput("G"); // scroll away from the top + overlay.setPlanContent("# Fresh plan\n\nbrand new body"); + const out = render(overlay); + expect(out).toContain("Fresh plan"); + expect(out).toContain("brand new body"); + expect(out).not.toContain("para 199"); + }); +});