Keep slash popup owned on zero-match filter and relayout in place

Author: TheGreatAxios

src/tui/prompt-slash-exit.test.ts

@@ -160,29 +160,38 @@ describe("slash command popup", () => {
     })
   })
 
-  test("an unmatched name prefix closes the popup and keeps the typed text", async () => {
+  test("an unmatched name prefix refreshes in place instead of closing", async () => {
     await withShell(async ({ shell, press, render, frame }) => {
       press("/")
       press("z")
       await render()
-      expect(isSlashPopupOpen(shell)).toBe(false)
-      expect(shell.overlayList).toBeNull()
+      // The popup was already open (from "/") when the filter zeroed out —
+      // closing here would release the host, which is exactly the gap a
+      // queued gate can drain into mid-filter. It stays owned and shows the
+      // same "(no matches)" row the general palette uses.
+      expect(isSlashPopupOpen(shell)).toBe(true)
+      expect(shell.overlayList).not.toBeNull()
       expect(shell.prompt.value).toBe("/z")
-      expect(shell.overlayItems).not.toContain("(no matches)")
-      expect(frame()).not.toContain("(no matches)")
+      expect(shell.overlayItems).toEqual(["(no matches)"])
+      expect(frame()).toContain("(no matches)")
+
+      // A backspace that restores a match refreshes back in place.
+      press("Backspace")
+      expect(isSlashPopupOpen(shell)).toBe(true)
+      expect(shell.paletteCommands.map((c) => c.id)).toEqual(CATALOG.map((c) => c.id))
     })
   })
 
-  test("description prose does not keep the slash list open", async () => {
+  test("description prose keeps the slash list open with no matches", async () => {
     await withShell(async ({ shell, press, render, frame }) => {
       press("/")
       press("p")
       await render()
-      expect(isSlashPopupOpen(shell)).toBe(false)
-      expect(shell.overlayList).toBeNull()
+      expect(isSlashPopupOpen(shell)).toBe(true)
+      expect(shell.overlayList).not.toBeNull()
       expect(shell.prompt.value).toBe("/p")
-      expect(shell.overlayItems).not.toContain("(no matches)")
-      expect(frame()).not.toContain("(no matches)")
+      expect(shell.overlayItems).toEqual(["(no matches)"])
+      expect(frame()).toContain("(no matches)")
     })
   })
 })

src/tui/shell.ts

@@ -1309,6 +1309,32 @@ function overlayRowsPerItem(kind: PrimaryOverlayKind | null): number {
   return isDecisionOverlay(kind) ? DECISION_CHOICE_ROWS : 1
 }
 
+/**
+ * Recompute the overlay host's row budget from the current item count and
+ * relayout into it. Callers that refresh an already-open overlay's items in
+ * place (rather than reopening) must call this themselves — a filter that
+ * narrows a list and then widens it again would otherwise stay pinned at
+ * whatever size it first opened at.
+ */
+function relayoutOverlayHost(shell: AppShell, itemCount: number): void {
+  const perItem = overlayRowsPerItem(shell.overlayKind)
+  const hostRows = overlayHostRows(
+    shell,
+    shell.overlayBodyLines.length,
+    itemCount * perItem,
+  )
+  const minHostRows = overlayMinHostRows(
+    shell,
+    shell.overlayBodyLines.length,
+    itemCount > 0,
+  )
+  relayout(shell, {
+    overlayMode: "inset",
+    overlayBodyRows: hostRows,
+    overlayMinBodyRows: minHostRows,
+  })
+}
+
 /** Columns a body/choice row may paint into, inside border and leading space. */
 function overlayRowWidth(shell: AppShell): number {
   return Math.max(8, Math.max(20, shell.layout.contentWidth) - 4)
@@ -3690,20 +3716,9 @@ export function openListOverlay(
   // against OVERLAY_MAX_FRACTION and the transcript floor, and applyLayout
   // shrinks the viewport to whatever survived — so a longer list scrolls
   // instead of growing, and a short one leaves no dead rows below it.
-  const perItem = overlayRowsPerItem(shell.overlayKind)
   // An empty list charges no rows: a chooser with nothing to choose must not
   // reserve a blank band the operator can neither read nor act on.
   const listItems = labels.length
-  const hostRows = overlayHostRows(
-    shell,
-    shell.overlayBodyLines.length,
-    listItems * perItem,
-  )
-  const minHostRows = overlayMinHostRows(
-    shell,
-    shell.overlayBodyLines.length,
-    listItems > 0,
-  )
 
   shell.overlayList = createListViewport({
     count: labels.length,
@@ -3720,11 +3735,7 @@ export function openListOverlay(
     target: focusTarget,
     scrollOwner: isPalette ? "palette" : "overlay",
   })
-  relayout(shell, {
-    overlayMode: "inset",
-    overlayBodyRows: hostRows,
-    overlayMinBodyRows: minHostRows,
-  })
+  relayoutOverlayHost(shell, listItems)
   applyFocus(shell)
   paintOverlayList(shell)
 }
@@ -4301,13 +4312,24 @@ export function setOverlayItems(
   items: readonly string[],
   itemIds?: readonly string[],
   itemValues?: readonly (string | undefined)[],
+  opts?: { readonly resetActive?: boolean },
 ): void {
   if (!shell.overlayList) return
   shell.overlayItems = items
   const bag = internals.get(shell)
   if (bag && itemIds) bag.overlayItemIds = [...itemIds]
   if (bag && itemValues) bag.overlayItemValues = [...itemValues]
-  shell.overlayList = setListCount(shell.overlayList, items.length)
+  // Most callers (mention/model-picker filtering) keep the operator's current
+  // selection as the list narrows. The `/` popup instead resets to the top
+  // row on every keystroke, matching pre-refresh behavior where each filter
+  // reopened the overlay fresh.
+  shell.overlayList = opts?.resetActive
+    ? createListViewport({
+        count: items.length,
+        height: shell.overlayList.height,
+        activeIndex: 0,
+      })
+    : setListCount(shell.overlayList, items.length)
   paintOverlayList(shell)
 }
 
@@ -5195,10 +5217,6 @@ export function openSlashCommands(shell: AppShell): boolean {
   const matches = resolvePaletteCatalog(shell).filter((cmd) =>
     cmd.id.toLowerCase().startsWith(q),
   )
-  if (matches.length === 0) {
-    closeSlashPopup(shell)
-    return false
-  }
 
   // Every keystroke lands here while the popup is already open. Closing and
   // reopening released the overlay host between the two calls (closeSlashPopup
@@ -5208,36 +5226,56 @@ export function openSlashCommands(shell: AppShell): boolean {
   // 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.
   if (isSlashPopupOpen(shell) && shell.overlayKind === "palette") {
-    shell.paletteCommands = matches
-    const bag = internals.get(shell)
-    if (bag) {
-      bag.paletteFilter = {
-        query: bag.paletteFilter?.query ?? "",
-        title: "commands · /",
-        catalog: matches,
-        typeToFilter: false,
-      }
-      bag.overlayDescribe = (id) => {
-        const cmd = matches.find((c) => c.id === id)
-        const what = cmd?.description?.trim()
-        return what ? { what } : null
-      }
-    }
-    setOverlayItems(
-      shell,
-      paletteLabels(matches),
-      matches.map((c) => c.id),
-    )
+    refreshSlashPopupInPlace(shell, matches)
     return true
   }
 
+  if (matches.length === 0) {
+    closeSlashPopup(shell)
+    return false
+  }
+
   closeSlashPopup(shell)
   openPalette(shell, { catalog: matches, title: "commands · /" })
   slashPopups.add(shell)
   return true
 }
 
+/** Refresh the already-open `/` popup's rows in place for the given matches. */
+function refreshSlashPopupInPlace(
+  shell: AppShell,
+  matches: readonly PaletteCommand[],
+): void {
+  const labels = matches.length > 0 ? paletteLabels(matches) : ["(no matches)"]
+  shell.paletteCommands = matches
+  const bag = internals.get(shell)
+  if (bag) {
+    bag.paletteFilter = {
+      query: bag.paletteFilter?.query ?? "",
+      title: "commands · /",
+      catalog: matches,
+      typeToFilter: false,
+    }
+    bag.overlayDescribe = (id) => {
+      const cmd = matches.find((c) => c.id === id)
+      const what = cmd?.description?.trim()
+      return what ? { what } : null
+    }
+  }
+  setOverlayItems(shell, labels, matches.map((c) => c.id), undefined, {
+    resetActive: true,
+  })
+  relayoutOverlayHost(shell, labels.length)
+}
+
 function setPromptText(shell: AppShell, value: string): void {
   shell.prompt.value = value
   shell.prompt.cursorOffset = value.length

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

@@ -14,6 +14,7 @@ import { wireGates } from "./gate-wire"
 import {
   createAppShell,
   isSlashPopupOpen,
+  onOverlayClosed,
   type AppShell,
 } from "./shell"
 
@@ -62,6 +63,18 @@ 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
+      // 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.
+      let closedCount = 0
+      const disposeClosedSpy = onOverlayClosed(shell, () => {
+        closedCount++
+      })
       try {
         press("/")
         expect(isSlashPopupOpen(shell)).toBe(true)
@@ -83,6 +96,7 @@ describe("/ popup keeps a queued gate queued across a filter refresh", () => {
         // Queued, not opened — the slash popup still owns the host.
         expect(shell.overlayKind).toBe("palette")
         expect(resolved).toBeUndefined()
+        expect(closedCount).toBe(0)
 
         // Refreshing the filter must not release the host to the queued gate.
         press("m")
@@ -94,20 +108,44 @@ describe("/ popup keeps a queued gate queued across a filter refresh", () => {
           "mcp",
         ])
         expect(resolved).toBeUndefined()
+        expect(closedCount).toBe(0)
 
         // Filtering keeps working after the refresh.
         press("o")
         expect(shell.prompt.value).toBe("/mo")
         expect(shell.paletteCommands.map((c) => c.id)).toEqual(["model"])
         expect(isSlashPopupOpen(shell)).toBe(true)
         expect(resolved).toBeUndefined()
+        expect(closedCount).toBe(0)
+
+        // A keystroke that drops matches to zero must not dismiss the popup
+        // either — it stays owned with a "(no matches)" row, and the gate
+        // stays queued behind it.
+        press("z")
+        expect(shell.prompt.value).toBe("/moz")
+        expect(isSlashPopupOpen(shell)).toBe(true)
+        expect(shell.overlayKind).toBe("palette")
+        expect(shell.paletteCommands).toEqual([])
+        expect(shell.overlayItems).toEqual(["(no matches)"])
+        expect(resolved).toBeUndefined()
+        expect(closedCount).toBe(0)
+
+        // A backspace that restores matches refreshes back in place too.
+        press("Backspace")
+        expect(shell.prompt.value).toBe("/mo")
+        expect(shell.paletteCommands.map((c) => c.id)).toEqual(["model"])
+        expect(isSlashPopupOpen(shell)).toBe(true)
+        expect(resolved).toBeUndefined()
+        expect(closedCount).toBe(0)
 
         // A true dismiss still drains the queue as before.
         press("Escape")
         await Bun.sleep(60)
         expect(shell.overlayKind).toBe("permissions")
         expect(resolved).toBeUndefined()
+        expect(closedCount).toBe(1)
       } finally {
+        disposeClosedSpy()
         dispose()
       }
     })