diff --git a/changelog.d/question-card-cursor-recheck.md b/changelog.d/question-card-cursor-recheck.md new file mode 100644 index 0000000000..d880ccc038 --- /dev/null +++ b/changelog.d/question-card-cursor-recheck.md @@ -0,0 +1,4 @@ +### Fixed +- An answer from a question card no longer presses Enter on another row when the terminal's cursor + moves at the same instant, and a Hermes multi-select answer stops instead of ticking another + question when the terminal switches questions after its first tick. diff --git a/server/prompt.test.ts b/server/prompt.test.ts index 91c9f80249..c7d1ab3aea 100644 --- a/server/prompt.test.ts +++ b/server/prompt.test.ts @@ -5730,6 +5730,54 @@ Enter to select · ↑/↓ to navigate · Esc to cancel }); }); + describe("Hermes's question, changed in the terminal between the last look and a key", () => { + const hermes = (question: string, at: number, ticked: number[] = [], multi = false) => `\n ask 1 question\n ▸ ${question}\n${["Red", "Blue", "Green"].map((row, index) => ` ${at === index ? "▸" : " "} ${multi ? (ticked.includes(index) ? "[x] " : "[ ] ") : ""}${index + 1}. ${row}`).join("\n")}\n ${at === 3 ? "▸" : " "} 4. Other (type your answer)\n 0/1 answered · ${multi ? "Space toggle · " : ""}↑/↓ select · Enter confirm and continue · Tab/Shift+Tab switch question · Esc/Ctrl+C cancel\n ─ (⌐■_■) deliberating… · 10s │ opus 5.5\n`; + + // the reads after the move: the look that finds Blue under the cursor, the read before Enter, its settling read + for (const [moveOn, read] of [[2, "the read before Enter"], [3, "the settling read before Enter"]] as const) { + test(`an arrow pressed in the terminal by ${read}: no Enter on the row it moved to`, async () => { + await withPane("hermes", "blocked", hermes("Pick a colour", 0), async (pane) => { + const prompt = (await card())!; + let reads = 0; + pane.onSent = (sent) => { if (sent === "down") { pane.screen = hermes("Pick a colour", 1); reads = 0; } }; + pane.onRead = () => { reads += 1; if (pane.sent.length > 0 && reads === moveOn) pane.screen = hermes("Pick a colour", 2); }; + const result = await answer(prompt.id, { option_index: 1 }); + expect(reads, "the cursor moved under the answer").toBeGreaterThanOrEqual(moveOn); + expect(pane.sent, "Enter would answer Green, not the Blue tapped on the card").toEqual(["down"]); + expect(result).toEqual({ status: 409, code: "prompt_changed" }); + }); + }); + } + + test("a multi-select answer stops once the terminal ticks a row the answer's next Space would untick", async () => { + await withPane("hermes", "blocked", hermes("Pick colours", 0, [], true), async (pane) => { + const prompt = (await card())!; + let at = 0; + const ticked: number[] = []; + pane.onSent = (sent) => { + if (sent === "down") at += 1; + // Space in the terminal on Green right after the answer's first one + if (sent === "space") ticked.push(at, ...(ticked.length === 0 ? [2] : [])); + pane.screen = hermes("Pick colours", at, ticked, true); + }; + const result = await answer(prompt.id, { option_indices: [0, 2] }); + expect(pane.sent, "a second Space on Green would untick it and Enter would lock Red alone").toEqual(["space", "down", "down"]); + expect(result).toEqual({ status: 409, code: "prompt_changed" }); + }); + }); + + test("a multi-select answer stops once the terminal switches to another question after the first tick", async () => { + await withPane("hermes", "blocked", hermes("Pick colours", 0, [], true), async (pane) => { + const prompt = (await card())!; + // Tab in the terminal right after the first Space: the next question, drawn alike + pane.onSent = (sent) => { if (sent === "space") pane.screen = hermes("Pick more colours", 0, [], true); }; + const result = await answer(prompt.id, { option_indices: [0, 2] }); + expect(pane.sent, "the other question must not take the remaining moves, ticks or Enter").toEqual(["space"]); + expect(result).toEqual({ status: 409, code: "prompt_changed" }); + }); + }); + }); + test("an interaction plan sends no commit when Claude's highlight moves past the requested model", async () => { await withPane("claude", "idle", claudeModelList(1), async (pane) => { const prompt = (await card())!; diff --git a/server/prompt.ts b/server/prompt.ts index 467709770f..07c07880fe 100644 --- a/server/prompt.ts +++ b/server/prompt.ts @@ -879,7 +879,8 @@ async function readKnownPrompt( * read before and parsed with what this read found; a card the screen no longer draws so authorizes no key */ settle = false, -): Promise<{ prompt: InteractivePrompt | null; screen?: string; stale?: boolean; inputUnverified?: true; suppressFallback?: true }> { +): Promise<{ prompt: InteractivePrompt | null; screen?: string; stale?: boolean; inputUnverified?: true; suppressFallback?: true; + /** the card as the settling read drew it: the same card, its cursor where the screen has it now */ settled?: InteractivePrompt }> { // no native card (no session herdr reports, no verified v2 service, no pending form): OpenCode // v1's own question prompt is read off the screen below (dialogs/opencode.ts, #1386) native: if (agent === "opencode") { @@ -1138,6 +1139,7 @@ async function readKnownPrompt( if (parsedByPublicPrompt.get(fallback)?.responder === "fallback-gjc-menu") return { prompt: fallback, screen }; } const count = agent === "codex" && prompt === null ? queuedQuestionCount(screen) : 0; + let settled: InteractivePrompt | undefined; if (settle && prompt) { // read as the card was read (its colours, or the screen's text), after every await above: parsed with no await // between, so the card checked is the one the screen draws right now @@ -1149,8 +1151,9 @@ async function readKnownPrompt( const now = again ? rebind(again) : null; const at = (card: InteractivePrompt): number | undefined => card.steps?.findIndex(step => step.current); if (!now || now.id !== prompt.id || at(now) !== at(prompt)) inputUnverified = true; + else settled = now; } - if (count === 0 || !pane.cwd) return { prompt, screen, inputUnverified }; + if (count === 0 || !pane.cwd) return { prompt, screen, inputUnverified, ...(settled ? { settled } : {}) }; let rollout = queueRollouts.get(paneId); if (!rollout || Date.now() - rollout.at > QUEUE_ROLLOUT_MS) { rollout = { path: await codexTranscriptPath(paneId, pane.cwd, await paneCodexHome(paneId, codexHome)), at: Date.now() }; @@ -1408,7 +1411,7 @@ export async function handlePromptRequest(request: Request, url: URL, options: P * first row), whose read has no card of its own to settle. */ const verifyThenSend = async (look: AbortSignal, guardUntil: number | null, readUntil: number, - same: (shown: ParsedPrompt | null, screen: string) => boolean, write: () => T, screenOnly = false): Promise => { + same: (shown: ParsedPrompt | null, screen: string, settled: ParsedPrompt | null) => boolean, write: () => T, screenOnly = false): Promise => { const guarding = guardKey(body.pane_id, agent, look); guarding.catch(() => undefined); const guarded = guardUntil === null ? await guarding.then(() => true) @@ -1420,7 +1423,8 @@ export async function handlePromptRequest(request: Request, url: URL, options: P reading.catch(() => undefined); const read = await Promise.race([reading, Bun.sleep(Math.max(0, readUntil - Date.now())).then(() => null)]); const shown = read?.prompt ? parsedByPublicPrompt.get(read.prompt) ?? null : null; - if (!read || read.inputUnverified || Date.now() > readUntil || !same(shown, read.screen ?? "")) throw new KeyRefused(); + const settled = read?.settled ? parsedByPublicPrompt.get(read.settled) ?? null : null; + if (!read || read.inputUnverified || Date.now() > readUntil || !same(shown, read.screen ?? "", settled)) throw new KeyRefused(); // pi's select and confirm compare by the dialog drawn, which pi leaves on the screen when it ends, and a // dialog moved off its first row has no card to compare: pi must still hold the pane after this read too if ((parsedPrompt?.responder === "pi-question" || parsedPrompt?.responder === "pi-confirm") && !(await piOwnsInput(body.pane_id))) throw new KeyRefused(); @@ -1840,7 +1844,7 @@ export async function handlePromptRequest(request: Request, url: URL, options: P const dialog = parsePiDialog(screen, true); return dialog?.id === parsed.id && dialog.selectedIndex === cursor && promptTailIsActive(dialog, screen); } - return shown?.id === parsed.id && shown.selectedIndex === cursor + return shown?.id === parsed.id && shown.selectedIndex === cursor && ticked(shown) && (responder !== "pi-questionnaire-question" || shown.piQuestionnaireInput === undefined); }; /** @@ -1866,16 +1870,34 @@ export async function handlePromptRequest(request: Request, url: URL, options: P await Bun.sleep(50); } }; - /** Whether a read still shows the question this answer is for (its id carries its call and, for questions drawn alike, its tab). */ - const sameQuestion = (shown: ParsedPrompt | null, screen: string): boolean => { + /** + * Whether a read still shows the question this answer is for (its id carries its call and, for questions drawn alike, its tab). + * `onCursor`: a key that acts on the row under the cursor (Enter, Space), which must still be the row the moves left it + * on, in the read and in its settling read (`settled`, the screen as it is right before the key). + */ + const sameQuestion = (shown: ParsedPrompt | null, screen: string, onCursor: boolean, settled: ParsedPrompt | null): boolean => { if (interaction) return shown?.responder === responder && interaction.matches(screen); - if (responder === "pi-question" || responder === "pi-confirm") return parsePiDialog(screen, true)?.id === parsed.id; + if (responder === "pi-question" || responder === "pi-confirm") { + const dialog = parsePiDialog(screen, true); + return dialog?.id === parsed.id && (!onCursor || dialog.selectedIndex === cursor); + } // a fallback card is the screen's own (readPrompt draws it when no reader knows the screen): the same card by // what it asks and offers, whatever else ticked on the screen since (a spinner, a timer, a token count) - if (responder.startsWith("fallback")) return parsedByPublicPrompt.get(parseFallbackPrompt(agent, screen))?.id === parsed.id; + if (responder.startsWith("fallback")) { + const card = parsedByPublicPrompt.get(parseFallbackPrompt(agent, screen)); + return card?.id === parsed.id && (!onCursor || card.selectedIndex === cursor); + } return shown?.id === parsed.id && shown.question === parsed.question && shown.ompCall === parsed.ompCall - && shown.nativeSteps?.index === parsed.nativeSteps?.index; + && shown.nativeSteps?.index === parsed.nativeSteps?.index + && (!onCursor || (shown.selectedIndex === cursor && ticked(shown) && (settled === null || (settled.selectedIndex === cursor && ticked(settled))))); }; + // Hermes's multi-select: every key, the ticks after the first and the Enter that locks them, is on the same + // question, so each is checked against it as the first was, with the rows ticked as this answer's own Spaces left them + const togglesInPlace = responder === "hermes-question" && body.option_indices !== undefined; + const ticks = new Set(parsed.checkedOptionIndices); + function ticked(shown: ParsedPrompt): boolean { + return !togglesInPlace || (shown.checkedOptionIndices.length === ticks.size && shown.checkedOptionIndices.every((index) => ticks.has(index))); + } const sendStep = async (step: AnswerStep, move: boolean, more: boolean): Promise => { const write = async (): Promise => { const first = !move && !committed; @@ -1894,9 +1916,11 @@ export async function handlePromptRequest(request: Request, url: URL, options: P // with, as before. pi's look is as it was (pi-ask's own check, awaited with the request's deadline). const look = agent === "pi" ? deadline : AbortSignal.timeout(SETTLE_MS); const guardUntil = agent === "pi" ? null : Date.now() + SETTLE_MS; - if (!committed) { + if (!committed || togglesInPlace) { // every key up to the answer's first one that cannot be undone, that one included: the question as asked - const { written } = await verifyThenSend(look, guardUntil, Date.now() + SETTLE_MS, sameQuestion, () => ({ written: write() }), + const onCursor = !move && (step.keys?.every((key) => key === KEY.enter || key === KEY.space) ?? false); + const { written } = await verifyThenSend(look, guardUntil, Date.now() + SETTLE_MS, + (shown, screen, settled) => sameQuestion(shown, screen, onCursor, settled), () => ({ written: write() }), interaction !== undefined || responder === "pi-question" || responder === "pi-confirm" || responder.startsWith("fallback")); await written; } else { @@ -1970,10 +1994,11 @@ export async function handlePromptRequest(request: Request, url: URL, options: P } // GJC's selector can close under the answer: its composer must not take the moves. if (move && walked && (list || reader?.reread === "each-move" || responder === "fallback-gjc-menu") && !(await onRow())) return promptChanged(); - if (!move && !committed) { + if (!move && (!committed || togglesInPlace)) { // The first irreversible step goes only once the menu shows the expected cursor. - // Later keys follow as before: the menu itself changes under them. - if (moved && !(await onRow())) return promptChanged(); + // Later keys follow as before: the menu itself changes under them. Hermes's ticks and + // Enter after the first tick wait for the cursor and the ticks the last keys left. + if ((moved || (togglesInPlace && committed)) && !(await onRow())) return promptChanged(); if (request.signal.aborted) return promptChanged(); } // herdr may press the key and lose its reply; a possibly pressed key ends the asking. @@ -1984,6 +2009,8 @@ export async function handlePromptRequest(request: Request, url: URL, options: P moved = true; walked = true; for (const key of step.keys!) cursor = key === KEY.down || key === KEY.right ? cursor + 1 : Math.max(0, cursor - 1); + } else if (togglesInPlace && step.keys?.length === 1 && step.keys[0] === KEY.space) { + if (!ticks.delete(cursor)) ticks.add(cursor); } } }