diff --git a/src/web/response-viewer-transcript.ts b/src/web/response-viewer-transcript.ts index 1404aa81..2cfe523b 100644 --- a/src/web/response-viewer-transcript.ts +++ b/src/web/response-viewer-transcript.ts @@ -3,20 +3,68 @@ export type ResponseViewerTranscriptKind = 'prompt' | 'response' | 'status' | 't export interface ResponseViewerTranscriptBlock { kind: ResponseViewerTranscriptKind; label: 'Prompt' | 'Response' | 'Status' | 'Tool'; + /** What the frontend renders by: 'user' gets the "You" badge, everything else the agent badge. */ + role: 'user' | 'assistant'; text: string; } -const EXTERNAL_CLI_MODES = new Set(['codex', 'gemini', 'opencode', 'antigravity']); +// Keep in lockstep with isExternalCliMode() in src/session.ts. Importing it here +// would drag node-pty and the whole session layer into this pure module, so the +// list is duplicated and test/response-viewer-transcript.test.ts pins the parity. +const EXTERNAL_CLI_MODES = new Set(['codex', 'gemini', 'opencode', 'antigravity', 'pi']); function isPromptLine(line: string): boolean { return /^\s*›\s*/.test(line); } +function isDividerDashChar(ch: string): boolean { + return ch === '─' || ch === '-'; +} + +function isWhitespaceChar(ch: string): boolean { + return /\s/.test(ch); +} + +// Linear-time equivalent of the old /^[─-]+\s*(.+?)\s*[─-]{3,}$/. The lazy +// middle of that pattern backtracked catastrophically on a long dash run that +// does NOT end in 3+ dashes (measured >2min at 8,000 chars) — and pane text is +// agent-controlled with buffers up to 32MB, so this ran on hostile input. Same +// accept set and same captured content, computed with counters; equivalence is +// pinned char-for-char against the old regex by the brute-force corpus test in +// test/response-viewer-transcript.test.ts. function normalizeDividerStatusLine(line: string): string | null { - const trimmed = line.trim(); - const matched = trimmed.match(/^[─-]+\s*(.+?)\s*[─-]{3,}$/); - if (!matched) return null; - return matched[1]?.trim() || null; + const s = line.trim(); + const n = s.length; + // Minimum match: 1 leading dash + 1 content char + 3 trailing dashes. + if (n < 5) return null; + + let lead = 0; + while (lead < n && isDividerDashChar(s.charAt(lead))) lead += 1; + if (lead === 0) return null; + + let trail = 0; + while (trail < n && isDividerDashChar(s.charAt(n - 1 - trail))) trail += 1; + if (trail < 3) return null; + + // The regex was greedy on the leading run but gave dashes back until at least + // one content char plus the 3-dash tail fit (an all-dash line matched with a + // single leftover dash as its "content"), so the content window starts at the + // end of the leading run, clamped to leave 4 chars. + const contentStart = Math.min(lead, n - 4); + let ws = 0; + while (contentStart + ws < n - 4 && isWhitespaceChar(s.charAt(contentStart + ws))) ws += 1; + const from = contentStart + ws; + + // The lazy middle stopped at the first position from which "optional + // whitespace, then dashes to end-of-line" matches: the start of the + // whitespace padding in front of the trailing dash run (never before the + // first content char). + const tailStart = n - trail; + let padded = tailStart; + while (padded > 0 && isWhitespaceChar(s.charAt(padded - 1))) padded -= 1; + const end = Math.max(from + 1, padded); + + return s.slice(from, end).trim() || null; } function isDividerOnlyLine(line: string): boolean { @@ -208,7 +256,9 @@ function pushBlock( : normalizeWrappedText(normalizedLines); if (!text) return; const label = (kind.charAt(0).toUpperCase() + kind.slice(1)) as ResponseViewerTranscriptBlock['label']; - blocks.push({ kind, label, text }); + // The frontend's loadFullContext() renders via msg.role — a block without it + // lost the "You" badge on prompts and rendered every block as the agent. + blocks.push({ kind, label, role: kind === 'prompt' ? 'user' : 'assistant', text }); } export function isExternalCliTranscriptMode(mode: string | null | undefined): boolean { diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index aa012142..f5f838a1 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1903,11 +1903,11 @@ export function registerSessionRoutes( return await readCodexLastResponse(session, codexQuery.context === 'full'); } - // OpenCode / Gemini / Antigravity render their own TUIs and write no Claude - // transcript, so the scan below finds nothing and the response viewer renders - // permanently empty for them. Segment the terminal buffer instead — the pane - // IS the transcript for these CLIs. Codex is already handled above, where a - // real rollout file is the better source. + // OpenCode / Gemini / Antigravity / Pi render their own TUIs and write no + // Claude transcript, so the scan below finds nothing and the response viewer + // renders permanently empty for them. Segment the terminal buffer instead — + // the pane IS the transcript for these CLIs. Codex is already handled above, + // where a real rollout file is the better source. if (isExternalCliTranscriptMode(session.mode)) { const externalQuery = req.query as { context?: string }; const blocks = parseExternalCliTranscript(session.terminalBuffer, session.mode); diff --git a/test/response-viewer-transcript.test.ts b/test/response-viewer-transcript.test.ts index 818c69d7..68616943 100644 --- a/test/response-viewer-transcript.test.ts +++ b/test/response-viewer-transcript.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { getLastTranscriptResponse, parseExternalCliTranscript } from '../src/web/response-viewer-transcript.js'; +import { + getLastTranscriptResponse, + isExternalCliTranscriptMode, + parseExternalCliTranscript, +} from '../src/web/response-viewer-transcript.js'; +import { isExternalCliMode } from '../src/session.js'; +import type { SessionMode } from '../src/types.js'; describe('response viewer transcript parser', () => { it('extracts structured COD transcript blocks and keeps labeled status/tool entries', () => { @@ -46,6 +52,15 @@ gpt-5.4 medium · kb · main · Ready · Context 92% left expect(blocks[3]?.label).toBe('Tool'); expect(blocks[4]?.text).toContain('reduces the risk and effort involved'); expect(blocks[4]?.text).not.toContain('reduces\nthe risk'); + + // The frontend's loadFullContext() renders via msg.role: 'user' gets the + // "You" badge, everything else the agent badge. A block without role + // rendered every prompt as the agent. + expect(blocks[1]?.role).toBe('user'); + expect(blocks[5]?.role).toBe('user'); + expect(blocks[0]?.role).toBe('assistant'); + expect(blocks[3]?.role).toBe('assistant'); + expect(blocks[4]?.role).toBe('assistant'); }); it('returns the most recent completed response when the last block is a new prompt', () => { @@ -320,4 +335,128 @@ Here is the assistant answer at column zero. expect(getLastTranscriptResponse(blocks)).toBe('Here is the assistant answer at column zero.'); }); }); + + // The divider-status detector used to be /^[─-]+\s*(.+?)\s*[─-]{3,}$/, whose + // lazy middle backtracked catastrophically on a long dash run that does not + // end in 3+ dashes (measured >2min at 8,000 chars). Pane text is + // agent-controlled and buffers reach 32MB, so the pattern was replaced by a + // linear counter walk. Same approach as the glob-matcher ReDoS fix (68ae9a8): + // the hostile input below fails by timeout with the RegExp version. + describe('divider-status ReDoS hardening', () => { + it('classifies a hostile 10k dash run in linear time', () => { + const hostile = '-'.repeat(10_000) + '>'; + const started = Date.now(); + const blocks = parseExternalCliTranscript(`› q\n${hostile}`, 'opencode'); + const elapsed = Date.now() - started; + + expect(elapsed).toBeLessThan(2_000); + // No 3-dash tail, so the line is not a status divider — it stays prose. + expect(blocks.some((b) => b.kind === 'response' && b.text.includes('>'))).toBe(true); + expect(blocks.some((b) => b.kind === 'status')).toBe(false); + }); + + // The counter walk must keep the EXACT accept set and captured content of + // the old regex (including its backtracking quirks on all-dash lines), so + // brute-force both against a corpus. The old regex is safe here: probes are + // capped at 24 chars, far below the blowup threshold. + it('matches the old regex char-for-char on a brute-force corpus', () => { + const oldNormalize = (line: string): string | null => { + const trimmed = line.trim(); + const matched = trimmed.match(/^[─-]+\s*(.+?)\s*[─-]{3,}$/); + if (!matched) return null; + return matched[1]?.trim() || null; + }; + + // Observe the private normalizeDividerStatusLine through the parser: after + // a prompt, a column-zero probe from this alphabet is a status block iff + // the divider detector matched, and the status text is its return value. + // (The alphabet triggers no other status arm; letters are limited to 'x'.) + const observedStatusText = (probe: string): string | null => { + const blocks = parseExternalCliTranscript(`› q\n${probe}`, 'opencode'); + const status = blocks.find((b) => b.kind === 'status'); + return status ? status.text : null; + }; + + const directed = [ + '──── Status ────', + '─ Worked for 1m 51s ───────', + '---', + '----', + '-----', + '------', + '-------', + '---- ---', + '- ---', + '--x', + 'x---', + '----x', + '----x--', + '----x---', + '─x───', + '- x ---', + '--- x -', + '─── ──', + '-\tx\t---', + '--->----', + '─-─- x x ─-─', + '- x x x ----', + ]; + + // Deterministic LCG so a failure reproduces byte-identically. + let seed = 0x2f6e2b1; + const rand = () => { + seed = (seed * 1103515245 + 12345) & 0x7fffffff; + return seed / 0x80000000; + }; + const alphabet = ['-', '-', '─', '─', ' ', ' ', '\t', 'x', '>']; + const probes = [...directed]; + for (let i = 0; i < 3000; i += 1) { + const len = Math.floor(rand() * 25); + let probe = ''; + for (let j = 0; j < len; j += 1) { + probe += alphabet[Math.floor(rand() * alphabet.length)]; + } + probes.push(probe); + } + + for (const raw of probes) { + const probe = raw.trim(); + // Divider-only lines (8+ dash/whitespace chars) are consumed by + // isDividerOnlyLine before the status detector ever runs, in both the + // old and new worlds — the detector is unobservable there. + if (!probe || /^[\s─-]{8,}$/.test(probe)) continue; + expect(observedStatusText(probe), `probe: ${JSON.stringify(probe)}`).toBe(oldNormalize(probe)); + } + }); + }); + + // EXTERNAL_CLI_MODES is a local duplicate of isExternalCliMode() in + // src/session.ts (importing session.ts would drag node-pty and the whole + // session layer into the pure transcript module). This pin is what keeps the + // two from drifting — 'pi' was missing here while isExternalCliMode() had it, + // which left pi sessions with the exact empty-viewer symptom the transcript + // branch exists to fix. + describe('mode parity with isExternalCliMode()', () => { + // Exhaustive by construction: adding a SessionMode without deciding its + // transcript behavior fails to compile here. + const ALL_SESSION_MODES: Record = { + claude: true, + shell: true, + opencode: true, + codex: true, + gemini: true, + antigravity: true, + pi: true, + }; + + it('agrees with isExternalCliMode() for every SessionMode', () => { + for (const mode of Object.keys(ALL_SESSION_MODES) as SessionMode[]) { + expect(isExternalCliTranscriptMode(mode), `mode: ${mode}`).toBe(isExternalCliMode(mode)); + } + }); + + it('covers pi', () => { + expect(isExternalCliTranscriptMode('pi')).toBe(true); + }); + }); }); diff --git a/test/routes/session-routes-external-cli-last-response.test.ts b/test/routes/session-routes-external-cli-last-response.test.ts index 9853ea1c..5bddd07b 100644 --- a/test/routes/session-routes-external-cli-last-response.test.ts +++ b/test/routes/session-routes-external-cli-last-response.test.ts @@ -4,12 +4,14 @@ * Uses app.inject() — no real HTTP ports needed. * Port: N/A (app.inject doesn't open ports) * - * OpenCode / Gemini / Antigravity render their own TUIs and never write a Claude - * transcript under ~/.claude/projects, so before this branch existed the handler - * fell through to the Claude scan, found nothing, and the response viewer was - * permanently empty for those modes. These tests pin: + * OpenCode / Gemini / Antigravity / Pi render their own TUIs and never write a + * Claude transcript under ~/.claude/projects, so before this branch existed the + * handler fell through to the Claude scan, found nothing, and the response viewer + * was permanently empty for those modes. These tests pin: * - the pane buffer is segmented and the LAST response is returned * - ?context=full carries the parsed blocks, and the short form omits them + * - ?context=full blocks carry role — the frontend renders via msg.role, so a + * block without it lost the "You" badge on prompts * - a pane that has produced no output reports hasContext: false rather than 404ing * - Claude mode still takes the Claude path (regression guard) */ @@ -100,7 +102,7 @@ describe('GET /api/sessions/:id/last-response — external CLI panes', () => { return { res, body: JSON.parse(res.body) }; } - for (const mode of ['opencode', 'gemini', 'antigravity'] as const) { + for (const mode of ['opencode', 'gemini', 'antigravity', 'pi'] as const) { it(`returns the last assistant response from the ${mode} pane buffer`, async () => { session.mode = mode; session.terminalBuffer = PANE; @@ -131,6 +133,12 @@ describe('GET /api/sessions/:id/last-response — external CLI panes', () => { const prompts = body.data.messages.filter((b: { kind: string }) => b.kind === 'prompt'); expect(prompts).toHaveLength(2); expect(prompts[1].text).toContain('now document it'); + + // loadFullContext() renders via msg.role — without it every block got the + // agent badge and the user's own prompts lost their "You" attribution. + for (const block of body.data.messages as Array<{ kind: string; role: string }>) { + expect(block.role).toBe(block.kind === 'prompt' ? 'user' : 'assistant'); + } }); it('reports hasContext false for a pane that has produced no output', async () => {