From 38847563f4b0865b145136d8e82566643ded685e Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Tue, 20 Oct 2026 20:40:34 +0200 Subject: [PATCH] fix(coding-agent): stack concurrent extension dialogs instead of replacing them A second select/confirm/input/editor dialog or inline custom() component replaced the visible one and left its promise pending forever. Dialogs in the editor slot now form a stack: closing the visible dialog shows the previous one again, and dialogs opened from inside a custom component work. closes #11166 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/modes/interactive/interactive-mode.ts | 203 +++++++----------- ...interactive-mode-extension-dialogs.test.ts | 152 +++++++++++++ .../test/interactive-mode-status.test.ts | 11 + 4 files changed, 243 insertions(+), 124 deletions(-) create mode 100644 packages/coding-agent/test/interactive-mode-extension-dialogs.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 65026a4f2..6b6687d8c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -17,6 +17,7 @@ - Fixed `pi update` failing with "Managed install marker is missing or invalid" when `managed-install.json` starts with a UTF-8 BOM, which Windows PowerShell 5.1 adds when it rewrites the file. - Fixed HTML export showing literal asterisks for Markdown bold and italic next to CJK punctuation ([#3353](https://github.com/earendil-works/pi/issues/3353), [#10154](https://github.com/earendil-works/pi/issues/10154)) - Fixed the terminal staying in mouse-tracking, focus-reporting, Kitty keyboard, and alternate-screen modes when pi exits without its normal terminal teardown, which flooded the shell prompt with escape sequences ([#11170](https://github.com/earendil-works/pi/issues/11170)) +- Fixed concurrent extension `select`/`confirm`/`input`/`editor` dialogs and inline `custom()` components in interactive mode: a second dialog replaced the first and the first promise never settled; closing the visible dialog now shows the previous one again ([#11166](https://github.com/earendil-works/pi/issues/11166), [#9909](https://github.com/earendil-works/pi/issues/9909)) ## [1.3.0] - 2026-10-15 diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index ce2398475..e24d150be 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -264,6 +264,8 @@ class BuiltInHeader extends ExpandableText { } } +type ExtensionDialogComponent = Component & { dispose?(): void }; + type CompactionQueuedMessage = { text: string; mode: "steer" | "followUp"; @@ -558,9 +560,9 @@ export class InteractiveMode { private installChangeWarningShown = false; // Extension UI state - private extensionSelector: ExtensionSelectorComponent | undefined = undefined; - private extensionInput: ExtensionInputComponent | undefined = undefined; - private extensionEditor: ExtensionEditorComponent | undefined = undefined; + // Extension dialogs shown in the editor slot (select, confirm, input, editor, inline custom). + // The last entry is visible; see pushExtensionDialog(). + private extensionDialogs: Array<{ component: ExtensionDialogComponent; blocked: BlockedStatus | undefined }> = []; private extensionTerminalInputSubscriptions = new Set<{ handler: (data: string) => { consume?: boolean; data?: string } | undefined; unsubscribe: () => void; @@ -2450,14 +2452,12 @@ export class InteractiveMode { } private resetExtensionUI(): void { - if (this.extensionSelector) { - this.hideExtensionSelector(); - } - if (this.extensionInput) { - this.hideExtensionInput(); - } - if (this.extensionEditor) { - this.hideExtensionEditor(); + if (this.extensionDialogs.length > 0) { + for (const dialog of this.extensionDialogs) { + dialog.component.dispose?.(); + } + this.extensionDialogs = []; + this.showTopExtensionDialog(); } this.ui.hideOverlay(); this.clearExtensionTerminalInputListeners(); @@ -2684,6 +2684,36 @@ export class InteractiveMode { }; } + /** + * Show an extension dialog in the editor slot. + * Dialogs form a stack: a new dialog hides the visible one, and closing the visible dialog shows + * the previous one again. Every dialog stays answerable, so no dialog promise is left pending, + * and dialogs opened from inside another dialog (for example a confirm from a custom component) work. + */ + private pushExtensionDialog(component: ExtensionDialogComponent, blocked?: BlockedStatus): void { + this.disposeActiveSelector(); + this.extensionDialogs.push({ component, blocked }); + this.showTopExtensionDialog(); + } + + /** Remove an extension dialog. If it was visible, show the previous dialog or the editor. */ + private removeExtensionDialog(component: ExtensionDialogComponent): void { + const index = this.extensionDialogs.findIndex((dialog) => dialog.component === component); + if (index === -1) return; + this.extensionDialogs.splice(index, 1); + if (index === this.extensionDialogs.length) this.showTopExtensionDialog(); + } + + private showTopExtensionDialog(): void { + const top = this.extensionDialogs.at(-1); + const component = top?.component ?? this.editor; + this.editorContainer.clear(); + this.editorContainer.addChild(component); + this.ui.setFocus(component); + this.programStatus.setBlocked("extension-dialog", top?.blocked); + this.ui.requestRender(); + } + /** * Show a selector for extensions. */ @@ -2699,51 +2729,26 @@ export class InteractiveMode { return; } - const onAbort = () => { - this.hideExtensionSelector(); - resolve(undefined); + const finish = (option: string | undefined) => { + opts?.signal?.removeEventListener("abort", onAbort); + selector.dispose(); + this.removeExtensionDialog(selector); + resolve(option); }; + const onAbort = () => finish(undefined); opts?.signal?.addEventListener("abort", onAbort, { once: true }); - this.extensionSelector = new ExtensionSelectorComponent( + const selector = new ExtensionSelectorComponent( title, options, - (option) => { - opts?.signal?.removeEventListener("abort", onAbort); - this.hideExtensionSelector(); - resolve(option); - }, - () => { - opts?.signal?.removeEventListener("abort", onAbort); - this.hideExtensionSelector(); - resolve(undefined); - }, + (option) => finish(option), + () => finish(undefined), { tui: this.ui, timeout: opts?.timeout, onToggleToolsExpanded: () => this.toggleToolOutputExpansion() }, ); - - this.disposeActiveSelector(); - this.editorContainer.clear(); - this.editorContainer.addChild(this.extensionSelector); - this.ui.setFocus(this.extensionSelector); - // Extension dialogs share the editor slot: opening one replaces the status of a displaced one. - this.programStatus.setBlocked("extension-dialog", blocked); - this.ui.requestRender(); + this.pushExtensionDialog(selector, blocked); }); } - /** - * Hide the extension selector. - */ - private hideExtensionSelector(): void { - this.extensionSelector?.dispose(); - this.editorContainer.clear(); - this.editorContainer.addChild(this.editor); - this.extensionSelector = undefined; - this.programStatus.setBlocked("extension-dialog", undefined); - this.ui.setFocus(this.editor); - this.ui.requestRender(); - } - /** * Show a confirmation dialog for extensions. */ @@ -2781,93 +2786,49 @@ export class InteractiveMode { return; } - const onAbort = () => { - this.hideExtensionInput(); - resolve(undefined); + const finish = (value: string | undefined) => { + opts?.signal?.removeEventListener("abort", onAbort); + input.dispose(); + this.removeExtensionDialog(input); + resolve(value); }; + const onAbort = () => finish(undefined); opts?.signal?.addEventListener("abort", onAbort, { once: true }); - this.extensionInput = new ExtensionInputComponent( + const input = new ExtensionInputComponent( title, placeholder, - (value) => { - opts?.signal?.removeEventListener("abort", onAbort); - this.hideExtensionInput(); - resolve(value); - }, - () => { - opts?.signal?.removeEventListener("abort", onAbort); - this.hideExtensionInput(); - resolve(undefined); - }, + (value) => finish(value), + () => finish(undefined), { tui: this.ui, timeout: opts?.timeout }, ); - - this.disposeActiveSelector(); - this.editorContainer.clear(); - this.editorContainer.addChild(this.extensionInput); - this.ui.setFocus(this.extensionInput); - this.programStatus.setBlocked("extension-dialog", { kind: "question", message: title }); - this.ui.requestRender(); + this.pushExtensionDialog(input, { kind: "question", message: title }); }); } - /** - * Hide the extension input. - */ - private hideExtensionInput(): void { - this.extensionInput?.dispose(); - this.editorContainer.clear(); - this.editorContainer.addChild(this.editor); - this.extensionInput = undefined; - this.programStatus.setBlocked("extension-dialog", undefined); - this.ui.setFocus(this.editor); - this.ui.requestRender(); - } - /** * Show a multi-line editor for extensions (with Ctrl+G support). */ private showExtensionEditor(title: string, prefill?: string): Promise { return new Promise((resolve) => { - this.extensionEditor = new ExtensionEditorComponent( + const finish = (value: string | undefined) => { + this.removeExtensionDialog(editor); + resolve(value); + }; + const editor = new ExtensionEditorComponent( this.ui, this.keybindings, title, prefill, - (value) => { - this.hideExtensionEditor(); - resolve(value); - }, - () => { - this.hideExtensionEditor(); - resolve(undefined); - }, + (value) => finish(value), + () => finish(undefined), undefined, this.settingsManager.getExternalEditorCommand(), ); - - this.disposeActiveSelector(); - this.editorContainer.clear(); - this.editorContainer.addChild(this.extensionEditor); - this.ui.setFocus(this.extensionEditor); - this.programStatus.setBlocked("extension-dialog", { kind: "question", message: title }); - this.ui.requestRender(); + this.pushExtensionDialog(editor, { kind: "question", message: title }); }); } - /** - * Hide the extension editor. - */ - private hideExtensionEditor(): void { - this.editorContainer.clear(); - this.editorContainer.addChild(this.editor); - this.extensionEditor = undefined; - this.programStatus.setBlocked("extension-dialog", undefined); - this.ui.setFocus(this.editor); - this.ui.requestRender(); - } - /** * Set a custom editor component from an extension. * Pass undefined to restore the default editor. @@ -2979,19 +2940,17 @@ export class InteractiveMode { const savedText = this.editor.getText(); const isOverlay = options?.overlay ?? false; - const restoreEditor = () => { - this.editorContainer.clear(); - this.editorContainer.addChild(this.editor); - this.editor.setText(savedText); - this.ui.setFocus(this.editor); - this.ui.requestRender(); - }; - return new Promise((resolve, reject) => { - let component: Component & { dispose?(): void }; + let component: ExtensionDialogComponent | undefined; let overlayHandle: OverlayHandle | undefined; let closed = false; + const restoreEditor = () => { + if (component) this.removeExtensionDialog(component); + this.editor.setText(savedText); + this.ui.requestRender(); + }; + const close = (result: T) => { if (closed) return; closed = true; @@ -3022,18 +2981,14 @@ export class InteractiveMode { return opts; } // Fallback: use component's width property if available - const w = (component as { width?: number }).width; + const w = (c as { width?: number }).width; return w ? { width: w } : undefined; }; - overlayHandle = this.ui.showOverlay(component, resolveOptions()); + overlayHandle = this.ui.showOverlay(c, resolveOptions()); // Expose handle to caller for visibility control options?.onHandle?.(overlayHandle); } else { - this.disposeActiveSelector(); - this.editorContainer.clear(); - this.editorContainer.addChild(component); - this.ui.setFocus(component); - this.ui.requestRender(); + this.pushExtensionDialog(c); } }) .catch((err) => { diff --git a/packages/coding-agent/test/interactive-mode-extension-dialogs.test.ts b/packages/coding-agent/test/interactive-mode-extension-dialogs.test.ts new file mode 100644 index 000000000..d296ff2b4 --- /dev/null +++ b/packages/coding-agent/test/interactive-mode-extension-dialogs.test.ts @@ -0,0 +1,152 @@ +import { type Component, Container, Text } from "@earendil-works/pi-tui"; +import { beforeAll, describe, expect, it, vi } from "vitest"; +import { InteractiveMode } from "../src/modes/interactive/interactive-mode.ts"; +import { initTheme } from "../src/modes/interactive/theme/theme.ts"; + +type DialogContext = { + extensionDialogs: Component[]; + editorContainer: Container; + editor: Component & { getText(): string; setText(text: string): void }; + focused: Component | undefined; + ui: { setFocus: (component: Component) => void; requestRender: () => void }; + programStatus: { setBlocked: ReturnType }; + disposeActiveSelector: () => void; + toggleToolOutputExpansion: () => void; +}; + +type DialogMethods = { + showExtensionConfirm(title: string, message: string, opts?: { signal?: AbortSignal }): Promise; + showExtensionCustom( + factory: (tui: unknown, theme: unknown, keybindings: unknown, done: (result: T) => void) => Component, + ): Promise; +}; + +function createContext(): DialogContext & DialogMethods { + const prototype = InteractiveMode.prototype as unknown as Record; + let editorText = ""; + const editor = Object.assign(new Text("editor"), { + getText: () => editorText, + setText: (text: string) => { + editorText = text; + }, + }); + const context: DialogContext = { + extensionDialogs: [], + editorContainer: new Container(), + editor, + focused: undefined, + ui: { + setFocus: (component) => { + context.focused = component; + }, + requestRender: vi.fn(), + }, + programStatus: { setBlocked: vi.fn() }, + disposeActiveSelector: vi.fn(), + toggleToolOutputExpansion: vi.fn(), + }; + return new Proxy(context, { + get(target, key, receiver) { + if (key in target) return Reflect.get(target, key, receiver); + const value = prototype[key as string]; + return typeof value === "function" ? value.bind(receiver) : value; + }, + }) as DialogContext & DialogMethods; +} + +function visible(context: DialogContext): string { + expect(context.editorContainer.children).toEqual([context.focused]); + return (context.focused?.render(80) ?? []).join("\n"); +} + +describe("InteractiveMode extension dialogs", () => { + beforeAll(() => initTheme("dark")); + + // Regression for #11166: a second dialog replaced the first one and the first promise never settled. + it("shows the earlier confirm again after the later one is answered", async () => { + const context = createContext(); + const first = context.showExtensionConfirm("First", "first dialog"); + const second = context.showExtensionConfirm("Second", "second dialog"); + + expect(visible(context)).toContain("Second"); + expect(context.programStatus.setBlocked).toHaveBeenLastCalledWith("extension-dialog", { + kind: "permission", + message: "Second", + }); + context.focused?.handleInput?.("\x1b"); + await expect(second).resolves.toBe(false); + + expect(visible(context)).toContain("First"); + expect(context.programStatus.setBlocked).toHaveBeenLastCalledWith("extension-dialog", { + kind: "permission", + message: "First", + }); + context.focused?.handleInput?.("\r"); + await expect(first).resolves.toBe(true); + expect(context.focused).toBe(context.editor); + expect(context.programStatus.setBlocked).toHaveBeenLastCalledWith("extension-dialog", undefined); + }); + + it("removes a hidden dialog when its signal aborts", async () => { + const context = createContext(); + const controller = new AbortController(); + const first = context.showExtensionConfirm("First", "first dialog", { signal: controller.signal }); + const second = context.showExtensionConfirm("Second", "second dialog"); + + controller.abort(); + await expect(first).resolves.toBe(false); + expect(visible(context)).toContain("Second"); + + context.focused?.handleInput?.("\r"); + await expect(second).resolves.toBe(true); + expect(context.focused).toBe(context.editor); + }); + + // Regression for #9909: a second inline custom component replaced the first one and the first promise never settled. + it("stacks concurrent inline custom components", async () => { + const context = createContext(); + const dones: Array<(result: string) => void> = []; + const open = (label: string) => + context.showExtensionCustom((_tui, _theme, _keybindings, done) => { + dones.push(done); + return new Text(label); + }); + const first = open("custom one"); + const second = open("custom two"); + await vi.waitFor(() => expect(dones).toHaveLength(2)); + await vi.waitFor(() => expect(visible(context)).toContain("custom two")); + + dones[1]("two"); + await expect(second).resolves.toBe("two"); + expect(visible(context)).toContain("custom one"); + + dones[0]("one"); + await expect(first).resolves.toBe("one"); + expect(context.focused).toBe(context.editor); + }); + + it("shows a confirm opened from an inline custom component and then restores the component", async () => { + const context = createContext(); + let confirmed: Promise | undefined; + let done: ((result: string) => void) | undefined; + const custom = context.showExtensionCustom((_tui, _theme, _keybindings, finish) => { + done = finish; + return Object.assign(new Text("custom form"), { + handleInput: () => { + confirmed = context.showExtensionConfirm("Discard?", "discard changes"); + }, + }); + }); + await vi.waitFor(() => expect(visible(context)).toContain("custom form")); + + context.focused?.handleInput?.("x"); + expect(visible(context)).toContain("Discard?"); + context.focused?.handleInput?.("\x1b"); + await expect(confirmed).resolves.toBe(false); + + expect(visible(context)).toContain("custom form"); + done?.("kept"); + await expect(custom).resolves.toBe("kept"); + expect(context.focused).toBe(context.editor); + }); +}); diff --git a/packages/coding-agent/test/interactive-mode-status.test.ts b/packages/coding-agent/test/interactive-mode-status.test.ts index 4351fc05a..72de1fa96 100644 --- a/packages/coding-agent/test/interactive-mode-status.test.ts +++ b/packages/coding-agent/test/interactive-mode-status.test.ts @@ -234,6 +234,15 @@ describe("InteractiveMode.showExtensionCustom", () => { initTheme("dark"); }); + // Inline custom components go through the extension dialog stack. + const extensionDialogState = () => ({ + extensionDialogs: [], + programStatus: { setBlocked: vi.fn() }, + pushExtensionDialog: (InteractiveMode as any).prototype.pushExtensionDialog, + removeExtensionDialog: (InteractiveMode as any).prototype.removeExtensionDialog, + showTopExtensionDialog: (InteractiveMode as any).prototype.showTopExtensionDialog, + }); + test("overlay custom UI reclaims input after non-overlay custom UI closes", async () => { const terminal = new VirtualTerminal(80, 24); const ui: TUI = new TuiMainScreen(terminal); @@ -254,6 +263,7 @@ describe("InteractiveMode.showExtensionCustom", () => { keybindings: {}, ui, disposeActiveSelector: vi.fn(), + ...extensionDialogState(), }; const showExtensionCustom = ( factory: (tui: TUI, theme: unknown, keybindings: unknown, done: (result: T) => void) => Component, @@ -326,6 +336,7 @@ describe("InteractiveMode.showExtensionCustom", () => { keybindings: {}, ui, disposeActiveSelector: vi.fn(), + ...extensionDialogState(), }; const showExtensionCustom = ( factory: (tui: TUI, theme: unknown, keybindings: unknown, done: (result: T) => void) => Component,