diff --git a/CLAUDE-TRANSCRIPT-CURSOR-INTEGRITY-P1-FIX-REPORT.md b/CLAUDE-TRANSCRIPT-CURSOR-INTEGRITY-P1-FIX-REPORT.md new file mode 100644 index 00000000000..e69386bdf70 --- /dev/null +++ b/CLAUDE-TRANSCRIPT-CURSOR-INTEGRITY-P1-FIX-REPORT.md @@ -0,0 +1,97 @@ +# Claude transcript cursor-integrity P1 fix + +## Outcome + +Child HEAD is `f2cca94c0272a5930858f8d42796d4b22c3f69fb`, with the immutable +parent requested by dispatch. This child closes the two remaining proven +transcript cursor-integrity holes without changing the JSONL/snapshot journal, +lease/fence/lifecycle, account-root, resume-identity, approval/question, +cancel/error, or legacy/native toggle-off contracts. + +1. Root re-proof now retries only for the typed `ClaudeTranscriptTailIncompleteError` + and `ClaudeTranscriptPreviousCursorMissingError` cases. A proof that established + a sibling branch, malformed durable content, invalid ancestry, a wrong session, + or a sidechain cursor is not retried from the root, so a divergent marker cannot + be accepted or persisted as the new cursor. +2. Previous-cursor proof now validates the cursor's complete `parentUuid` ancestry + before accepting either `same` or `descendant` relationships. A cursor whose + ancestry crosses a `parent_tool_use_id` sidechain is rejected even when the + latest marker equals that cursor or is a descendant of it. Graceful close and + first-hand crash persistence consequently retain the last observed main-line + cursor when durable proof rejects the sidechain. + +## Exact files changed + +- `src/main/claude/claude-transcript-branch-proof.ts` +- `src/main/claude/claude-structured-session-recovery.test.ts` +- `src/main/native-chat/session-file-resolver.test.ts` +- `CLAUDE-TRANSCRIPT-CURSOR-INTEGRITY-P1-FIX-REPORT.md` + +No source/dependency files outside transcript proof, close/crash adapter tests, +and this report were changed. `pnpm-lock.yaml` was restored byte-for-byte to the +parent and has an empty diff. + +## Red-first proof + +Before implementation, the newly added tests failed in the expected ways: + +- A root → `subagent(parent_tool_use_id)` → `main-after` graph resolved a + sidechain-descended previous cursor for both the `same` and descendant-marker + cases. +- A root → `old` and root → `new` sibling graph caused the re-proof helper to + call the root fallback and accept `new`, rather than preserving the sibling + rejection. + +The post-fix tests assert the initial sibling proof rejects, the fallback is not +called for that typed-divergence error, and close/crash persistence retains the +observed leaf instead of the divergent or sidechain leaf. Existing valid +main-line same/descendant, stale-tail, malformed-tail, and parent-tool-use +sampling tests remain green. + +## Verification + +- Focused resolver/recovery suites (`session-file-resolver.test.ts` and + `claude-structured-session-recovery.test.ts`): 2 files, 38 tests passed. +- Relevant Claude/router/session suites (direct Vitest invocation; 10 explicit + test files): 10 files, 103 tests passed. +- Claude child-process/TUI lifecycle checks (`claude-agent-sdk-exit-proof.test.ts`, + `claude-agent-sdk-process-spawn.test.ts`, `claude-tui-exit.test.ts`, and + `claude-tui-resume-proof.test.ts`): 4 files, 41 tests passed. +- Node typecheck (`pnpm run typecheck:node`) passed. +- Web typecheck (`pnpm run typecheck:web`) passed. +- Changed-code quality gate passed with zero new findings. +- Changed-file `oxfmt --check` and `git diff --check` passed. +- The broader Claude-directory run had two environment-dependent failures + (real-CLI credential/init timeout and a timing-sensitive SIGTERM-resistant + descendant test); the focused process/lifecycle rerun passed, and neither + failure touches this diff. + +## Five-finding disposition + +- Prior P1 #1, parent-tool-use/sidechain UUID sampling: fixed on the immutable + parent; this child preserves the filter and adds the missing previous-cursor + ancestry proof. +- Prior P1 #2, Claude Code SDK system-prompt preset: fixed on the immutable + parent; no regression found. +- Prior P1 #3, router close ownership/retry: fixed on the immutable parent; no + regression found. +- Current P1 #4, unrestricted root re-proof after any cursor error: fixed here + with typed stale-tail/previous-cursor-missing gating and sibling fail-closed + coverage. +- Current P1 #5, sidechain-descended previous cursor accepted for same/descendant + marker relationships: fixed here with complete previous ancestry validation + and close/crash persistence coverage. + +Grouped-answer, local-image, and cleanup-retry leads remain out of scope unless +future evidence proves a regression; no such regression was found in this child. + +## Lineage and artifact checks + +- `git rev-parse HEAD` before the child fix commit: + `f2cca94c0272a5930858f8d42796d4b22c3f69fb`. +- Child worktree is at the requested branch/parent lineage; the immutable + parent SHA was not edited. +- `git diff --name-status` contains only the three modified test/proof files + above (plus this report); there are no deletions. +- No pre-existing untracked artifacts were edited or removed. The only new + artifact is this report, retained for coordinator handoff. diff --git a/src/main/claude/claude-structured-session-recovery.test.ts b/src/main/claude/claude-structured-session-recovery.test.ts index 4bbdf32809d..abb531c294e 100644 --- a/src/main/claude/claude-structured-session-recovery.test.ts +++ b/src/main/claude/claude-structured-session-recovery.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import type { ClaudeStructuredSessionEvent } from './claude-structured-session-adapter' +import { ClaudeTranscriptPreviousCursorMissingError } from './claude-transcript-branch-proof' import { adapterFor, fakeClaude, @@ -72,12 +73,12 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => { expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'durable-tail' }) }) - it('re-proves from the transcript root when the observed cursor rejects a stale branch', async () => { + it('re-proves from the transcript root when the observed cursor is missing', async () => { const claude = fakeClaude() const persistedHandles: unknown[] = [] const readTranscriptLeaf = vi .fn() - .mockRejectedValueOnce(new Error('sibling branch')) + .mockRejectedValueOnce(new ClaudeTranscriptPreviousCursorMissingError()) .mockResolvedValueOnce('reproved-main-leaf') const adapter = adapterFor(claude, {}, [], persistedHandles, undefined, readTranscriptLeaf) await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) @@ -100,6 +101,26 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => { expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'reproved-main-leaf' }) }) + it('keeps the observed leaf when transcript validation proves a sibling branch', async () => { + const claude = fakeClaude() + const persistedHandles: unknown[] = [] + const readTranscriptLeaf = vi + .fn() + .mockRejectedValue(new Error('latest marker is on a sibling branch')) + const adapter = adapterFor(claude, {}, [], persistedHandles, undefined, readTranscriptLeaf) + await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) + claude.connections[0].handlers.onMessage?.({ + type: 'assistant', + session_id: PROVIDER_SESSION_ID, + uuid: 'observed-tail' + }) + + await adapter.closeSession('session-1') + + expect(readTranscriptLeaf).toHaveBeenCalledTimes(1) + expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'observed-tail' }) + }) + it('persists the last transcript leaf before an unexpected first-hand exit', async () => { const claude = fakeClaude() const persistedHandles: unknown[] = [] @@ -161,7 +182,7 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => { const persistedHandles: unknown[] = [] const readTranscriptLeaf = vi .fn() - .mockRejectedValueOnce(new Error('stale cursor')) + .mockRejectedValueOnce(new ClaudeTranscriptPreviousCursorMissingError()) .mockResolvedValueOnce('reproved-crash-leaf') const adapter = adapterFor(claude, {}, [], persistedHandles, undefined, readTranscriptLeaf) await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) @@ -184,6 +205,26 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => { expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'reproved-crash-leaf' }) }) + it('keeps the observed crash leaf when transcript validation proves a sibling branch', async () => { + const claude = fakeClaude() + const persistedHandles: unknown[] = [] + const readTranscriptLeaf = vi + .fn() + .mockRejectedValue(new Error('latest marker is on a sibling branch')) + const adapter = adapterFor(claude, {}, [], persistedHandles, undefined, readTranscriptLeaf) + await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) + claude.connections[0].handlers.onMessage?.({ + type: 'assistant', + session_id: PROVIDER_SESSION_ID, + uuid: 'observed-crash-tail' + }) + claude.connections[0].handlers.onExit?.(new Error('crashed')) + await tick() + + expect(readTranscriptLeaf).toHaveBeenCalledTimes(1) + expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'observed-crash-tail' }) + }) + it('publishes lifecycle recovery even when crash-cursor persistence fails', async () => { const claude = fakeClaude() const events: ClaudeStructuredSessionEvent[] = [] diff --git a/src/main/claude/claude-transcript-branch-proof.ts b/src/main/claude/claude-transcript-branch-proof.ts index 4baf545eb95..07f3c327a9d 100644 --- a/src/main/claude/claude-transcript-branch-proof.ts +++ b/src/main/claude/claude-transcript-branch-proof.ts @@ -29,6 +29,42 @@ export class ClaudeTranscriptTailIncompleteError extends Error { } } +/** The sampled cursor is no longer present, so a root proof may still recover safely. */ +export class ClaudeTranscriptPreviousCursorMissingError extends Error { + constructor() { + super( + 'Claude transcript branch proof failed: previous cursor is missing from the session graph' + ) + this.name = 'ClaudeTranscriptPreviousCursorMissingError' + } +} + +function proveMainLineAncestry( + nodes: Map, + startUuid: string, + providerSessionId: string +): void { + const visited = new Set() + let cursor: string | null = startUuid + for (let depth = 0; cursor !== null && depth < MAX_CLAUDE_TRANSCRIPT_ANCESTRY; depth += 1) { + if (visited.has(cursor)) { + throw transcriptError('cycle in parentUuid ancestry') + } + visited.add(cursor) + const node = nodes.get(cursor) + if (!node || node.sessionId !== providerSessionId) { + throw transcriptError(`missing ancestor ${cursor}`) + } + if (node.disallowedLeaf) { + throw transcriptError(`ancestor ${cursor} is not on the main transcript`) + } + cursor = node.parentUuid + } + if (cursor !== null) { + throw transcriptError('ancestry exceeds the bounded proof limit') + } +} + export function proveClaudeTranscriptBranchFromJsonl(input: { contents: string providerSessionId: string @@ -97,31 +133,21 @@ export function proveClaudeTranscriptBranchFromJsonl(input: { } const previousLeafUuid = input.previousLeafUuid if (!previousLeafUuid) { - const visited = new Set() - let cursor: string | null = leafUuid - for (let depth = 0; cursor !== null && depth < MAX_CLAUDE_TRANSCRIPT_ANCESTRY; depth += 1) { - if (visited.has(cursor)) { - throw transcriptError('cycle in parentUuid ancestry') - } - visited.add(cursor) - const node = nodes.get(cursor) - if (!node || node.sessionId !== input.providerSessionId) { - throw transcriptError(`missing ancestor ${cursor}`) - } - if (node.disallowedLeaf) { - throw transcriptError(`ancestor ${cursor} is not on the main transcript`) - } - cursor = node.parentUuid - } - if (cursor !== null) { - throw transcriptError('ancestry exceeds the bounded proof limit') - } + proveMainLineAncestry(nodes, leafUuid, input.providerSessionId) return { leafUuid, relation: 'initial' } } const previous = nodes.get(previousLeafUuid) - if (!previous || previous.sessionId !== input.providerSessionId || previous.disallowedLeaf) { - throw transcriptError('previous cursor is missing from the session graph') + if (!previous) { + throw new ClaudeTranscriptPreviousCursorMissingError() } + if (previous.sessionId !== input.providerSessionId || previous.disallowedLeaf) { + throw transcriptError('previous cursor is not on the main transcript') + } + // The latest marker can be equal to, or descend from, a sampled cursor. In + // either case prove the sampled cursor's own ancestry before accepting it; + // otherwise a cursor that descended through a parent-tool-use sidechain + // could be persisted and resumed as if it were on the main transcript. + proveMainLineAncestry(nodes, previousLeafUuid, input.providerSessionId) if (leafUuid === previousLeafUuid) { return { leafUuid, relation: 'same' } } @@ -177,7 +203,11 @@ export async function readClaudeTranscriptLeafWithReproof(input: { previousLeafUuid: input.previousLeafUuid }) } catch (error) { - if (input.previousLeafUuid === null) { + if ( + input.previousLeafUuid === null || + (!(error instanceof ClaudeTranscriptTailIncompleteError) && + !(error instanceof ClaudeTranscriptPreviousCursorMissingError)) + ) { throw error } return input.readTranscriptLeaf({ diff --git a/src/main/native-chat/session-file-resolver.test.ts b/src/main/native-chat/session-file-resolver.test.ts index 9e2960d70eb..aa2a535566a 100644 --- a/src/main/native-chat/session-file-resolver.test.ts +++ b/src/main/native-chat/session-file-resolver.test.ts @@ -3,7 +3,10 @@ import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' -import { ClaudeTranscriptTailIncompleteError } from '../claude/claude-transcript-branch-proof' +import { + ClaudeTranscriptTailIncompleteError, + readClaudeTranscriptLeafWithReproof +} from '../claude/claude-transcript-branch-proof' import { readClaudeTranscriptLeafUuid, resolveSessionFilePath } from './session-file-resolver' let tempRoots: string[] = [] @@ -238,6 +241,115 @@ describe('resolveSessionFilePath', () => { ) }) + it('rejects a previous cursor descended from a parent-tool-use sidechain', async () => { + const root = await makeRoot('orca-native-chat-resolve-claude-parent-tool-cursor-') + const transcript = join(root, 'transcript.jsonl') + await writeFile( + transcript, + [ + { type: 'user', uuid: 'main-user', parentUuid: null, sessionId: 'session-1' }, + { + type: 'assistant', + uuid: 'subagent-assistant', + parentUuid: 'main-user', + sessionId: 'session-1', + parent_tool_use_id: 'tool-use-1' + }, + { + type: 'assistant', + uuid: 'main-after-sidechain', + parentUuid: 'subagent-assistant', + sessionId: 'session-1' + }, + { type: 'last-prompt', leafUuid: 'main-after-sidechain', sessionId: 'session-1' } + ] + .map((record) => JSON.stringify(record)) + .join('\n'), + 'utf8' + ) + + await expect( + readClaudeTranscriptLeafUuid(transcript, 'session-1', 'main-after-sidechain') + ).rejects.toThrow('not on the main transcript') + }) + + it('rejects a latest marker descended from a parent-tool-use cursor sidechain', async () => { + const root = await makeRoot('orca-native-chat-resolve-claude-parent-tool-cursor-descendant-') + const transcript = join(root, 'transcript.jsonl') + await writeFile( + transcript, + [ + { type: 'user', uuid: 'main-user', parentUuid: null, sessionId: 'session-1' }, + { + type: 'assistant', + uuid: 'subagent-assistant', + parentUuid: 'main-user', + sessionId: 'session-1', + parent_tool_use_id: 'tool-use-1' + }, + { + type: 'assistant', + uuid: 'main-after-sidechain', + parentUuid: 'subagent-assistant', + sessionId: 'session-1' + }, + { + type: 'assistant', + uuid: 'latest-after-sidechain', + parentUuid: 'main-after-sidechain', + sessionId: 'session-1' + }, + { type: 'last-prompt', leafUuid: 'latest-after-sidechain', sessionId: 'session-1' } + ] + .map((record) => JSON.stringify(record)) + .join('\n'), + 'utf8' + ) + + await expect( + readClaudeTranscriptLeafUuid(transcript, 'session-1', 'main-after-sidechain') + ).rejects.toThrow('not on the main transcript') + }) + + it('does not re-prove a divergent sibling after the sampled cursor rejects', async () => { + const root = await makeRoot('orca-native-chat-resolve-claude-sibling-reproof-') + const transcript = join(root, 'transcript.jsonl') + await writeFile( + transcript, + [ + { type: 'user', uuid: 'root', parentUuid: null, sessionId: 'session-1' }, + { type: 'assistant', uuid: 'old', parentUuid: 'root', sessionId: 'session-1' }, + { type: 'assistant', uuid: 'new', parentUuid: 'root', sessionId: 'session-1' }, + { type: 'last-prompt', leafUuid: 'new', sessionId: 'session-1' } + ] + .map((record) => JSON.stringify(record)) + .join('\n'), + 'utf8' + ) + const calls: (string | null)[] = [] + const readTranscriptLeaf = async ({ + previousLeafUuid + }: { + previousLeafUuid: string | null + }) => { + calls.push(previousLeafUuid) + return readClaudeTranscriptLeafUuid(transcript, 'session-1', previousLeafUuid) + } + + await expect(readClaudeTranscriptLeafUuid(transcript, 'session-1', 'old')).rejects.toThrow( + 'sibling branch' + ) + + await expect( + readClaudeTranscriptLeafWithReproof({ + readTranscriptLeaf, + providerSessionId: 'session-1', + previousLeafUuid: 'old' + }) + ).rejects.toThrow('sibling branch') + expect(calls).toEqual(['old']) + }) + it('globs Claude project subdirs for .jsonl', async () => { const root = await makeRoot('orca-native-chat-resolve-claude-') const claudeProjectsDir = join(root, 'claude-projects')