Skip to content

Commit 60bdaa8

Browse files
committed
fix(tui): keep suspended overlays live when a gate preempts them
Suspend was a dismiss: it ran onCancel/onDispose and destroyed the list, so restore painted a dead widget and MCP could steal the host from the arriving gate. The preemptable-kind set also used command aliases, so /model and /connect never yielded.
1 parent 88a9c41 commit 60bdaa8

4 files changed

Lines changed: 179 additions & 20 deletions

File tree

‎src/tui/allow-once-reprompt.test.ts‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,15 @@ import { createAppShell } from "./shell/index.js";
3434
import type { AppShell } from "./shell/internals.js";
3535
import {
3636
acceptOverlaySelection,
37+
closeInsetOverlay,
3738
openListOverlay,
3839
} from "./shell/overlay-host.js";
3940
import { moveOverlaySelection } from "./shell/overlay-list.js";
4041
import { streamRowCount } from "./shell/transcript.js";
4142
import { wireGates } from "./gate-wire.js";
4243
import type { PermissionGateEvent } from "./gate-events.js";
4344
import { createGateRequestApproval } from "./request-approval.js";
45+
import { openAddProviderOverlay, openModelPickerOverlay } from "./overlays.js";
4446

4547
const shellCall = (command: string): ToolCall => ({
4648
id: "c",
@@ -475,3 +477,148 @@ describe("CL-8792 elapsed: pending tool row freezes while a gate is outstanding
475477
);
476478
});
477479
});
480+
481+
describe("CL-8792 overlay host: suspend preserves the surface instead of dismissing it", () => {
482+
test("suspend does not fire onCancel or onDispose", async () => {
483+
await withWiredWorld(async ({ shell, emitter }) => {
484+
const events: string[] = [];
485+
openListOverlay(shell, {
486+
kind: "help",
487+
title: "Slash",
488+
items: ["Help"],
489+
onCancel: () => events.push("cancel"),
490+
onDispose: () => events.push("dispose"),
491+
});
492+
expect(shell.overlayKind).toBe("help");
493+
494+
let resolved: unknown;
495+
emitter.emit("permission.gate", {
496+
id: "req-suspend-hooks",
497+
request: destructiveRequest("rm -rf /tmp/cl8792-suspend-hooks"),
498+
resolve: (outcome: unknown) => {
499+
resolved = outcome;
500+
},
501+
});
502+
expect(shell.overlayKind).toBe("permissions");
503+
expect(events).toEqual([]);
504+
505+
acceptChoice(shell, 1);
506+
expect(resolved).toEqual({ allow: true });
507+
expect(shell.overlayKind).toBe("help");
508+
expect(events).toEqual([]);
509+
510+
closeInsetOverlay(shell);
511+
expect(events).toEqual(["dispose", "cancel"]);
512+
});
513+
});
514+
515+
test("restored SelectRenderable is live and parented in overlayView.body", async () => {
516+
await withWiredWorld(async ({ shell, emitter }) => {
517+
openSlash(shell);
518+
const list = defined(shell.overlayList, "slash list");
519+
expect(list.select.isDestroyed).toBe(false);
520+
expect(list.select.parent).toBe(shell.overlayView.body);
521+
522+
let resolved: unknown;
523+
emitter.emit("permission.gate", {
524+
id: "req-restore-list",
525+
request: destructiveRequest("rm -rf /tmp/cl8792-restore-list"),
526+
resolve: (outcome: unknown) => {
527+
resolved = outcome;
528+
},
529+
});
530+
expect(shell.overlayKind).toBe("permissions");
531+
532+
acceptChoice(shell, 1);
533+
expect(resolved).toEqual({ allow: true });
534+
expect(shell.overlayKind).toBe("help");
535+
expect(shell.overlayList).toBe(list);
536+
expect(list.select.isDestroyed).toBe(false);
537+
expect(list.select.parent).toBe(shell.overlayView.body);
538+
expect(shell.overlayView.body.getChildren()).toContain(list.select);
539+
});
540+
});
541+
542+
test.each([
543+
{
544+
kind: "model_picker" as const,
545+
open: (shell: AppShell) =>
546+
openModelPickerOverlay(shell, { items: ["grok-3"] }),
547+
},
548+
{
549+
kind: "add_provider" as const,
550+
open: (shell: AppShell) =>
551+
openAddProviderOverlay(shell, {
552+
items: ["custom"],
553+
itemIds: ["custom"],
554+
}),
555+
},
556+
])(
557+
"$kind yields to a newly raised gate and returns after settle",
558+
async ({ kind, open }) => {
559+
await withWiredWorld(async ({ shell, emitter }) => {
560+
open(shell);
561+
expect(shell.overlayKind).toBe(kind);
562+
563+
let resolved: unknown;
564+
emitter.emit("permission.gate", {
565+
id: `req-yield-${kind}`,
566+
request: destructiveRequest(`rm -rf /tmp/cl8792-yield-${kind}`),
567+
resolve: (outcome: unknown) => {
568+
resolved = outcome;
569+
},
570+
});
571+
expect(shell.overlayKind).toBe("permissions");
572+
573+
acceptChoice(shell, 1);
574+
expect(resolved).toEqual({ allow: true });
575+
expect(shell.overlayKind).toBe(kind);
576+
});
577+
},
578+
);
579+
580+
test("MCP onCancel during suspend does not steal the host from a queued gate while a deferred slash occupies idle", async () => {
581+
await withWiredWorld(async ({ shell, emitter }) => {
582+
let cancelOpens = 0;
583+
openListOverlay(shell, {
584+
kind: "mcp",
585+
title: `remove stolen`,
586+
items: ["Remove stolen", "Cancel"],
587+
onCancel: () => {
588+
cancelOpens += 1;
589+
openListOverlay(shell, {
590+
kind: "mcp",
591+
title: "mcp",
592+
items: ["stolen-server"],
593+
});
594+
},
595+
});
596+
expect(shell.overlayKind).toBe("mcp");
597+
598+
openSlash(shell);
599+
expect(shell.overlayKind).toBe("mcp");
600+
601+
let resolved: unknown;
602+
emitter.emit("permission.gate", {
603+
id: "req-mcp-cancel-steal",
604+
request: destructiveRequest("rm -rf /tmp/cl8792-mcp-steal"),
605+
resolve: (outcome: unknown) => {
606+
resolved = outcome;
607+
},
608+
});
609+
expect(cancelOpens).toBe(0);
610+
expect(shell.overlayKind).toBe("permissions");
611+
expect(shell.overlayItems).toContain("Accept once");
612+
613+
acceptChoice(shell, 1);
614+
expect(resolved).toEqual({ allow: true });
615+
expect(cancelOpens).toBe(0);
616+
expect(shell.overlayKind).toBe("mcp");
617+
expect(shell.overlayItems).toEqual(["Remove stolen", "Cancel"]);
618+
619+
await Promise.resolve();
620+
expect(shell.overlayKind).toBe("mcp");
621+
expect(shell.overlayItems).not.toEqual(["Help"]);
622+
});
623+
});
624+
});

