From ab36bcc434aac25e0b21c2b4111faf0d95f8e146 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 00:27:22 -0700 Subject: [PATCH 1/4] Fix stale mention accept during overlay reopen Enter can still splice a completion from a prior overlay frame while a mention lookup is in flight, using a cursor captured before the await. Accept now requires the current generation and a live @token. Enter that fails those checks dismisses the popup so the in-flight lookup cannot reopen it. --- docs/TUI.md | 8 +- src/tui/mention-popup.test.ts | 136 ++++++++++++++++++++++++++++++++++ src/tui/shell.ts | 51 ++++++++++--- 3 files changed, 183 insertions(+), 12 deletions(-) diff --git a/docs/TUI.md b/docs/TUI.md index 030e873e5..4c6503539 100644 --- a/docs/TUI.md +++ b/docs/TUI.md @@ -547,8 +547,12 @@ heuristic is permanently skipped for the rest of the session @-mention path completion opens a popup keyed off the `@token` under the cursor (`openAtMentionSuggestions`, `shell.ts`); every keystroke re-queries, and a generation counter discards a slower, stale query's results if a newer -one already landed. Directory picks re-open one level down so the operator -can drill into a path without retyping it. +one already landed. Accept is refused unless that generation is still current +and a live `@` token is under the cursor. Enter that fails those checks +dismisses the popup (same generation bump as Esc) so an in-flight lookup +cannot reopen it. Dismiss clears the accept handler. +Directory picks re-open one level down so the operator can drill into a path +without retyping it. A readline-style kill ring backs Ctrl+K/U/W (kill) and Ctrl+Y/Alt+Y (yank/yank-pop) on top of the textarea's native delete bindings, which diff --git a/src/tui/mention-popup.test.ts b/src/tui/mention-popup.test.ts index 2617b2c0d..176e1c5a7 100644 --- a/src/tui/mention-popup.test.ts +++ b/src/tui/mention-popup.test.ts @@ -276,3 +276,139 @@ describe("@ popup narrows as you type", () => { }); }); }); + +describe("CL-6718 mention accept gating", () => { + function hangableSource(): { + source: (prefix: string) => Promise; + resolveNext: (entries: readonly string[]) => void; + } { + const pending: ((entries: readonly string[]) => void)[] = []; + return { + source: () => + new Promise((resolve) => { + pending.push(resolve); + }), + resolveNext: (entries) => { + const resolve = pending.shift(); + if (resolve === undefined) throw new Error("no pending mention lookup"); + resolve(entries); + }, + }; + } + + const ROOT = ["AGENTS.md", "README.md", "session-notes.md", "src/"] as const; + + test("accept mid-reopen does not splice a stale completion", async () => { + await withShell(async (shell) => { + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const first = openAtMentionSuggestions(shell); + resolveNext(ROOT); + expect(await first).toBe(true); + expect(isMentionPopupOpen(shell)).toBe(true); + + expect(handleMentionPopupKey(shell, printable("s"))).toBe(true); + expect(shell.prompt.value).toBe("read @s"); + // Second lookup is in flight; do not resolve it. + + acceptOverlaySelection(shell); + expect(shell.prompt.value).toBe("read @s"); + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.streamLog.some((row) => /Chose /.test(row.text))).toBe(false); + + resolveNext(ROOT); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.prompt.value).toBe("read @s"); + }); + }); + + test("accept after a failed reopen does not splice a mention", async () => { + await withShell(async (shell) => { + const emitter = new EventEmitter(); + const dispose = wireGates(emitter, shell); + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + try { + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const pending = openAtMentionSuggestions(shell); + + emitter.emit("permission.gate", { + request: { + tool: "run_shell", + action: "Run shell command", + subject: "bun test", + scopes: [], + }, + resolve: () => {}, + }); + expect(shell.overlayKind).toBe("permissions"); + + resolveNext(ROOT); + expect(await pending).toBe(false); + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + + acceptOverlaySelection(shell); + expect(shell.prompt.value).toBe("read @"); + } finally { + dispose(); + } + }); + }); + + test("accept with cursor off the @token does not splice", async () => { + await withShell(async (shell) => { + await openAt(shell, "read @"); + expect(isMentionPopupOpen(shell)).toBe(true); + + shell.prompt.cursorOffset = 0; + acceptOverlaySelection(shell); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.prompt.value).toBe("read @"); + expect(shell.streamLog.some((row) => /Chose /.test(row.text))).toBe(false); + }); + }); + + test("dismiss during an in-flight lookup does not reopen or splice", async () => { + await withShell(async (shell) => { + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const first = openAtMentionSuggestions(shell); + resolveNext(ROOT); + expect(await first).toBe(true); + expect(isMentionPopupOpen(shell)).toBe(true); + + expect(handleMentionPopupKey(shell, printable("s"))).toBe(true); + expect(shell.prompt.value).toBe("read @s"); + + closeMentionPopup(shell); + expect(isMentionPopupOpen(shell)).toBe(false); + + resolveNext(ROOT); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + + if (shell.overlayList !== null) acceptOverlaySelection(shell); + expect(shell.prompt.value).toBe("read @s"); + }); + }); +}); diff --git a/src/tui/shell.ts b/src/tui/shell.ts index dc433dade..bb2d4df01 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -4076,6 +4076,7 @@ export function closeInsetOverlay(shell: AppShell): void { if (!shell.overlayList) return; // Esc (or any other dismiss) must also drop the `/` and `@` popups' key claim. slashPopups.delete(shell); + if (mentionPopups.has(shell)) clearMentionAccept(shell); mentionPopups.delete(shell); const wasPalette = shell.overlayKind === "palette"; @@ -4451,6 +4452,12 @@ export function acceptOverlaySelection(shell: AppShell): void { return; } + if (kind === "mentions" && mentionPopups.has(shell) && !mentionAcceptIsLive(shell)) { + // Stale generation or cursor off the @token: operator dismiss, not accept. + closeInsetOverlay(shell); + return; + } + const id = bag?.overlayItemIds[idx]; // Type-to-filter plants "(no matches)" with an empty-id sentinel. Stay open. if (id === "") return; @@ -4475,6 +4482,9 @@ export function acceptOverlaySelection(shell: AppShell): void { meta: overlayKindWord(kind), }); } + // Accept is not operator dismiss: keep mention accept state for onAccept + // after this close (closeInsetOverlay would otherwise bump the generation). + if (kind === "mentions") mentionPopups.delete(shell); closeInsetOverlay(shell); dispatchOverlayAccept(shell, selection, perOpen); } @@ -5058,7 +5068,6 @@ export async function openAtMentionSuggestions(shell: AppShell): Promise { const state = mentionAcceptState.get(shell); - const completion = state?.suggestions[selection.index]; - if (completion === undefined || state === undefined) return; + if (state === undefined) return; + if (mentionGenerations.get(shell) !== state.generation) return; + const live = parseAtState(shell.prompt.value, shell.prompt.cursorOffset); + if (live === null) return; + const completion = state.suggestions[selection.index]; + if (completion === undefined) return; const spliced = spliceMentionCompletion( shell.prompt.value, - state.atStart, - state.cursor, + live.atStart, + shell.prompt.cursorOffset, completion, ); shell.prompt.value = spliced.value; @@ -5116,6 +5131,8 @@ export async function openAtMentionSuggestions(shell: AppShell): Promise(); const mentionGenerations = new WeakMap(); interface MentionAcceptState { readonly suggestions: readonly string[]; - readonly atStart: number; - readonly cursor: number; + readonly generation: number; } const mentionAcceptState = new WeakMap(); +/** Drop accept state and invalidate in-flight lookups on operator dismiss. */ +function clearMentionAccept(shell: AppShell): void { + mentionAcceptState.delete(shell); + mentionGenerations.set(shell, (mentionGenerations.get(shell) ?? 0) + 1); +} + +/** True when Enter would splice a live @token, not a stale overlay row. */ +function mentionAcceptIsLive(shell: AppShell): boolean { + const state = mentionAcceptState.get(shell); + if (state === undefined) return false; + if (mentionGenerations.get(shell) !== state.generation) return false; + return parseAtState(shell.prompt.value, shell.prompt.cursorOffset) !== null; +} + /** True while the `@` path popup owns typed characters. */ export function isMentionPopupOpen(shell: AppShell): boolean { return mentionPopups.has(shell) && shell.overlayKind === "mentions"; @@ -5136,6 +5166,7 @@ export function isMentionPopupOpen(shell: AppShell): boolean { export function closeMentionPopup(shell: AppShell): void { if (!mentionPopups.has(shell)) return; + clearMentionAccept(shell); mentionPopups.delete(shell); if (shell.overlayList) closeInsetOverlay(shell); } From 07a82eb022c1151dd596c13e04269fb4ffa95115 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 15:26:16 -0700 Subject: [PATCH 2/4] Tighten mention overlay live-token checks Dismiss, including Esc, bumps mention generation so an in-flight lookup cannot reopen. Accept requires the current generation and the same live @token the lookup started on. Enter that fails those checks dismisses. --- docs/TUI.md | 8 +- src/tui/mention-popup.test.ts | 196 ++++++++++++++++++++++------------ src/tui/shell.ts | 50 ++++++--- 3 files changed, 165 insertions(+), 89 deletions(-) diff --git a/docs/TUI.md b/docs/TUI.md index 4c6503539..a70df7d82 100644 --- a/docs/TUI.md +++ b/docs/TUI.md @@ -545,12 +545,14 @@ heuristic is permanently skipped for the rest of the session (`shell.ts`, the `sawBracketedPaste` guard). @-mention path completion opens a popup keyed off the `@token` under the -cursor (`openAtMentionSuggestions`, `shell.ts`); every keystroke re-queries, +cursor (`openAtMentionSuggestions`, `src/tui/shell.ts`); every keystroke re-queries, and a generation counter discards a slower, stale query's results if a newer one already landed. Accept is refused unless that generation is still current -and a live `@` token is under the cursor. Enter that fails those checks +and a live `@` token is under the cursor (the same `@` the lookup started on). +Enter that fails those checks dismisses the popup (same generation bump as Esc) so an in-flight lookup -cannot reopen it. Dismiss clears the accept handler. +cannot reopen it. A lookup that finishes after the cursor has left that +token does not open. Dismiss clears mention accept state and bumps generation. Directory picks re-open one level down so the operator can drill into a path without retyping it. diff --git a/src/tui/mention-popup.test.ts b/src/tui/mention-popup.test.ts index 176e1c5a7..fbf7b0f5c 100644 --- a/src/tui/mention-popup.test.ts +++ b/src/tui/mention-popup.test.ts @@ -1,6 +1,7 @@ /** * Integration: the `@` path popup narrows as you type, the same contract the - * `/` command popup already honours. + * `/` command popup already honours. Mention accept is gated on a current + * generation and a live `@` token under the cursor. */ import { EventEmitter } from "node:events"; import { describe, expect, test } from "bun:test"; @@ -11,6 +12,7 @@ import { wireGates } from "./gate-wire"; import { withTestRenderer } from "./harness"; import { acceptOverlaySelection, + closeInsetOverlay, closeMentionPopup, createAppShell, handleMentionPopupKey, @@ -68,12 +70,17 @@ const BACKSPACE = { option: false, } as unknown as KeyEvent; -/** Drive one key and let the popup's async re-query settle. */ -async function type(shell: AppShell, key: KeyEvent): Promise { - const handled = handleMentionPopupKey(shell, key); +/** Let the popup's async re-query settle the way `type()` already does. */ +async function drainMicrotasks(): Promise { await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); +} + +/** Drive one key and let the popup's async re-query settle. */ +async function type(shell: AppShell, key: KeyEvent): Promise { + const handled = handleMentionPopupKey(shell, key); + await drainMicrotasks(); return handled; } @@ -83,6 +90,26 @@ async function openAt(shell: AppShell, value: string): Promise { await openAtMentionSuggestions(shell); } +function hangableSource(): { + source: (prefix: string) => Promise; + resolveNext: (entries: readonly string[]) => void; +} { + const pending: ((entries: readonly string[]) => void)[] = []; + return { + source: (_prefix) => + new Promise((resolve) => { + pending.push(resolve); + }), + resolveNext: (entries) => { + const resolve = pending.shift(); + if (resolve === undefined) throw new Error("no pending mention lookup"); + resolve(entries); + }, + }; +} + +const ROOT = TREE[""] ?? []; + describe("@ popup narrows as you type", () => { test("printable keys filter the list and land in the prompt", async () => { await withShell(async (shell) => { @@ -198,9 +225,7 @@ describe("@ popup narrows as you type", () => { acceptOverlaySelection(shell); // The accept splices `src/` and re-opens; let the re-query settle. - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); + await drainMicrotasks(); expect(shell.prompt.value).toBe("@src/"); expect(shell.overlayKind).toBe("mentions"); expect(shell.overlayItems).toEqual([ @@ -275,29 +300,42 @@ describe("@ popup narrows as you type", () => { } }); }); -}); -describe("CL-6718 mention accept gating", () => { - function hangableSource(): { - source: (prefix: string) => Promise; - resolveNext: (entries: readonly string[]) => void; - } { - const pending: ((entries: readonly string[]) => void)[] = []; - return { - source: () => - new Promise((resolve) => { - pending.push(resolve); - }), - resolveNext: (entries) => { - const resolve = pending.shift(); - if (resolve === undefined) throw new Error("no pending mention lookup"); - resolve(entries); - }, - }; - } + test("a permission gate that opened during lookup keeps mentions closed", async () => { + await withShell(async (shell) => { + const emitter = new EventEmitter(); + const dispose = wireGates(emitter, shell); + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + try { + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const pending = openAtMentionSuggestions(shell); - const ROOT = ["AGENTS.md", "README.md", "session-notes.md", "src/"] as const; + emitter.emit("permission.gate", { + request: { + tool: "run_shell", + action: "Run shell command", + subject: "bun test", + scopes: [], + }, + resolve: () => {}, + }); + expect(shell.overlayKind).toBe("permissions"); + + resolveNext(ROOT); + expect(await pending).toBe(false); + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).toBe("permissions"); + expect(shell.prompt.value).toBe("read @"); + } finally { + dispose(); + } + }); + }); +}); +describe("mention accept requires a live @token", () => { test("accept mid-reopen does not splice a stale completion", async () => { await withShell(async (shell) => { const { source, resolveNext } = hangableSource(); @@ -318,12 +356,9 @@ describe("CL-6718 mention accept gating", () => { expect(shell.prompt.value).toBe("read @s"); expect(isMentionPopupOpen(shell)).toBe(false); expect(shell.overlayKind).not.toBe("mentions"); - expect(shell.streamLog.some((row) => /Chose /.test(row.text))).toBe(false); resolveNext(ROOT); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); + await drainMicrotasks(); expect(isMentionPopupOpen(shell)).toBe(false); expect(shell.overlayKind).not.toBe("mentions"); @@ -331,53 +366,52 @@ describe("CL-6718 mention accept gating", () => { }); }); - test("accept after a failed reopen does not splice a mention", async () => { + test("accept with cursor off the @token does not splice", async () => { + await withShell(async (shell) => { + await openAt(shell, "read @"); + expect(isMentionPopupOpen(shell)).toBe(true); + + shell.prompt.cursorOffset = 0; + acceptOverlaySelection(shell); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.prompt.value).toBe("read @"); + }); + }); + + test("a lookup whose cursor has left the token does not open", async () => { await withShell(async (shell) => { - const emitter = new EventEmitter(); - const dispose = wireGates(emitter, shell); const { source, resolveNext } = hangableSource(); setMentionSuggestionSource(shell, source); - try { - shell.prompt.value = "read @"; - shell.prompt.cursorOffset = shell.prompt.value.length; - const pending = openAtMentionSuggestions(shell); - - emitter.emit("permission.gate", { - request: { - tool: "run_shell", - action: "Run shell command", - subject: "bun test", - scopes: [], - }, - resolve: () => {}, - }); - expect(shell.overlayKind).toBe("permissions"); - resolveNext(ROOT); - expect(await pending).toBe(false); - expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const pending = openAtMentionSuggestions(shell); + shell.prompt.cursorOffset = 0; + resolveNext(ROOT); - acceptOverlaySelection(shell); - expect(shell.prompt.value).toBe("read @"); - } finally { - dispose(); - } + expect(await pending).toBe(false); + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); }); }); - test("accept with cursor off the @token does not splice", async () => { + test("a lookup whose cursor moved onto a different @token does not open", async () => { await withShell(async (shell) => { - await openAt(shell, "read @"); - expect(isMentionPopupOpen(shell)).toBe(true); + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); - shell.prompt.cursorOffset = 0; - acceptOverlaySelection(shell); + const value = "see @a and @b"; + shell.prompt.value = value; + shell.prompt.cursorOffset = "see @a".length; + const pending = openAtMentionSuggestions(shell); + shell.prompt.cursorOffset = value.length; + resolveNext(ROOT); + expect(await pending).toBe(false); expect(isMentionPopupOpen(shell)).toBe(false); expect(shell.overlayKind).not.toBe("mentions"); - expect(shell.prompt.value).toBe("read @"); - expect(shell.streamLog.some((row) => /Chose /.test(row.text))).toBe(false); }); }); @@ -398,16 +432,40 @@ describe("CL-6718 mention accept gating", () => { closeMentionPopup(shell); expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayList).toBeNull(); resolveNext(ROOT); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); + await drainMicrotasks(); expect(isMentionPopupOpen(shell)).toBe(false); expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.prompt.value).toBe("read @s"); + }); + }); + + test("Esc during an in-flight lookup does not reopen or splice", async () => { + await withShell(async (shell) => { + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const first = openAtMentionSuggestions(shell); + resolveNext(ROOT); + expect(await first).toBe(true); + expect(isMentionPopupOpen(shell)).toBe(true); - if (shell.overlayList !== null) acceptOverlaySelection(shell); + expect(handleMentionPopupKey(shell, printable("s"))).toBe(true); + expect(shell.prompt.value).toBe("read @s"); + + closeInsetOverlay(shell); + expect(isMentionPopupOpen(shell)).toBe(false); + + resolveNext(ROOT); + await drainMicrotasks(); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); expect(shell.prompt.value).toBe("read @s"); }); }); diff --git a/src/tui/shell.ts b/src/tui/shell.ts index bb2d4df01..3d965f069 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -34,7 +34,7 @@ import { } from "./components/prompt-action-bar-label.js"; import { sliceTailToWidth, sliceToWidth, stringWidth } from "./view/height.js"; import { listPathSuggestions } from "./components/at-mention/list.js"; -import { parseAtState } from "./components/at-mention/parse.js"; +import { parseAtState, type AtState } from "./components/at-mention/parse.js"; import { formatAttachmentSummary, readClipboardImage, @@ -5050,11 +5050,19 @@ const MOTION_KEYS: ReadonlySet = new Set([ const defaultMentionSource: MentionSuggestionSource = (prefix) => listPathSuggestions(prefix, process.cwd()); +interface MentionAcceptState { + readonly suggestions: readonly string[]; + readonly generation: number; +} + /** * Open path suggestions for the @token under the cursor and splice the * accepted entry back into the prompt. Directory picks re-open one level * down so the operator can drill in without typing the path. * Returns false when the cursor is not inside an @token or nothing matched. + * + * Accept requires a current generation and a live `@` token under the cursor. + * A lookup that finishes after the cursor has left this token does not open. */ export async function openAtMentionSuggestions(shell: AppShell): Promise { const at = parseAtState(shell.prompt.value, shell.prompt.cursorOffset); @@ -5090,11 +5098,19 @@ export async function openAtMentionSuggestions(shell: AppShell): Promise { - const state = mentionAcceptState.get(shell); - if (state === undefined) return; - if (mentionGenerations.get(shell) !== state.generation) return; - const live = parseAtState(shell.prompt.value, shell.prompt.cursorOffset); - if (live === null) return; - const completion = state.suggestions[selection.index]; + const ready = liveMentionAccept(shell); + if (ready === null) return; + const completion = ready.state.suggestions[selection.index]; if (completion === undefined) return; const spliced = spliceMentionCompletion( shell.prompt.value, - live.atStart, + ready.live.atStart, shell.prompt.cursorOffset, completion, ); @@ -5139,10 +5152,6 @@ export async function openAtMentionSuggestions(shell: AppShell): Promise(); const mentionGenerations = new WeakMap(); -interface MentionAcceptState { - readonly suggestions: readonly string[]; - readonly generation: number; -} const mentionAcceptState = new WeakMap(); /** Drop accept state and invalidate in-flight lookups on operator dismiss. */ @@ -5151,12 +5160,19 @@ function clearMentionAccept(shell: AppShell): void { mentionGenerations.set(shell, (mentionGenerations.get(shell) ?? 0) + 1); } +/** Live accept snapshot, or null when generation is stale or the cursor left the token. */ +function liveMentionAccept(shell: AppShell): { state: MentionAcceptState; live: AtState } | null { + const state = mentionAcceptState.get(shell); + if (state === undefined) return null; + if (mentionGenerations.get(shell) !== state.generation) return null; + const live = parseAtState(shell.prompt.value, shell.prompt.cursorOffset); + if (live === null) return null; + return { state, live }; +} + /** True when Enter would splice a live @token, not a stale overlay row. */ function mentionAcceptIsLive(shell: AppShell): boolean { - const state = mentionAcceptState.get(shell); - if (state === undefined) return false; - if (mentionGenerations.get(shell) !== state.generation) return false; - return parseAtState(shell.prompt.value, shell.prompt.cursorOffset) !== null; + return liveMentionAccept(shell) !== null; } /** True while the `@` path popup owns typed characters. */ From f617263e5c33a8002ffb32e850c1b5b0ebcaea70 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 16:52:52 -0700 Subject: [PATCH 3/4] Pin mention accept to the lookup's @token Accept splices only the @token the overlay opened on. Cursor on a different live @token dismisses instead of completing into the wrong token. --- docs/TUI.md | 8 ++++---- src/tui/mention-popup.test.ts | 17 +++++++++++++++++ src/tui/shell.ts | 12 +++++++++--- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/docs/TUI.md b/docs/TUI.md index a70df7d82..d1ed7e347 100644 --- a/docs/TUI.md +++ b/docs/TUI.md @@ -549,10 +549,10 @@ cursor (`openAtMentionSuggestions`, `src/tui/shell.ts`); every keystroke re-quer and a generation counter discards a slower, stale query's results if a newer one already landed. Accept is refused unless that generation is still current and a live `@` token is under the cursor (the same `@` the lookup started on). -Enter that fails those checks -dismisses the popup (same generation bump as Esc) so an in-flight lookup -cannot reopen it. A lookup that finishes after the cursor has left that -token does not open. Dismiss clears mention accept state and bumps generation. +Enter that fails those checks dismisses the popup (same generation bump as Esc) +so an in-flight lookup cannot reopen it. A lookup that finishes after the +cursor has left that token does not open. Dismiss clears mention accept state +and bumps generation. Directory picks re-open one level down so the operator can drill into a path without retyping it. diff --git a/src/tui/mention-popup.test.ts b/src/tui/mention-popup.test.ts index fbf7b0f5c..7e1059530 100644 --- a/src/tui/mention-popup.test.ts +++ b/src/tui/mention-popup.test.ts @@ -380,6 +380,23 @@ describe("mention accept requires a live @token", () => { }); }); + test("accept with cursor on a different @token does not splice", async () => { + await withShell(async (shell) => { + const value = "see @a and @b"; + shell.prompt.value = value; + shell.prompt.cursorOffset = "see @a".length; + expect(await openAtMentionSuggestions(shell)).toBe(true); + expect(isMentionPopupOpen(shell)).toBe(true); + + shell.prompt.cursorOffset = value.length; + acceptOverlaySelection(shell); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.prompt.value).toBe(value); + }); + }); + test("a lookup whose cursor has left the token does not open", async () => { await withShell(async (shell) => { const { source, resolveNext } = hangableSource(); diff --git a/src/tui/shell.ts b/src/tui/shell.ts index 3d965f069..82546d9c6 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -5053,13 +5053,15 @@ const defaultMentionSource: MentionSuggestionSource = (prefix) => interface MentionAcceptState { readonly suggestions: readonly string[]; readonly generation: number; + readonly atStart: number; } /** * Open path suggestions for the @token under the cursor and splice the * accepted entry back into the prompt. Directory picks re-open one level * down so the operator can drill in without typing the path. - * Returns false when the cursor is not inside an @token or nothing matched. + * Returns false when the cursor is not inside this @token, nothing matched, + * a newer lookup superseded this one, or the overlay host was taken. * * Accept requires a current generation and a live `@` token under the cursor. * A lookup that finishes after the cursor has left this token does not open. @@ -5110,7 +5112,11 @@ export async function openAtMentionSuggestions(shell: AppShell): Promise Date: Thu, 3 Sep 2026 17:58:33 -0700 Subject: [PATCH 4/4] Clean up mention accept tests and splice path Splice goes through editPromptAt. Tests name the functions they call, assert a closed overlay, and cover Enter during a no-match re-query. --- src/tui/mention-popup.test.ts | 90 ++++++++++++++++++++++++++--------- src/tui/shell.ts | 29 +++++------ 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/src/tui/mention-popup.test.ts b/src/tui/mention-popup.test.ts index 7e1059530..87df1de8d 100644 --- a/src/tui/mention-popup.test.ts +++ b/src/tui/mention-popup.test.ts @@ -70,7 +70,7 @@ const BACKSPACE = { option: false, } as unknown as KeyEvent; -/** Let the popup's async re-query settle the way `type()` already does. */ +/** Flush the three microtask hops `openAtMentionSuggestions` takes after a key. */ async function drainMicrotasks(): Promise { await Promise.resolve(); await Promise.resolve(); @@ -108,7 +108,7 @@ function hangableSource(): { }; } -const ROOT = TREE[""] ?? []; +const ROOT = TREE[""]!; describe("@ popup narrows as you type", () => { test("printable keys filter the list and land in the prompt", async () => { @@ -153,14 +153,8 @@ describe("@ popup narrows as you type", () => { wireKeys: false, run: "idle", }); - let resolveLookup: (entries: readonly string[]) => void = () => {}; - setMentionSuggestionSource( - shell, - () => - new Promise((resolve) => { - resolveLookup = resolve; - }), - ); + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); shell.prompt.value = "read @"; shell.prompt.cursorOffset = shell.prompt.value.length; @@ -168,7 +162,7 @@ describe("@ popup narrows as you type", () => { // The operator quits before the filesystem lookup answers. shell.dispose(); - resolveLookup(["AGENTS.md", "README.md"]); + resolveNext(["AGENTS.md", "README.md"]); await expect(pending).resolves.toBe(false); expect(shell.overlayKind).toBeNull(); @@ -336,7 +330,28 @@ describe("@ popup narrows as you type", () => { }); describe("mention accept requires a live @token", () => { - test("accept mid-reopen does not splice a stale completion", async () => { + test("accept after the lookup resolves splices the live token", async () => { + await withShell(async (shell) => { + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const pending = openAtMentionSuggestions(shell); + resolveNext(ROOT); + expect(await pending).toBe(true); + expect(isMentionPopupOpen(shell)).toBe(true); + const first = shell.overlayItems[0]; + expect(first).toBeDefined(); + + acceptOverlaySelection(shell); + expect(shell.prompt.value).toBe(`read @${first}`); + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).toBeNull(); + }); + }); + + test("accept during an in-flight re-query does not splice", async () => { await withShell(async (shell) => { const { source, resolveNext } = hangableSource(); setMentionSuggestionSource(shell, source); @@ -355,17 +370,46 @@ describe("mention accept requires a live @token", () => { acceptOverlaySelection(shell); expect(shell.prompt.value).toBe("read @s"); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); resolveNext(ROOT); await drainMicrotasks(); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); expect(shell.prompt.value).toBe("read @s"); }); }); + test("accept during an in-flight no-match re-query does not splice", async () => { + await withShell(async (shell) => { + const { source, resolveNext } = hangableSource(); + setMentionSuggestionSource(shell, source); + + shell.prompt.value = "read @"; + shell.prompt.cursorOffset = shell.prompt.value.length; + const first = openAtMentionSuggestions(shell); + resolveNext(ROOT); + expect(await first).toBe(true); + expect(isMentionPopupOpen(shell)).toBe(true); + + expect(handleMentionPopupKey(shell, printable("z"))).toBe(true); + expect(shell.prompt.value).toBe("read @z"); + + acceptOverlaySelection(shell); + expect(shell.prompt.value).toBe("read @z"); + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).toBeNull(); + + resolveNext([]); + await drainMicrotasks(); + + expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).toBeNull(); + expect(shell.prompt.value).toBe("read @z"); + }); + }); + test("accept with cursor off the @token does not splice", async () => { await withShell(async (shell) => { await openAt(shell, "read @"); @@ -375,7 +419,7 @@ describe("mention accept requires a live @token", () => { acceptOverlaySelection(shell); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); expect(shell.prompt.value).toBe("read @"); }); }); @@ -392,7 +436,7 @@ describe("mention accept requires a live @token", () => { acceptOverlaySelection(shell); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); expect(shell.prompt.value).toBe(value); }); }); @@ -410,7 +454,7 @@ describe("mention accept requires a live @token", () => { expect(await pending).toBe(false); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); }); }); @@ -428,11 +472,11 @@ describe("mention accept requires a live @token", () => { expect(await pending).toBe(false); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); }); }); - test("dismiss during an in-flight lookup does not reopen or splice", async () => { + test("closeMentionPopup during an in-flight lookup does not reopen", async () => { await withShell(async (shell) => { const { source, resolveNext } = hangableSource(); setMentionSuggestionSource(shell, source); @@ -450,17 +494,18 @@ describe("mention accept requires a live @token", () => { closeMentionPopup(shell); expect(isMentionPopupOpen(shell)).toBe(false); expect(shell.overlayList).toBeNull(); + expect(shell.overlayKind).toBeNull(); resolveNext(ROOT); await drainMicrotasks(); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); expect(shell.prompt.value).toBe("read @s"); }); }); - test("Esc during an in-flight lookup does not reopen or splice", async () => { + test("closeInsetOverlay during an in-flight lookup does not reopen", async () => { await withShell(async (shell) => { const { source, resolveNext } = hangableSource(); setMentionSuggestionSource(shell, source); @@ -477,12 +522,13 @@ describe("mention accept requires a live @token", () => { closeInsetOverlay(shell); expect(isMentionPopupOpen(shell)).toBe(false); + expect(shell.overlayKind).toBeNull(); resolveNext(ROOT); await drainMicrotasks(); expect(isMentionPopupOpen(shell)).toBe(false); - expect(shell.overlayKind).not.toBe("mentions"); + expect(shell.overlayKind).toBeNull(); expect(shell.prompt.value).toBe("read @s"); }); }); diff --git a/src/tui/shell.ts b/src/tui/shell.ts index 82546d9c6..9848005bf 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -4421,7 +4421,8 @@ export function pageOverlaySelection(shell: AppShell, dir: -1 | 1): void { paintOverlayList(shell); } -/** Accept active overlay item → callback + system line + close (palette dispatches action). */ +/** Accept active overlay item → callback + system line + close (palette dispatches action). + * Mention Enter that is not live (stale generation or cursor off that `@`) dismisses. */ export function acceptOverlaySelection(shell: AppShell): void { if (!shell.overlayList) return; @@ -4452,7 +4453,7 @@ export function acceptOverlaySelection(shell: AppShell): void { return; } - if (kind === "mentions" && mentionPopups.has(shell) && !mentionAcceptIsLive(shell)) { + if (kind === "mentions" && mentionPopups.has(shell) && liveMentionAccept(shell) === null) { // Stale generation or cursor off the @token: operator dismiss, not accept. closeInsetOverlay(shell); return; @@ -5060,7 +5061,7 @@ interface MentionAcceptState { * Open path suggestions for the @token under the cursor and splice the * accepted entry back into the prompt. Directory picks re-open one level * down so the operator can drill in without typing the path. - * Returns false when the cursor is not inside this @token, nothing matched, + * Returns false when the cursor is not inside an @token, nothing matched, * a newer lookup superseded this one, or the overlay host was taken. * * Accept requires a current generation and a live `@` token under the cursor. @@ -5101,21 +5102,22 @@ export async function openAtMentionSuggestions(shell: AppShell): Promise