Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/question-card-cursor-recheck.md
Original file line number Diff line number Diff line change
@@ -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.
48 changes: 48 additions & 0 deletions server/prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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())!;
Expand Down
57 changes: 42 additions & 15 deletions server/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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
Expand All @@ -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() };
Expand Down Expand Up @@ -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 <T>(look: AbortSignal, guardUntil: number | null, readUntil: number,
same: (shown: ParsedPrompt | null, screen: string) => boolean, write: () => T, screenOnly = false): Promise<T> => {
same: (shown: ParsedPrompt | null, screen: string, settled: ParsedPrompt | null) => boolean, write: () => T, screenOnly = false): Promise<T> => {
const guarding = guardKey(body.pane_id, agent, look);
guarding.catch(() => undefined);
const guarded = guardUntil === null ? await guarding.then(() => true)
Expand All @@ -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();
Expand Down Expand Up @@ -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);
};
/**
Expand All @@ -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<void> => {
const write = async (): Promise<void> => {
const first = !move && !committed;
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
}
}
Expand Down
Loading