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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<number>;
|
||||
#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),
|
||||
];
|
||||
}
|
||||
}
|
||||
@@ -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<string | undefined> {
|
||||
this.#hidePlanReview();
|
||||
const { promise, resolve } = Promise.withResolvers<string | undefined>();
|
||||
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<void> {
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user