diff --git a/docs/plans/2026-09-25-claude-rollback-attribute-composer.md b/docs/plans/2026-09-25-claude-rollback-attribute-composer.md new file mode 100644 index 000000000..816c4469c --- /dev/null +++ b/docs/plans/2026-09-25-claude-rollback-attribute-composer.md @@ -0,0 +1,16 @@ +# Claude delivery rollback reads the attribute-aware composer state (#1291) + +## Evidence +- Paste-debug journals: 4 `rollback-exhausted {presses: 64, restored: true}` against 1 `rollback-cleared`. Each exhausted case takes 6.7–8.6 s: 5 s absorption timeout, then about 0.4 s, then 64 × 25 ms of kills. The delivery then reports "still in Claude's composer — clear it there", and the gate reads `occupied`. +- Prompts: "yes fix all 9" (typical of a Claude prompt suggestion), "draft the WhatsApp message for Falco", and two long human messages (144 and 380 chars). +- `rollbackWrittenPrompt` classifies with `parseClaudeComposerState(screen, null)`, i.e. text-only. That path allowlists only two placeholder hints, and every other placeholder row reads as `drafted`. After a kill empties the composer, Claude repaints placeholder text (a suggestion, a hint), so the text-only read never becomes `empty`. The rollback presses 64 times and then yanks the prompt back: stranded. +- The prompt gate already avoids this. `claudeSession.derivePromptGateState` reads `headless.getComposerState()`, which classifies with cell attributes, so dim placeholder text is `empty`. The rollback bypasses it. +- No PTY frame of these moments survives. The proxy run was pruned, and feed-debug has no screen text. The mechanism is shown by the parser's two paths plus the outcome journals. + +## Change +- `ClaudeSession.getComposerState()` exposes the headless's attribute-aware state, and `AgentSession` gets it as an optional capability. +- The rollback prefers it and keeps the text-only fail-closed read for sessions without it. +- The observe/kill/yank structure is unchanged. + +## Test +A delivery whose composer shows prompt-suggestion text before and after the kill. The attribute-aware state goes `drafted` to `empty` after the first kill. The rollback must report `cleared` after 1 press, not exhaust 64. Red on main. diff --git a/packages/claude-code-headless b/packages/claude-code-headless index 93f5a5450..64fd0eaf1 160000 --- a/packages/claude-code-headless +++ b/packages/claude-code-headless @@ -1 +1 @@ -Subproject commit 93f5a545061cdca67f66cfab68d276d72bcaba45 +Subproject commit 64fd0eaf18a0a6cef58b9048e3768843b3e0e65b diff --git a/src/providers/claude/runtime/claudeSession.suspension.test.ts b/src/providers/claude/runtime/claudeSession.suspension.test.ts index a1e10906b..70b692614 100644 --- a/src/providers/claude/runtime/claudeSession.suspension.test.ts +++ b/src/providers/claude/runtime/claudeSession.suspension.test.ts @@ -35,3 +35,19 @@ describe('ClaudeSession.noteSystemSuspension', () => { })).not.toThrow() }) }) + +// #1309 round 2 B: the production adapter the rollback reads. Both halves +// must come from the live headless at the same call, and a session without a +// headless reports nothing rather than an empty (= "cleared") screen. +describe('ClaudeSession.readComposer', () => { + it('reads screen and attributes from the live headless together', () => { + const session = new ClaudeSession() + expect(session.readComposer()).toBeNull() + const attributes = { dim: 0, inverse: 1, plain: 4 } + ;(session as unknown as { headless: unknown }).headless = { + getScreen: () => '❯ typed', + getComposerAttributes: () => attributes, + } + expect(session.readComposer()).toEqual({ screen: '❯ typed', attributes }) + }) +}) diff --git a/src/providers/claude/runtime/claudeSession.ts b/src/providers/claude/runtime/claudeSession.ts index ec30cd5ea..361e97aa8 100644 --- a/src/providers/claude/runtime/claudeSession.ts +++ b/src/providers/claude/runtime/claudeSession.ts @@ -733,6 +733,12 @@ export class ClaudeSession extends EventEmitter { return this.headless?.getScreen() ?? '' } + readComposer(): { screen: string; attributes: { dim: number; inverse: number; plain: number } | null } | null { + if (!this.headless) return null + // Same instant: text and attributes both from the live buffer. + return { screen: this.headless.getScreen(), attributes: this.headless.getComposerAttributes() } + } + snapshotScreenAsMarkdown(): string { return this.headless?.getScreenMarkdown() ?? '' } diff --git a/src/providers/claude/runtime/promptDelivery.rollbackPlaceholder.test.ts b/src/providers/claude/runtime/promptDelivery.rollbackPlaceholder.test.ts new file mode 100644 index 000000000..40a060062 --- /dev/null +++ b/src/providers/claude/runtime/promptDelivery.rollbackPlaceholder.test.ts @@ -0,0 +1,117 @@ +import { afterEach, expect, it, vi } from 'vitest' + +import { deliverClaudePrompt } from './promptDelivery.js' +import type { PromptDeliveryIo } from '@shared/types/providerConfig.js' + +// #1291: the paste-debug journals hold 4 `rollback-exhausted {presses: 64}` +// against 1 `rollback-cleared`. After absorption timed out, the rollback read +// the composer TEXT-ONLY, and Claude repaints placeholder text (a prompt +// suggestion such as the recorded "yes fix all 9", or a hint) into an emptied +// composer; text-only reads that as a draft, so every kill looked like it +// failed and 64 presses later the prompt was yanked back and stranded. +// #1309 review: the fix must read the LIVE buffer (not the lagging per-frame +// cache) and must not call a leftover inverse-cursor character "empty". +afterEach(() => { vi.useRealTimers() }) + +const RULE = '─'.repeat(40) +const composer = (row: string) => [RULE, row, RULE].join('\n') +type Attributes = { dim: number; inverse: number; plain: number } +type Frame = { screen: string; attributes: Attributes | null } + +// Our bytes, as typed (plain cells). +const TYPED: Frame = { screen: composer('❯ yes fix all 9'), attributes: { dim: 0, inverse: 1, plain: 12 } } + +async function rollback(afterKill: Frame, before: Frame = TYPED) { + vi.useFakeTimers() + const writes: string[] = [] + const records: string[] = [] + let killed = false + const io = { + sessionId: 's1', + prompt: 'yes fix all 9', + write: (data: string) => { + writes.push(data) + if (data === '\x15') killed = true + return true + }, + record: (event: string) => { records.push(event) }, + session: { + // The same frame is visible before our write too, so absorption never + // sees a transition and the delivery reaches the rollback (#1230). + snapshotScreen: () => (killed ? afterKill : before).screen, + readComposer: () => (killed ? afterKill : before), + armPromptAcceptance: () => ({ promise: new Promise(() => {}), cancel: vi.fn() }), + }, + } as unknown as PromptDeliveryIo + const delivery = deliverClaudePrompt(io) + await vi.advanceTimersByTimeAsync(30_000) + const result = await delivery + return { result, records, kills: writes.filter(data => data === '\x15').length } +} + +it('clears a stranded prompt when the emptied composer repaints a dim suggestion', async () => { + // After the kill only the suggestion remains: dim text, the cursor on its + // first cell, nothing typed. + const { result, records, kills } = await rollback({ screen: composer('❯ yes fix all 9'), attributes: { dim: 12, inverse: 1, plain: 0 } }) + expect(records.join(',')).toContain('rollback-cleared') + expect(kills).toBe(1) + expect(result).toMatchObject({ ok: false, promptWritten: false, retrySafe: true }) +}) + +it('does not call a leftover character under the cursor empty', async () => { + // A partial kill: one real character remains, drawn inverse (the cursor on + // it), no dim placeholder. Attributes alone would say "nothing typed". + const { records } = await rollback({ screen: composer('❯ y'), attributes: { dim: 0, inverse: 1, plain: 0 } }) + expect(records.join(',')).not.toContain('rollback-cleared') +}) + +it('trusts a bare prompt marker even when attributes are unavailable', async () => { + // The emptied composer shows only the marker; this is the text read's own + // reliable "empty", independent of any attribute or cache timing. + const { records, kills } = await rollback({ screen: composer('❯'), attributes: null }) + expect(records.join(',')).toContain('rollback-cleared') + expect(kills).toBe(1) +}) + +// #1309 review B: the classifier fed by a REAL ClaudeCodeHeadless, so the +// live screen + attribute pipeline (not a hand-written object) reaches it. +it('classifies real painted frames: a dim suggestion is empty, typed text is not', async () => { + vi.useRealTimers() + const { ClaudeCodeHeadless } = await import('claude-code-headless') + const { classifyRollbackComposer } = await import('./promptDelivery.js') + const pty = { + pid: 1, process: 'claude', cols: 120, rows: 40, handleFlowControl: false, + write: vi.fn(), resize: vi.fn(), clear: vi.fn(), pause: vi.fn(), resume: vi.fn(), kill: vi.fn(), + onData: vi.fn(() => ({ dispose: vi.fn() })), onExit: vi.fn(() => ({ dispose: vi.fn() })), + } + const headless = new ClaudeCodeHeadless({ pty: pty as never, cwd: '/tmp' }) + const terminal = (headless as unknown as { terminal: { writeForTest(data: string): Promise } }).terminal + const paint = async (row: string) => { + await terminal.writeForTest('\x1b[2J\x1b[H' + ['─'.repeat(60), row, '─'.repeat(60)].join('\r\n')) + return classifyRollbackComposer({ screen: headless.getScreen(), attributes: headless.getComposerAttributes() }, '') + } + expect(await paint('❯ \x1b[2myes fix all 9\x1b[22m')).toBe('empty') + expect(await paint('❯ yes fix all 9')).toBe('drafted') + expect(await paint('❯')).toBe('empty') + // #1309 round 2 (A and B): a known hint is empty only when it is PAINTED + // as one. The same words typed (a first visual row left behind by a kill + // of the row below) are a draft. + expect(await paint('❯ \x1b[2mPress up to edit\x1b[22m')).toBe('empty') + expect(await paint('❯ Press up to edit')).toBe('drafted') +}) + +// #1309 round 2: the rollback itself must not report that typed row cleared; +// a retry would append to it and submit the fragment. +it('does not call typed text that matches a known hint empty', async () => { + const { records, result } = await rollback({ screen: composer('❯ Press up to edit'), attributes: { dim: 0, inverse: 1, plain: 15 } }) + expect(records.join(',')).not.toContain('rollback-cleared') + expect(result).not.toMatchObject({ retrySafe: true }) +}) + +// #1309 round 2 B mutation: the attributes sample only the MARKER row, so a +// dim marker row says nothing about typed text on a continuation row. +it('does not call a dim marker row empty while a continuation row holds text', async () => { + const { classifyRollbackComposer } = await import('./promptDelivery.js') + const screen = [RULE, '❯ yes fix all 9', ' and the rest of a draft', RULE].join('\n') + expect(classifyRollbackComposer({ screen, attributes: { dim: 12, inverse: 1, plain: 0 } }, '')).toBe('drafted') +}) diff --git a/src/providers/claude/runtime/promptDelivery.ts b/src/providers/claude/runtime/promptDelivery.ts index c6da53fd8..4b679b5ee 100644 --- a/src/providers/claude/runtime/promptDelivery.ts +++ b/src/providers/claude/runtime/promptDelivery.ts @@ -453,12 +453,7 @@ async function rollbackWrittenPrompt( io: PromptDeliveryIo, ): Promise<'cleared' | 'restored' | 'unrecoverable'> { const readComposer = (): 'empty' | 'drafted' | 'unpainted' => - // Classified WITHOUT cell attributes, which is the fail-closed path: with - // no attributes an unrecognised row is reported as 'drafted'. That error - // direction is the safe one here — a false 'drafted' aborts and restores, - // whereas a false 'empty' would let us report success over a composer still - // holding half a prompt. - parseClaudeComposerState(io.session.snapshotScreen?.() ?? '', null) + classifyRollbackComposer(io.session.readComposer?.() ?? null, io.session.snapshotScreen?.() ?? '') // STEP 1 — wait until our bytes are actually VISIBLE before touching anything. // @@ -534,3 +529,48 @@ function describeReadiness( if (outcome.kind === 'occupied') return 'occupied by a human draft' return `unavailable (${outcome.reason})` } + +/** + * The rollback's reading of Claude's composer (#1291, #1309 review). + * + * After a kill empties the composer, Claude repaints placeholder text into it + * (a prompt suggestion, a hint), and the text-only read calls any unrecognised + * row 'drafted'. The paste-debug corpus holds 4 `rollback-exhausted` (all 64 + * presses, then yanked back), 1 `rollback-cleared` and 7 `rollback-unobserved`. + * The exhausted records carry no screen, so a repainted placeholder is the + * explanation that fits them (one prompt was a typical suggestion, "yes fix + * all 9"), not a recorded observation. + * + * Exported so a test can drive it with a real ClaudeCodeHeadless frame. + */ +export function classifyRollbackComposer( + live: { screen: string; attributes: { dim: number; inverse: number; plain: number } | null } | null, + fallbackScreen: string, +): 'empty' | 'drafted' | 'unpainted' { + // The rule (#1309 review): + // - the text-only read stays the base. It fails closed, and its 'empty' + // is trustworthy for a bare prompt marker; + // - but NOT for its allowlisted hints ("Press up to edit") when the live + // cells show typed text (#1309 round 2, reviews A and B). A kill clears + // only the current VISUAL row, so a prompt whose first row reads exactly + // like a hint can leave that row behind, plain. Calling it cleared told + // the caller a retry was safe, and the retry would append to it; + // - it is overruled to 'empty' only when the LIVE cell attributes show a + // painted placeholder: dim cells and no typed (plain) cells. A leftover + // character under the inverse cursor has no dim cells, so it stays + // 'drafted'; + // - text and attributes come from the live buffer at the same instant, + // never the per-frame cache, which lags a 25 ms kill loop and can + // stall behind pendingWrites. + // A false 'drafted' aborts and restores; a false 'empty' would report + // success over half a prompt, so every doubt resolves to 'drafted'. + const screen = live?.screen ?? fallbackScreen + const textOnly = parseClaudeComposerState(screen, null) + const attributes = live?.attributes + if (textOnly === 'empty' && attributes && attributes.plain > 0) return 'drafted' + if (textOnly !== 'drafted') return textOnly + if (attributes && attributes.plain === 0 && attributes.dim > 0 + && parseClaudeComposerState(screen, attributes) === 'empty') return 'empty' + return 'drafted' +} + diff --git a/src/shared/types/session.ts b/src/shared/types/session.ts index 5b5e3b904..e46b17260 100644 --- a/src/shared/types/session.ts +++ b/src/shared/types/session.ts @@ -605,6 +605,13 @@ export interface AgentSession extends AgentSessionEmitter { * headless instance isn't up yet. */ snapshotScreen?(): string + /** Optional (Claude today): the screen and the composer's cell-attribute + * counts read from the LIVE buffer at the same instant (#1291/#1309). Not + * the per-frame cached classification the prompt gate uses: the delivery + * rollback verifies its own keystroke 25 ms later, faster than that cache + * moves. Null when the headless instance isn't up. */ + readComposer?(): { screen: string; attributes: { dim: number; inverse: number; plain: number } | null } | null + /** * Optional (Claude today): arm an authoritative prompt-acceptance waiter * BEFORE Enter is written. Claude acknowledges a finished prompt through