Merge pull request #519 from corbitsdev/cl-6699-stop-slash-typeahead-from-opening-permission-gates-mid

Refresh slash popup in place instead of close+reopen

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,16 +5217,65 @@ export function openSlashCommands(shell: AppShell): boolean {
   const matches = resolvePaletteCatalog(shell).filter((cmd) =>
     cmd.id.toLowerCase().startsWith(q),
   )
+
+  // 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.
+  //
+  // 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") {
+    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
@@ -5242,8 +5313,12 @@ export function handleSlashPopupKey(shell: AppShell, key: KeyEvent): boolean {
     !key.option
   ) {
     closeSlashPopup(shell)
+    // Zero matches: nothing to dispatch. Preserve the typed text instead of
+    // wiping it — pre-refresh this state was unreachable (popup closed on
+    // zero matches, so Enter fell through to normal prompt handling).
+    if (!active) return true
     setPromptText(shell, "")
-    if (active) dispatchPaletteSelection(shell, active)
+    dispatchPaletteSelection(shell, active)
     return true
   }