mirror of https://github.com/garrytan/gstack.git
fix(evals): parse single-logical-line AskUserQuestions in the PTY runner
When the PTY reflows a boxed AUQ, ALL options land on ONE logical line after stripAnsi — parseNumberedOptions parsed one option per line, found only '1.', and the >=2 check failed forever while the correct question sat on screen (plan-design-with-ui timed out this way twice, with the rendered scope-gate AUQ visible in both failure buffers). The cursor line is now parsed as a stream of ascending N. tokens; DEC cursor- visibility residue is stripped before matching; plan-design-with-ui's budgets grow to fit observed ~6min preamble+thinking latency. Pinned by test/pty-auq-single-line.test.ts using the real failure buffers; all 142 existing parser-consumer unit tests still green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
bd11416d80
commit
828b229900
|
|
@ -305,6 +305,17 @@ export function isPermissionDialogVisible(visible: string): boolean {
|
|||
}
|
||||
|
||||
/** Detect any AskUserQuestion-shaped numbered option list with cursor. */
|
||||
/**
|
||||
* Strip terminal residue that survives ANSI-stripping and can interleave
|
||||
* with AUQ text: DEC cursor-visibility fragments (`[?25l` / `[?25h` — the ESC
|
||||
* byte is gone but the bracket sequence remains) and the spinner frames
|
||||
* rendered between them. Observed in plan-design-with-ui's failure buffer,
|
||||
* where `[?25l✻Sprouting…[?25h` fragments sat inside the option lines.
|
||||
*/
|
||||
export function stripPtyResidue(visible: string): string {
|
||||
return visible.replace(/\[\?25[lh]/g, '');
|
||||
}
|
||||
|
||||
export function isNumberedOptionListVisible(visible: string): boolean {
|
||||
// ❯ cursor + at least two numbered options 1-9.
|
||||
// Matches the trust dialog AND plan-ready prompt AND skill questions.
|
||||
|
|
@ -316,7 +327,8 @@ export function isNumberedOptionListVisible(visible: string): boolean {
|
|||
// because `t-2` is a word-to-word transition. We use the weaker
|
||||
// `[^0-9]2\.` to require a non-digit before `2` (so we don't match
|
||||
// `12.0`) without requiring whitespace.
|
||||
return /❯\s*1\./.test(visible) && /(^|[^0-9])2\./.test(visible);
|
||||
const cleaned = stripPtyResidue(visible);
|
||||
return /❯\s*1\./.test(cleaned) && /(^|[^0-9])2\./.test(cleaned);
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
|
|
@ -692,6 +704,7 @@ export function isScopeGateAutoSelectVisible(visible: string): boolean {
|
|||
export function parseNumberedOptions(
|
||||
visible: string,
|
||||
): Array<{ index: number; label: string }> {
|
||||
visible = stripPtyResidue(visible);
|
||||
const tail = visible.length > 4096 ? visible.slice(-4096) : visible;
|
||||
// Split on lines, look for `❯ N.` or ` N.` patterns. Up to N=9.
|
||||
// The `\s*` after `.` (not `\s+`) is required because stripAnsi removes
|
||||
|
|
@ -733,30 +746,41 @@ export function parseNumberedOptions(
|
|||
const seenIndices = new Set<number>();
|
||||
|
||||
// Cursor line: option 1 may be inline after box dividers + prompt header
|
||||
// (`...divider...header...❯1. label`). Use a non-anchored regex that
|
||||
// captures `❯N. label` from anywhere on the line through end-of-line.
|
||||
// Only used for the cursor line — subsequent options are parsed with the
|
||||
// start-of-line `optionRe`.
|
||||
// (`...divider...header...❯1. label`) — and, when the PTY reflows the whole
|
||||
// AUQ onto ONE logical line, options 2..N sit on the SAME line after it
|
||||
// (observed with /plan-design-review's Step-0 scope gate: `❯1.Branch diff
|
||||
// ... 2.Plan or design doc ... 5.Chat about this ... Enter to select`).
|
||||
// Parse the cursor line as a STREAM: find every `N.` token (not preceded
|
||||
// by a digit, not followed by one — excludes "12." and "1.5"), require
|
||||
// ascending indices starting from the cursor's option, and take each
|
||||
// label as the text between successive number tokens.
|
||||
const cursorLine = lines[cursorLineIdx] ?? '';
|
||||
const cursorInlineRe = /❯\s*([1-9])\.\s*(\S.*?)\s*$/;
|
||||
const inlineMatch = cursorInlineRe.exec(cursorLine);
|
||||
if (inlineMatch) {
|
||||
const idx = Number(inlineMatch[1]);
|
||||
const label = (inlineMatch[2] ?? '').trim();
|
||||
if (label.length > 0 && !seenIndices.has(idx)) {
|
||||
seenIndices.add(idx);
|
||||
found.push({ index: idx, label });
|
||||
}
|
||||
} else {
|
||||
// No inline cursor match — fall back to start-of-line regex.
|
||||
const startMatch = optionRe.exec(cursorLine);
|
||||
if (startMatch) {
|
||||
const idx = Number(startMatch[1]);
|
||||
const label = (startMatch[2] ?? '').trim();
|
||||
if (label.length > 0 && !seenIndices.has(idx)) {
|
||||
seenIndices.add(idx);
|
||||
found.push({ index: idx, label });
|
||||
}
|
||||
const cursorStart = cursorLine.indexOf('❯');
|
||||
const cursorSegment = cursorStart >= 0 ? cursorLine.slice(cursorStart) : cursorLine;
|
||||
const tokenRe = /(?:^|[^0-9])([1-9])\.(?!\d)\s*/g;
|
||||
const tokens: Array<{ idx: number; labelStart: number; matchStart: number }> = [];
|
||||
for (let m = tokenRe.exec(cursorSegment); m !== null; m = tokenRe.exec(cursorSegment)) {
|
||||
tokens.push({
|
||||
idx: Number(m[1]),
|
||||
labelStart: m.index + m[0].length,
|
||||
matchStart: m.index === 0 ? 0 : m.index + 1, // skip the [^0-9] guard char
|
||||
});
|
||||
}
|
||||
// Keep only the ascending run that starts the sequence (1, 2, 3, ...);
|
||||
// stray numbers inside labels break ascension and end the run.
|
||||
let expected = 1;
|
||||
for (let t = 0; t < tokens.length; t++) {
|
||||
const token = tokens[t]!;
|
||||
if (token.idx !== expected) continue;
|
||||
const next = tokens
|
||||
.slice(t + 1)
|
||||
.find((candidate) => candidate.idx === expected + 1 && candidate.matchStart > token.labelStart);
|
||||
const labelEnd = next ? next.matchStart : cursorSegment.length;
|
||||
const label = cursorSegment.slice(token.labelStart, labelEnd).trim();
|
||||
if (label.length > 0 && !seenIndices.has(token.idx)) {
|
||||
seenIndices.add(token.idx);
|
||||
found.push({ index: token.idx, label });
|
||||
expected += 1;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,65 @@
|
|||
/**
|
||||
* Pins single-logical-line AskUserQuestion detection (the plan-design-with-ui
|
||||
* gate-timeout class). When the PTY reflows a boxed AUQ, ALL options land on
|
||||
* ONE logical line after stripAnsi — the per-line option parser then finds
|
||||
* only option 1 and the >= 2 check fails forever while the (correct) question
|
||||
* sits on screen. Both fixture strings below are condensed from REAL observed
|
||||
* failure buffers of test/skill-e2e-plan-design-with-ui.test.ts.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'bun:test';
|
||||
import {
|
||||
isNumberedOptionListVisible,
|
||||
parseNumberedOptions,
|
||||
stripPtyResidue,
|
||||
} from './helpers/claude-pty-runner';
|
||||
|
||||
// Condensed from the 2026-08-13 failure buffer: whole AUQ on one logical
|
||||
// line — dividers, cursor option, options 2-5, footer. Note the missing
|
||||
// spaces ("2.Planordesigndoc") from stripped cursor-positioning escapes.
|
||||
const SINGLE_LINE_AUQ =
|
||||
'────────────Planning: /tmp/x/.claude/plans/soft-questing-jellyfish.md──────────' +
|
||||
' ☐ Review target What should I review? <gstack-qid:plan-design-review-scope-gate>' +
|
||||
'❯1.Branch diff (current WIP) Review the design implications of what changed. ' +
|
||||
'Recommendation: A when a branch diff exists.2.Planordesigndoc Paste or point me to a plan file. ' +
|
||||
'3. Spcific page, file, r pah Name a specific file. 4. Type something.' +
|
||||
'────────────5.ChataboutthisEnter to select · ↑/↓ o navigate · Escto cancel';
|
||||
|
||||
// The same AUQ with DEC cursor-visibility residue + spinner frames
|
||||
// interleaved, as captured before stripPtyResidue existed.
|
||||
const RESIDUE_AUQ =
|
||||
'[?25l✻Sprouting…[?25h[?25l✶[?25h' + SINGLE_LINE_AUQ.slice(0, 200) +
|
||||
'[?25l·still thinking[?25h' + SINGLE_LINE_AUQ.slice(200);
|
||||
|
||||
describe('single-logical-line AUQ detection', () => {
|
||||
it('isNumberedOptionListVisible matches the reflowed one-line AUQ', () => {
|
||||
expect(isNumberedOptionListVisible(SINGLE_LINE_AUQ)).toBe(true);
|
||||
});
|
||||
|
||||
it('parseNumberedOptions finds the full ascending option run on one line', () => {
|
||||
const options = parseNumberedOptions(SINGLE_LINE_AUQ);
|
||||
expect(options.length).toBeGreaterThanOrEqual(2);
|
||||
expect(options[0]).toEqual({ index: 1, label: expect.stringContaining('Branch diff') });
|
||||
expect(options[1]?.index).toBe(2);
|
||||
});
|
||||
|
||||
it('survives DEC residue + spinner interleave', () => {
|
||||
expect(isNumberedOptionListVisible(RESIDUE_AUQ)).toBe(true);
|
||||
expect(parseNumberedOptions(RESIDUE_AUQ).length).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
|
||||
it('stripPtyResidue removes cursor-visibility fragments only', () => {
|
||||
expect(stripPtyResidue('[?25la[?25hb')).toBe('ab');
|
||||
expect(stripPtyResidue('keep [?250] this')).toBe('keep [?250] this');
|
||||
});
|
||||
|
||||
it('still rejects prose numbered lists (no cursor sigil)', () => {
|
||||
expect(parseNumberedOptions('steps: 1. Read the file 2. Edit it 3. Done')).toEqual([]);
|
||||
});
|
||||
|
||||
it('still parses classic multi-line AUQs', () => {
|
||||
const multiLine = 'What should I do?\n❯ 1. First option\n 2. Second option\n 3. Third option\n';
|
||||
const options = parseNumberedOptions(multiLine);
|
||||
expect(options.map((o) => o.index)).toEqual([1, 2, 3]);
|
||||
});
|
||||
});
|
||||
|
|
@ -44,7 +44,7 @@ describeE2E('/plan-design-review with UI scope (gate)', () => {
|
|||
const session = await launchClaudePty({
|
||||
permissionMode: 'plan',
|
||||
cwd: ROOT,
|
||||
timeoutMs: 480_000,
|
||||
timeoutMs: 720_000,
|
||||
seedSkills: true,
|
||||
});
|
||||
|
||||
|
|
@ -71,7 +71,11 @@ describeE2E('/plan-design-review with UI scope (gate)', () => {
|
|||
`Reference plan file: ${fixtureRelPath}\r`
|
||||
);
|
||||
|
||||
const budgetMs = 360_000;
|
||||
// 600s, not 360s: the skill preamble (update-check, session bookkeeping,
|
||||
// learnings) plus extended model thinking can take ~6 minutes before the
|
||||
// scope-gate AskUserQuestion renders — a 360s budget expired seconds
|
||||
// before the (correct) AUQ appeared in the observed failure transcript.
|
||||
const budgetMs = 600_000;
|
||||
const start = Date.now();
|
||||
let lastPermSig = '';
|
||||
while (Date.now() - start < budgetMs) {
|
||||
|
|
@ -146,6 +150,6 @@ describeE2E('/plan-design-review with UI scope (gate)', () => {
|
|||
);
|
||||
}
|
||||
},
|
||||
540_000,
|
||||
780_000,
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue