Drop the idle-notify wrapper and abort reservations on dispose

Esc with a visible list was notifying twice (abort microtask plus close),
and occupancy comments still described overlayList instead of idle.

Author: TheGreatAxios

src/tui/product-host.ts

@@ -531,11 +531,11 @@ export async function mountProductHost(config: ProductHostConfig): Promise<Produ
 
     // Alt+A from the model picker: close it and open a fresh selector over
     // every first-class provider kind, no already-connected filtering. This
-    // gets its own PrimaryOverlayKind opened through the same close-then-open
-    // path openModels itself uses, rather than the palette's priorOverlay
-    // stack — that stack exists so the palette can float over a permission or
-    // operator question without dropping the awaited promise underneath it,
-    // which does not apply here.
+    // gets its own PrimaryOverlayKind opened through the same reserved
+    // close-then-open path openModels itself uses, rather than the palette's
+    // priorOverlay stack — that stack exists so the palette can float over a
+    // permission or operator question without dropping the awaited promise
+    // underneath it, which does not apply here.
     openAddProvider =
       addProviderChoices !== undefined && onConnect !== undefined
         ? (opts?: { returnToModels?: boolean }): void => {

src/tui/shell.ts

@@ -1964,7 +1964,7 @@ interface ShellInternals {
   overlayRawBodyText: string;
   /** Snapshot when palette stacks over another primary overlay. */
   priorOverlay: PriorOverlaySnapshot | null;
-  /** Advances whenever a new overlay takes ownership of the shared host. */
+  /** Advances on a new overlay taking the host, and when the host empties. */
   overlayGeneration: number;
   /** Optional stable ids aligned with overlayItems for the open primary. */
   overlayItemIds: readonly string[];
@@ -2013,13 +2013,13 @@ interface ShellInternals {
   overlayOnDispose: (() => void) | null;
   /** True while the open primary is a decision gate that must not be replaced. */
   overlayIsGate: boolean;
-  /** Fired once the shell has no overlay open, so queued gates can re-open. */
+  /** Fired once the overlay host is idle, so queued gates can re-open. */
   overlayClosedListeners: Set<() => void>;
   /**
    * Command-surface open while a live overlay still holds the host. One slot;
    * a newer command replaces an older one. Flushed only after that overlay
-   * has actually closed and the host is idle — never from notifyOverlayClosed,
-   * which would let wireGates drain a queued gate onto the same host.
+   * has actually closed and the host is idle — never from idle-notify, which
+   * would let wireGates drain a queued gate onto the same host.
    */
   deferredCommandOverlay: OpenListOverlayOpts | null;
   /** True while a microtask to flush deferredCommandOverlay is queued. */
@@ -4313,11 +4313,6 @@ function notifyOverlayClosed(shell: AppShell): void {
   for (const listener of [...bag.overlayClosedListeners]) listener();
 }
 
-function notifyIfHostIdle(shell: AppShell): void {
-  if (!isOverlayHostIdle(shell)) return;
-  notifyOverlayClosed(shell);
-}
-
 /**
  * Hold the overlay host idle-notify while an async command surface is still
  * claiming it (permissions.list() before settings/permissions paint). Release
@@ -4337,7 +4332,7 @@ export function reserveOverlayHost(shell: AppShell): () => void {
     if (!current || current.overlayReservationEpoch !== epoch) return;
     if (current.overlayHostReservations > 0) current.overlayHostReservations -= 1;
     scheduleDeferredCommandFlush(shell);
-    notifyIfHostIdle(shell);
+    notifyOverlayClosed(shell);
   };
 }
 
@@ -4349,8 +4344,6 @@ function abortOverlayHostReservations(shell: AppShell): void {
   bag.overlayHostReservations = 0;
   bag.overlayGeneration += 1;
   scheduleDeferredCommandFlush(shell);
-  // Next tick so the same Esc cannot also dismiss a gate that this abort drains.
-  queueMicrotask(() => notifyIfHostIdle(shell));
 }
 
 /** One deferred command-surface slot while the host is busy. */
@@ -4387,7 +4380,7 @@ function flushDeferredCommandOverlay(shell: AppShell): void {
   if (shell.overlayList !== null) return;
   const opts = bag.deferredCommandOverlay;
   if (opts === null) {
-    notifyIfHostIdle(shell);
+    notifyOverlayClosed(shell);
     return;
   }
   bag.deferredCommandOverlay = null;
@@ -4482,7 +4475,7 @@ export interface OverlayContinuationToken {
   readonly generation: number;
 }
 
-/** Capture overlay generation for an async continuation. Stale after a newer open or Esc during a reserved in-flight open. */
+/** Capture overlay generation for an async continuation. Stale after a newer open, a full close, or Esc abort. */
 export function captureOverlayContinuation(shell: AppShell): OverlayContinuationToken {
   return { generation: internals.get(shell)?.overlayGeneration ?? -1 };
 }
@@ -4495,7 +4488,7 @@ export function isOverlayContinuationCurrent(
   return isOverlayGenerationCurrent(shell, token) && shell.overlayList === null;
 }
 
-/** True while the shell is live and no newer overlay has advanced the generation. */
+/** True while the shell is live and generation has not advanced. */
 export function isOverlayGenerationCurrent(
   shell: AppShell,
   token: OverlayContinuationToken,
@@ -5453,19 +5446,19 @@ export function openSlashCommands(shell: AppShell): boolean {
 
   // Every keystroke lands here while the popup is already open. Closing and
   // reopening released the overlay host between the two calls (closeSlashPopup
-  // routes through closeInsetOverlay, which fires notifyOverlayClosed) — long
-  // enough for a queued permission/operator gate to drain onto it. Refreshing
-  // the open palette in place never releases the host, so a queued gate has
-  // nothing to drain into. priorOverlay stacking is untouched here (it is only
-  // ever written by openListOverlay's stack-on-open path), so a palette
-  // stacked over a prior overlay keeps that snapshot across the refresh.
+  // routes through closeInsetOverlay, which idle-notifies) — long enough for a
+  // queued permission/operator gate to drain onto it. Refreshing the open
+  // palette in place never releases the host, so a queued gate has nothing to
+  // drain into. priorOverlay stacking is untouched here (it is only ever
+  // written by openListOverlay's stack-on-open path), so a palette stacked
+  // over a prior overlay keeps that snapshot across the refresh.
   //
   // A typo that zeroes the matches must not fall through to closeSlashPopup
-  // while the popup is already open — that closes through the same
-  // notifyOverlayClosed path and drains a queued gate mid-filter. Instead
-  // this refreshes in place to a "(no matches)" row, same as the general
-  // palette does, and holds the host until a real dismiss (deleting the `/`,
-  // Esc, accept) or a backspace that restores matches.
+  // while the popup is already open — that closes through the same idle-notify
+  // path and drains a queued gate mid-filter. Instead this refreshes in place
+  // to a "(no matches)" row, same as the general palette does, and holds the
+  // host until a real dismiss (deleting the `/`, Esc, accept) or a backspace
+  // that restores matches.
   if (isSlashPopupOpen(shell) && shell.overlayKind === "palette") {
     refreshSlashPopupInPlace(shell, matches);
     return true;
@@ -5930,6 +5923,8 @@ export function createAppShell(renderer: ShellRenderer, options?: AppShellOption
       if (internals.get(shell)?.overlayHostReservations) {
         abortOverlayHostReservations(shell);
         key.preventDefault();
+        // Next tick so the same Esc cannot also dismiss a gate this abort drains.
+        queueMicrotask(() => notifyOverlayClosed(shell));
         return;
       }
       if (shell.observe) {
@@ -6480,11 +6475,7 @@ export function createAppShell(renderer: ShellRenderer, options?: AppShellOption
       // release subscriptions or settle awaited cancellation exactly once.
       let overlayGuard = 4;
       while (shell.overlayList !== null && overlayGuard-- > 0) closeInsetOverlay(shell);
-      const bag = internals.get(shell);
-      if (bag) {
-        bag.overlayReservationEpoch += 1;
-        bag.overlayHostReservations = 0;
-      }
+      abortOverlayHostReservations(shell);
       disposed = true;
       shell.disposed = true;
       if (wireKeys) {

src/tui/slash-popup-gate.test.ts

@@ -1,9 +1,8 @@
 /**
  * CL-6699: a queued permission/operator gate must not open onto the host in
  * the middle of a `/` command filter session. The old close-then-reopen
- * refresh (closeSlashPopup -> closeInsetOverlay -> notifyOverlayClosed)
- * released the host between the two calls, and a gate queued behind the
- * popup drained into that gap.
+ * refresh released the host between the two calls (idle-notify via
+ * onOverlayClosed), and a gate queued behind the popup drained into that gap.
  *
  * CL-6711: accepting a slash/palette command must not drain that same queue
  * onto the host before dispatch has claimed it. A live gate already on the
@@ -14,11 +13,7 @@ import { describe, expect, test } from "bun:test";
 
 import { withTestRenderer } from "./harness";
 import type { PaletteCommand } from "./command-catalog";
-import {
-  openCommandSurface,
-  type CommandSurfaceDeps,
-  type PluginsSurfaceDeps,
-} from "./command-surfaces";
+import { openCommandSurface, type CommandSurfaceDeps } from "./command-surfaces";
 import { wireGates } from "./gate-wire";
 import { openPermissionsOverlay } from "./overlays";
 import {
@@ -27,6 +22,7 @@ import {
   closeReplaceableOverlay,
   createAppShell,
   cycleOverlaySelection,
+  isOverlayHostIdle,
   isSlashPopupOpen,
   moveOverlaySelection,
   onOverlayClosed,
@@ -160,14 +156,13 @@ describe("/ popup keeps a queued gate queued across a filter refresh", () => {
     await withShell(async ({ shell, press }) => {
       const emitter = new EventEmitter();
       const dispose = wireGates(emitter, shell);
-      // The host closing (onOverlayClosed) is what the queued gate waits
+      // The host going idle (onOverlayClosed) is what the queued gate waits
       // on to drain — see gate-wire.ts's onOverlayClosed/pending. Under the
       // old close-then-reopen refresh this fires on every filter keystroke
-      // (closeSlashPopup -> closeInsetOverlay -> notifyOverlayClosed) even
-      // though the palette immediately re-stacks on top and every assertion
-      // on shell.overlayKind alone sees only "palette" again by the time it
-      // runs. Counting this call directly is what actually distinguishes
-      // the in-place refresh from the old close+reopen one.
+      // even though the palette immediately re-stacks on top and every
+      // assertion on shell.overlayKind alone sees only "palette" again by
+      // the time it runs. Counting this call directly is what actually
+      // distinguishes the in-place refresh from the old close+reopen one.
       let closedCount = 0;
       const disposeClosedSpy = onOverlayClosed(shell, () => {
         closedCount++;
@@ -802,7 +797,13 @@ describe("overlay host occupancy and opt-in deferral", () => {
           plugins: {
             list: () => [],
             setEnabled: () => Promise.resolve(undefined),
-          } as unknown as PluginsSurfaceDeps,
+            saveCredentials: async () => undefined,
+            verify: async () => ({ ok: true, message: "" }),
+            addPath: async () => ({ ok: true, message: "" }),
+            webProviders: () => [],
+            currentWebProvider: () => undefined,
+            setWebProvider: async () => undefined,
+          },
         });
         hanging.resolve();
         await Promise.resolve();
@@ -913,10 +914,13 @@ describe("overlay host occupancy and opt-in deferral", () => {
         press("Escape");
         await render();
         await Bun.sleep(60);
+        expect(shell.overlayKind).not.toBe("settings");
+        expect(isOverlayHostIdle(shell)).toBe(true);
         resolveSecond();
         await Promise.resolve();
         await Promise.resolve();
         expect(shell.overlayKind).not.toBe("settings");
+        expect(isOverlayHostIdle(shell)).toBe(true);
       },
       {
         onCommand: (name, shell) => {
@@ -956,16 +960,19 @@ describe("overlay host occupancy and opt-in deferral", () => {
       try {
         openHelpOverlay(shell);
         expect(shell.overlayKind).toBe("help");
+        expect(isOverlayHostIdle(shell)).toBe(false);
 
         let resolved: unknown;
         emitPermissionGate(emitter, (outcome) => {
           resolved = outcome;
         });
         expect(shell.overlayKind).toBe("help");
+        expect(isOverlayHostIdle(shell)).toBe(false);
         expect(resolved).toBeUndefined();
 
         openHelpOverlay(shell);
         expect(shell.overlayKind).toBe("help");
+        expect(isOverlayHostIdle(shell)).toBe(false);
         expect(resolved).toBeUndefined();
       } finally {
         dispose();