diff --git a/docs/audits/claude-history-window-budget/README.md b/docs/audits/claude-history-window-budget/README.md new file mode 100644 index 00000000000..2d5a9bddb47 --- /dev/null +++ b/docs/audits/claude-history-window-budget/README.md @@ -0,0 +1,25 @@ +# Claude history-window read quota + +Restart reconciliation's Claude history reader declares a 16 MiB source quota, but the original `stat` followed by unrestricted `readFile` admits later growth. Reuse `readNodeFileWithinLimit` to enforce the same quota while reading from one descriptor. Overflow preserves the existing inconsistent-history result; it cannot establish that a submitted message was absent. + +## Reproduce + +```sh +ORCA_BACKGROUND_LAUNCH=1 node docs/audits/claude-history-window-budget/reproduce.mjs +``` + +The seven-case fixture calls the actual history reader, branch proof and bounded reader against real temporary files. It injects an append immediately after the relevant size snapshot. The baseline reverses only `fix.patch` in a temporary Vite transform, leaving the checkout unchanged. Each child has a 45-second timeout and a 512 MiB old-space ceiling. No application window or provider process starts. + +| Case | Before | After | +| --------------------------------------------------------- | ---------------------------------------------- | ----------------------------------------------------------------------- | +| 305-byte valid source grows to 17 MiB after size snapshot | Reads 17,825,792 bytes and accepts the history | Stops after 16,777,217 bytes, rejects history and closes its descriptor | +| Stable valid source | Accepted | Accepted | +| Exactly 16 MiB | Accepted | Accepted | +| Initially 16 MiB plus one byte | Rejected before content read | Same | +| Growth within quota | Reads the additional bytes and accepts | Same | +| Read error | Inconsistent history | Same; descriptor closed | +| Missing anchor | No source read | Same | + +Before: one failing fixed-behavior assertion and six passing controls. After: seven passing cases. `results.json` records observed read bytes, size snapshots, descriptor counts, source hashes, exit codes and timeout status. These are read-budget measurements, not RSS or retained-heap measurements; the byte quota does not describe all parser allocations. + +This reader is **absent from v1.4.198**, so the finding cannot explain #19768 or #19831. It fixes a current-source quota race. The separately unrestricted `proveClaudeTranscriptBranch` file reader has no declared byte quota and is outside this change. The related legacy-import quota race is already addressed by #20976. diff --git a/docs/audits/claude-history-window-budget/fix.patch b/docs/audits/claude-history-window-budget/fix.patch new file mode 100644 index 00000000000..5952bc249d2 --- /dev/null +++ b/docs/audits/claude-history-window-budget/fix.patch @@ -0,0 +1,30 @@ +diff --git a/src/main/claude/claude-structured-history-window.ts b/src/main/claude/claude-structured-history-window.ts +index 1cfd48a8a6..f7a541fa31 100644 +--- a/src/main/claude/claude-structured-history-window.ts ++++ b/src/main/claude/claude-structured-history-window.ts +@@ -12,8 +12,8 @@ + // of those makes absence meaningless. Failing it reports an inconsistent + // boundary rather than an empty window, because the two decide opposite things. + +-import { readFile, stat } from 'node:fs/promises' + import { join } from 'node:path' ++import { readNodeFileWithinLimit } from '../../shared/node-bounded-file-reader' + import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' + import { resolveSessionFilePath } from '../native-chat/session-file-resolver' + import type { +@@ -284,10 +284,11 @@ export async function readClaudeProviderHistoryWindow(input: { + } + let contents: string + try { +- if ((await stat(input.transcriptPath)).size > MAX_HISTORY_WINDOW_SOURCE_BYTES) { +- return INCONSISTENT +- } +- contents = await readFile(input.transcriptPath, 'utf8') ++ const read = await readNodeFileWithinLimit( ++ input.transcriptPath, ++ MAX_HISTORY_WINDOW_SOURCE_BYTES ++ ) ++ contents = read.buffer.toString('utf8') + } catch { + return INCONSISTENT + } diff --git a/docs/audits/claude-history-window-budget/reproduce.mjs b/docs/audits/claude-history-window-budget/reproduce.mjs new file mode 100644 index 00000000000..aefb721afeb --- /dev/null +++ b/docs/audits/claude-history-window-budget/reproduce.mjs @@ -0,0 +1,161 @@ +import { createHash } from 'node:crypto' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' +import { applyPatch, parsePatch, reversePatch } from 'diff' +import { build } from 'esbuild' + +if (process.env.ORCA_BACKGROUND_LAUNCH !== '1') { + throw new Error('Run with ORCA_BACKGROUND_LAUNCH=1.') +} + +const root = fileURLToPath(new URL('../../../', import.meta.url)) +const patch = await readFile(new URL('./fix.patch', import.meta.url), 'utf8') +const beforeSources = {} +const sourceHashes = {} +for (const parsed of parsePatch(patch)) { + const path = parsed.newFileName.replace(/^b\//, '') + const absolute = resolve(root, path) + const current = await readFile(absolute, 'utf8') + const before = applyPatch(current, reversePatch(parsed)) + if (before === false) { + throw new Error(`Source changed; review the proof patch: ${path}`) + } + beforeSources[absolute.replaceAll('\\', '/')] = before + sourceHashes[path] = { + before: createHash('sha256').update(before).digest('hex'), + after: createHash('sha256').update(current).digest('hex') + } +} + +for (const path of [ + 'src/main/claude/claude-history-window-source-budget.test.ts', + 'src/shared/node-bounded-file-reader.ts', + 'src/main/claude/claude-transcript-branch-proof.ts' +]) { + sourceHashes[path] = { + current: createHash('sha256') + .update(await readFile(resolve(root, path))) + .digest('hex') + } +} + +const scratch = await mkdtemp(join(tmpdir(), 'orca-claude-history-window-budget-')) +const require = createRequire(import.meta.url) +let runnerModuleId +try { + const runnerPath = join(scratch, 'run-process.cjs') + await build({ + absWorkingDir: root, + entryPoints: [resolve(root, 'src/shared/child-process/run-process.ts')], + outfile: runnerPath, + bundle: true, + platform: 'node', + format: 'cjs', + logLevel: 'silent' + }) + runnerModuleId = require.resolve(runnerPath) + const { runProcess } = require(runnerModuleId) + const baselineConfig = join(scratch, 'before.config.mjs') + const fixedConfig = join(scratch, 'after.config.mjs') + const includes = ['src/main/claude/claude-history-window-source-budget.test.ts'] + const configImport = JSON.stringify(pathToFileURL(resolve(root, 'config/vitest.config.ts')).href) + await writeFile( + baselineConfig, + `import base from ${configImport}; +const beforeSources = ${JSON.stringify(beforeSources)}; +export default {...base, test: {...base.test, include: ${JSON.stringify(includes)}}, plugins: [{ + name: 'claude-history-window-budget-before-fix', enforce: 'pre', + transform(_code, id) { + const before = beforeSources[id.replaceAll('\\\\', '/').split('?')[0]]; + return before === undefined ? null : {code: before, map: null}; + } +}]};\n` + ) + + await writeFile( + fixedConfig, + `import base from ${configImport};\nexport default {...base, test: {...base.test, include: ${JSON.stringify(includes)}}};\n` + ) + + async function run(label, config) { + const report = join(scratch, `${label}.json`) + const observationPath = join(scratch, `${label}.observations.jsonl`) + const result = await runProcess({ + program: process.execPath, + args: [ + resolve(root, 'node_modules/vitest/vitest.mjs'), + 'run', + '--config', + config, + '--reporter=json', + `--outputFile=${report}` + ], + cwd: root, + env: { + ...process.env, + ORCA_HISTORY_BUDGET_PROOF_OUTPUT: observationPath, + NODE_OPTIONS: '--max-old-space-size=512' + }, + timeoutMs: 45_000, + maxOutputBytes: 4 * 1024 * 1024 + }) + let parsed + try { + parsed = JSON.parse(await readFile(report, 'utf8')) + } catch (error) { + throw new Error(`${label} runner failed: ${result.stderr || result.stdout}`, { cause: error }) + } + return { + observations: (await readFile(observationPath, 'utf8')) + .trim() + .split('\n') + .map((line) => JSON.parse(line)), + exitCode: result.code, + timedOut: result.timedOut, + passed: parsed.numPassedTests, + failed: parsed.numFailedTests, + failedCases: parsed.testResults.flatMap((suite) => + suite.assertionResults + .filter((test) => test.status === 'failed') + .map((test) => test.fullName) + ) + } + } + + const before = await run('before', baselineConfig) + const after = await run('after', fixedConfig) + const passed = + before.exitCode === 1 && + after.exitCode === 0 && + !before.timedOut && + !after.timedOut && + before.failed === 1 && + before.passed === 6 && + after.passed === 7 && + after.failed === 0 + console.log( + JSON.stringify( + { + comparison: + 'Actual history reader and bounded file reader with real files; growth injected after stat; before reverses only fix.patch in a temporary Vite transform', + sourceHashes, + before, + after, + passed + }, + null, + 2 + ) + ) + if (!passed) { + process.exitCode = 1 + } +} finally { + if (runnerModuleId) { + delete require.cache[runnerModuleId] + } + await rm(scratch, { recursive: true, force: true }) +} diff --git a/docs/audits/claude-history-window-budget/results.json b/docs/audits/claude-history-window-budget/results.json new file mode 100644 index 00000000000..716ddba13f8 --- /dev/null +++ b/docs/audits/claude-history-window-budget/results.json @@ -0,0 +1,137 @@ +{ + "comparison": "Actual history reader and bounded file reader with real files; growth injected after stat; before reverses only fix.patch in a temporary Vite transform", + "sourceHashes": { + "src/main/claude/claude-structured-history-window.ts": { + "before": "43a9868f6bb87bfda620ace31f680f36b3ecd88851ad98c24be5320a9814358b", + "after": "77098508b3f5ab5cf4a53163a14da2fa74b1de38859c4717496f7a97e2cd50ea" + }, + "src/main/claude/claude-history-window-source-budget.test.ts": { + "current": "88f7a9a9ad9669c636bd7369fdd9a1263325848971f67b499278bf78eb17681e" + }, + "src/shared/node-bounded-file-reader.ts": { + "current": "2cffa451b72d059226e0c2d4b6d2a5203cb8ddafb358d5e828dcd3436d197ecb" + }, + "src/main/claude/claude-transcript-branch-proof.ts": { + "current": "3224594befe1edae514724caf5eff6a4cda85212e2985be4920b6164b088b82c" + } + }, + "before": { + "observations": [ + { + "test": "reads a stable history without changing prompt evidence", + "bytesRead": 305, + "observedStatBytes": 305, + "opens": 0, + "closes": 0 + }, + { + "test": "accepts a complete source at the existing byte limit", + "bytesRead": 16777216, + "observedStatBytes": 16777216, + "opens": 0, + "closes": 0 + }, + { + "test": "rejects an initially oversized source before reading its contents", + "bytesRead": 0, + "observedStatBytes": 16777217, + "opens": 0, + "closes": 0 + }, + { + "test": "refuses concurrent growth beyond the existing source quota", + "bytesRead": 17825792, + "observedStatBytes": 305, + "opens": 0, + "closes": 0 + }, + { + "test": "accepts concurrent growth that stays within the quota", + "bytesRead": 369, + "observedStatBytes": 305, + "opens": 0, + "closes": 0 + }, + { + "test": "preserves the inconsistent result on a read error", + "bytesRead": 0, + "observedStatBytes": 305, + "opens": 0, + "closes": 0 + }, + { + "test": "does not open a source without an anchor", + "bytesRead": 0, + "observedStatBytes": 0, + "opens": 0, + "closes": 0 + } + ], + "exitCode": 1, + "timedOut": false, + "passed": 6, + "failed": 1, + "failedCases": [ + "Claude provider history source budget refuses concurrent growth beyond the existing source quota" + ] + }, + "after": { + "observations": [ + { + "test": "reads a stable history without changing prompt evidence", + "bytesRead": 305, + "observedStatBytes": 305, + "opens": 1, + "closes": 1 + }, + { + "test": "accepts a complete source at the existing byte limit", + "bytesRead": 16777216, + "observedStatBytes": 16777216, + "opens": 1, + "closes": 1 + }, + { + "test": "rejects an initially oversized source before reading its contents", + "bytesRead": 0, + "observedStatBytes": 16777217, + "opens": 1, + "closes": 1 + }, + { + "test": "refuses concurrent growth beyond the existing source quota", + "bytesRead": 16777217, + "observedStatBytes": 305, + "opens": 1, + "closes": 1 + }, + { + "test": "accepts concurrent growth that stays within the quota", + "bytesRead": 369, + "observedStatBytes": 305, + "opens": 1, + "closes": 1 + }, + { + "test": "preserves the inconsistent result on a read error", + "bytesRead": 0, + "observedStatBytes": 305, + "opens": 1, + "closes": 1 + }, + { + "test": "does not open a source without an anchor", + "bytesRead": 0, + "observedStatBytes": 0, + "opens": 0, + "closes": 0 + } + ], + "exitCode": 0, + "timedOut": false, + "passed": 7, + "failed": 0, + "failedCases": [] + }, + "passed": true +} diff --git a/src/main/claude/claude-history-window-source-budget.test.ts b/src/main/claude/claude-history-window-source-budget.test.ts new file mode 100644 index 00000000000..5e6e5332744 --- /dev/null +++ b/src/main/claude/claude-history-window-source-budget.test.ts @@ -0,0 +1,188 @@ +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import type * as FsPromises from 'node:fs/promises' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const state = vi.hoisted(() => ({ + path: '', + growth: '', + readError: false, + bytesRead: 0, + closes: 0, + opens: 0, + observedStatBytes: 0 +})) + +vi.mock('node:fs/promises', async (importOriginal) => { + const fs = await importOriginal() + const afterStat = async (path: string, size: number) => { + if (path !== state.path) { + return + } + state.observedStatBytes = size + if (state.growth) { + const growth = state.growth + state.growth = '' + await fs.appendFile(path, growth) + } + } + return { + ...fs, + stat: async (path: string) => { + const snapshot = await fs.stat(path) + await afterStat(path, snapshot.size) + return snapshot + }, + readFile: async (path: string, encoding: BufferEncoding) => { + if (path === state.path && state.readError) { + throw new Error('Injected read failure') + } + const result = await fs.readFile(path, encoding) + if (path === state.path) { + state.bytesRead += Buffer.byteLength(result) + } + return result + }, + open: async (path: string, flags: string) => { + const handle = await fs.open(path, flags) + if (path !== state.path) { + return handle + } + state.opens += 1 + return { + stat: async () => { + const snapshot = await handle.stat() + await afterStat(path, snapshot.size) + return snapshot + }, + read: async (buffer: Buffer, offset: number, length: number, position: number) => { + if (state.readError) { + throw new Error('Injected read failure') + } + const result = await handle.read(buffer, offset, length, position) + state.bytesRead += result.bytesRead + return result + }, + close: async () => { + state.closes += 1 + await handle.close() + } + } + } + } +}) + +import { appendFile, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { readClaudeProviderHistoryWindow } from './claude-structured-history-window' + +const LIMIT = 16 * 1024 * 1024 +const SOURCE = `${[ + { + type: 'user', + uuid: 'anchor', + parentUuid: null, + sessionId: 'provider', + message: { role: 'user', content: 'before' } + }, + { + type: 'user', + uuid: 'latest', + parentUuid: 'anchor', + sessionId: 'provider', + message: { role: 'user', content: 'after' } + }, + { type: 'last-prompt', sessionId: 'provider', leafUuid: 'latest' } +] + .map((row) => JSON.stringify(row)) + .join('\n')}\n` +let directory = '' + +const read = (previousLeafUuid: string | null = 'anchor') => + readClaudeProviderHistoryWindow({ + transcriptPath: state.path, + providerSessionId: 'provider', + previousLeafUuid, + sessionId: 'orca', + turnInFlight: false + }) + +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-history-source-budget-')) + Object.assign(state, { + path: join(directory, 'session.jsonl'), + growth: '', + readError: false, + bytesRead: 0, + opens: 0, + closes: 0, + observedStatBytes: 0 + }) + await writeFile(state.path, SOURCE) +}) + +afterEach(async (context) => { + try { + const output = process.env.ORCA_HISTORY_BUDGET_PROOF_OUTPUT + if (output) { + await appendFile( + output, + `${JSON.stringify({ + test: context.task.name, + bytesRead: state.bytesRead, + observedStatBytes: state.observedStatBytes, + opens: state.opens, + closes: state.closes + })}\n` + ) + } + expect(state.closes).toBe(state.opens) + } finally { + await rm(directory, { recursive: true, force: true }) + } +}) + +describe('Claude provider history source budget', () => { + it('reads a stable history without changing prompt evidence', async () => { + const result = await read() + expect(result.boundaryConsistent).toBe(true) + expect(result.items.map((item) => item.providerItemId)).toEqual(['latest']) + }) + + it('accepts a complete source at the existing byte limit', async () => { + await writeFile(state.path, SOURCE + ' '.repeat(LIMIT - Buffer.byteLength(SOURCE))) + expect((await read()).boundaryConsistent).toBe(true) + expect(state.bytesRead).toBe(LIMIT) + }) + + it('rejects an initially oversized source before reading its contents', async () => { + await writeFile(state.path, SOURCE + ' '.repeat(LIMIT + 1 - Buffer.byteLength(SOURCE))) + expect((await read()).boundaryConsistent).toBe(false) + expect(state.bytesRead).toBe(0) + }) + + it('refuses concurrent growth beyond the existing source quota', async () => { + state.growth = ' '.repeat(LIMIT + 1024 * 1024 - Buffer.byteLength(SOURCE)) + const result = await read() + expect(state.observedStatBytes).toBe(Buffer.byteLength(SOURCE)) + expect(result.boundaryConsistent).toBe(false) + expect(result.items).toEqual([]) + expect(state.bytesRead).toBeLessThanOrEqual(LIMIT + 1) + }) + + it('accepts concurrent growth that stays within the quota', async () => { + state.growth = ' \n'.repeat(32) + expect((await read()).boundaryConsistent).toBe(true) + expect(state.bytesRead).toBe(Buffer.byteLength(SOURCE) + 64) + }) + + it('preserves the inconsistent result on a read error', async () => { + state.readError = true + expect((await read()).boundaryConsistent).toBe(false) + }) + + it('does not open a source without an anchor', async () => { + expect((await read(null)).boundaryConsistent).toBe(false) + expect(state.opens).toBe(0) + expect(state.bytesRead).toBe(0) + }) +}) diff --git a/src/main/claude/claude-structured-history-window.ts b/src/main/claude/claude-structured-history-window.ts index 1cfd48a8a63..f7a541fa317 100644 --- a/src/main/claude/claude-structured-history-window.ts +++ b/src/main/claude/claude-structured-history-window.ts @@ -12,8 +12,8 @@ // of those makes absence meaningless. Failing it reports an inconsistent // boundary rather than an empty window, because the two decide opposite things. -import { readFile, stat } from 'node:fs/promises' import { join } from 'node:path' +import { readNodeFileWithinLimit } from '../../shared/node-bounded-file-reader' import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' import { resolveSessionFilePath } from '../native-chat/session-file-resolver' import type { @@ -284,10 +284,11 @@ export async function readClaudeProviderHistoryWindow(input: { } let contents: string try { - if ((await stat(input.transcriptPath)).size > MAX_HISTORY_WINDOW_SOURCE_BYTES) { - return INCONSISTENT - } - contents = await readFile(input.transcriptPath, 'utf8') + const read = await readNodeFileWithinLimit( + input.transcriptPath, + MAX_HISTORY_WINDOW_SOURCE_BYTES + ) + contents = read.buffer.toString('utf8') } catch { return INCONSISTENT }