‎src/tui/gate-wire.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ export function wireGates(
305305
if (shell.overlayList !== null) {
306306
// A replaceable command surface yields to the decision gate and is
307307
// restored after the gate settles. The suspend is a no-op for live
308-
// gates and non-surface popups (palette, mentions, pickers — they keep
309-
// their stacking contracts), so those arrivals simply stay queued.
308+
// gates and stacked popups (palette, mentions — they keep their
309+
// stacking contracts), so those arrivals simply stay queued.
310310
pending.push(open);
311311
suspendReplaceableOverlay(shell);
312312
// The suspend-close's idle-notify may already have opened an older

‎src/tui/overlay-view.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,5 +435,5 @@ export function createOverlayView(ctx: RenderContext) {
435435
);
436436
}
437437

438-
return { host, title, body, paintTitle, paintList, clearBody };
438+
return { host, title, body, paintTitle, paintList, clearBody, detachList };
439439
}

‎src/tui/shell/overlay-host.ts‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -174,28 +174,32 @@ function restorePrimaryFrame(
174174
}
175175

176176
/**
177-
* Kinds opened through `openCommandSurface` (`CommandSurfaceKind` in
178-
* ../command-surfaces.ts, restated here to keep shell/ import-clean). Only
179-
* these yield to a decision gate. Inline popups (mentions, palette, pickers)
180-
* keep their stacking contracts — suspending one would strand its owner, the
181-
* CL-6698 mention-refresh stall — so a gate arriving behind them stays queued.
177+
* Command surfaces that occupy the shared host and can yield to a decision
178+
* gate. Kinds are the live `PrimaryOverlayKind` values those surfaces open
179+
* with (`model_picker` / `add_provider`, not the command-surface aliases
180+
* `models` / `add-provider`). Inline popups (mentions, palette, pickers that
181+
* stack) keep their stacking contracts — suspending one would strand its
182+
* owner, the CL-6698 mention-refresh stall — so a gate arriving behind them
183+
* stays queued.
182184
*/
183-
const GATE_PREEMPTABLE_SURFACE_KINDS: ReadonlySet<string> = new Set([
184-
"help",
185-
"settings",
186-
"permissions",
187-
"plugins",
188-
"hooks",
189-
"mcp",
190-
"models",
191-
"add-provider",
192-
]);
185+
const GATE_PREEMPTABLE_SURFACE_KINDS: ReadonlySet<PrimaryOverlayKind> = new Set(
186+
[
187+
"help",
188+
"settings",
189+
"permissions",
190+
"plugins",
191+
"hooks",
192+
"mcp",
193+
"model_picker",
194+
"add_provider",
195+
],
196+
);
193197

194198
/**
195199
* Suspend the live replaceable command surface so a decision gate can take
196200
* the host; the surface returns after the gate settles (see
197-
* `resumeSuspendedCommandSurface`). Live gates and non-surface popups
198-
* (palette, mentions, pickers) keep their contracts: arrivals behind them
201+
* `resumeSuspendedCommandSurface`). Live gates and stacked popups
202+
* (palette, mentions) keep their contracts: arrivals behind them
199203
* stay queued. Never loses a surface: a second suspend is a no-op while one
200204
* is held.
201205
*/
@@ -209,6 +213,14 @@ export function suspendReplaceableOverlay(shell: AppShell): void {
209213
const frame = capturePrimaryFrame(shell, bag);
210214
if (frame === null) return;
211215
bag.suspendedCommandSurface = frame;
216+
// Suspend is not dismiss: keep the captured onCancel/onDispose for restore.
217+
// closeInsetOverlay would otherwise run both — MCP's onDispose unsubscribes
218+
// without a matching onOpened on restore, and remove-confirm onCancel would
219+
// reopen a list onto the empty host and steal it from the arriving gate.
220+
bag.primaryBindings.onCancel = null;
221+
bag.primaryBindings.onDispose = null;
222+
// Restore reuses this SelectRenderable; clearBody would destroy it.
223+
shell.overlayView.detachList(frame.list);
212224
// Unsuspended close: idle-notify lets an older queued gate take the host
213225
// first (FIFO); the caller opens its gate only if the host is still free.
214226
closeInsetOverlay(shell);

0 commit comments

Comments
 (0)