From a4f67eb65e49055833dd0a9bf95a7a00cc5d1178 Mon Sep 17 00:00:00 2001 From: Colin Mason Date: Thu, 15 Oct 2026 14:50:03 -0400 Subject: [PATCH] fix(coding-agent): close own custom ui overlay (#10999) done() from ctx.ui.custom({ overlay: true }) closed the overlay with ui.hideOverlay(), which removes the topmost overlay instead of the one that called done(). With a shell overlay open under a dialog overlay from another extension, the shell's exit removed the dialog: the finished shell overlay stayed on screen ignoring input and the dialog's promise never resolved. Hide the overlay through its own handle. closes #10648 --- .../src/modes/interactive/interactive-mode.ts | 8 +- .../test/interactive-mode-status.test.ts | 78 +++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 132c44b18..ce2398475 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -2989,12 +2989,14 @@ export class InteractiveMode { return new Promise((resolve, reject) => { let component: Component & { dispose?(): void }; + let overlayHandle: OverlayHandle | undefined; let closed = false; const close = (result: T) => { if (closed) return; closed = true; - if (isOverlay) this.ui.hideOverlay(); + // Remove this overlay, not the topmost one: another overlay may have opened above it. + if (isOverlay) overlayHandle?.hide(); else restoreEditor(); // Note: both branches above already call requestRender resolve(result); @@ -3023,9 +3025,9 @@ export class InteractiveMode { const w = (component as { width?: number }).width; return w ? { width: w } : undefined; }; - const handle = this.ui.showOverlay(component, resolveOptions()); + overlayHandle = this.ui.showOverlay(component, resolveOptions()); // Expose handle to caller for visibility control - options?.onHandle?.(handle); + options?.onHandle?.(overlayHandle); } else { this.disposeActiveSelector(); this.editorContainer.clear(); diff --git a/packages/coding-agent/test/interactive-mode-status.test.ts b/packages/coding-agent/test/interactive-mode-status.test.ts index 103b23d33..4351fc05a 100644 --- a/packages/coding-agent/test/interactive-mode-status.test.ts +++ b/packages/coding-agent/test/interactive-mode-status.test.ts @@ -300,6 +300,84 @@ describe("InteractiveMode.showExtensionCustom", () => { ui.stop(); } }); + + // Regression test for #10648: closing a lower overlay removed the topmost overlay instead. + test.each([ + { lowerKind: "capturing", nonCapturing: false }, + { lowerKind: "non-capturing", nonCapturing: true }, + ])( + "closing a lower $lowerKind overlay custom UI removes its own overlay, not the topmost one", + async ({ nonCapturing }) => { + const terminal = new VirtualTerminal(80, 24); + const ui: TUI = new TuiMainScreen(terminal); + const editorContainer = new Container(); + const editor = new TestFocusableComponent("EDITOR"); + const lower = new TestFocusableComponent("LOWER"); + const upper = new TestFocusableComponent("UPPER"); + let closeLower: (value: string) => void = () => { + throw new Error("closeLower was not initialized"); + }; + let closeUpper: (value: string) => void = () => { + throw new Error("closeUpper was not initialized"); + }; + const fakeThis = { + editor, + editorContainer, + keybindings: {}, + ui, + disposeActiveSelector: vi.fn(), + }; + const showExtensionCustom = ( + factory: (tui: TUI, theme: unknown, keybindings: unknown, done: (result: T) => void) => Component, + options?: { overlay?: boolean; overlayOptions?: { nonCapturing?: boolean } }, + ): Promise => + (InteractiveMode as any).prototype.showExtensionCustom.call(fakeThis, factory, options) as Promise; + + editorContainer.addChild(editor); + ui.addChild(editorContainer); + ui.setFocus(editor); + ui.start(); + try { + const lowerPromise = showExtensionCustom( + (_tui, _theme, _keybindings, done) => { + closeLower = done; + return lower; + }, + { overlay: true, overlayOptions: { nonCapturing } }, + ); + await flushTui(ui, terminal); + const upperPromise = showExtensionCustom( + (_tui, _theme, _keybindings, done) => { + closeUpper = done; + return upper; + }, + { overlay: true }, + ); + await flushTui(ui, terminal); + expect(upper.focused).toBe(true); + + closeLower("lower closed"); + await expect(lowerPromise).resolves.toBe("lower closed"); + await flushTui(ui, terminal); + + const viewport = terminal.getViewport().join("\n"); + expect(viewport).toContain("UPPER"); + expect(viewport).not.toContain("LOWER"); + terminal.sendInput("x"); + await flushTui(ui, terminal); + expect(upper.inputs).toEqual(["x"]); + expect(lower.inputs).toEqual([]); + + closeUpper("upper closed"); + await expect(upperPromise).resolves.toBe("upper closed"); + await flushTui(ui, terminal); + expect(ui.hasOverlay()).toBe(false); + expect(editor.focused).toBe(true); + } finally { + ui.stop(); + } + }, + ); }); describe("InteractiveMode.createExtensionUIContext addAutocompleteProvider", () => {