mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
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
This commit is contained in:
parent
9df819de39
commit
38847563f4
4 changed files with 243 additions and 124 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string | undefined> {
|
||||
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) => {
|
||||
|
|
|
|||
|
|
@ -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<typeof vi.fn> };
|
||||
disposeActiveSelector: () => void;
|
||||
toggleToolOutputExpansion: () => void;
|
||||
};
|
||||
|
||||
type DialogMethods = {
|
||||
showExtensionConfirm(title: string, message: string, opts?: { signal?: AbortSignal }): Promise<boolean>;
|
||||
showExtensionCustom<T>(
|
||||
factory: (tui: unknown, theme: unknown, keybindings: unknown, done: (result: T) => void) => Component,
|
||||
): Promise<T>;
|
||||
};
|
||||
|
||||
function createContext(): DialogContext & DialogMethods {
|
||||
const prototype = InteractiveMode.prototype as unknown as Record<string, unknown>;
|
||||
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<string>((_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<boolean> | undefined;
|
||||
let done: ((result: string) => void) | undefined;
|
||||
const custom = context.showExtensionCustom<string>((_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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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 = <T>(
|
||||
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 = <T>(
|
||||
factory: (tui: TUI, theme: unknown, keybindings: unknown, done: (result: T) => void) => Component,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue