diff --git a/src/main/runtime/agent-session-record-read-repair.test.ts b/src/main/runtime/agent-session-record-read-repair.test.ts new file mode 100644 index 00000000000..cfc07dac95d --- /dev/null +++ b/src/main/runtime/agent-session-record-read-repair.test.ts @@ -0,0 +1,185 @@ +// Read repair: a recoverable optional field must never cost the session its lease, options and +// provider-handle chain. Anything structural still quarantines the whole record. + +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { AgentSessionRecord } from '../../shared/agent-session-record' +import { AgentSessionRecordStore } from './agent-session-record-store' +import { agentSessionStorePath, loadAgentSessionStore } from './agent-session-record-store-file' +import type { AgentSessionReserveRequest } from './agent-session-reservation-admission' + +const NOW = 1_800_000_000_000 +const SESSION = 'session-alpha' + +let directory: string +let counter = 0 + +function operationId(): string { + counter += 1 + return `${NOW}-${String(counter) + .padStart(32, '0') + .replaceAll(/[^0-9a-f]/g, '0')}` +} + +const reserveRequest = (): AgentSessionReserveRequest => ({ + sessionId: SESSION, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'git-worktree' + }, + provider: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: '/home/dev/.claude-work' }, + runtimeKind: 'native', + expectedFence: null, + spawnToken: 'spawn-a', + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'indeterminate', reason: 'no answer' }, + operation: { callerKey: 'client-1', operationId: operationId(), fingerprint: 'fp-1' }, + now: NOW +}) + +/** Reserve, observe the spawn, prove the handle, carry options — a fully furnished record. */ +async function establishOwnedRecord(): Promise { + const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + const reserved = await store.reserveOwner(reserveRequest()) + const fence = reserved.record.lease.runtimeFence + await store.commitProcessIdentity({ + sessionId: SESSION, + fence, + process: { + hostId: 'local', + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: reserved.record.lease.reservedSpawnToken ?? 'spawn-a' + }, + now: NOW + }) + return store.proveOwner({ + sessionId: SESSION, + fence, + link: { + linkId: 'link-1', + handle: { provider: 'claude', sessionId: 'provider-session-1', leafUuid: 'leaf-1' }, + origin: 'created', + mintedAtFence: fence, + observedAt: NOW + }, + options: { model: 'opus' }, + now: NOW + }) +} + +/** Write a name the single writer would have normalized away, as a stale build could have left it. */ +async function writeStoredName(name: unknown): Promise { + const filePath = agentSessionStorePath(directory) + const raw = JSON.parse(await readFile(filePath, 'utf-8')) + raw.records[SESSION].conversationName = name + await writeFile(filePath, JSON.stringify(raw)) +} + +async function corruptStoredLease(): Promise { + const filePath = agentSessionStorePath(directory) + const raw = JSON.parse(await readFile(filePath, 'utf-8')) + raw.records[SESSION].lease.runtimeFence = 'not-a-number' + await writeFile(filePath, JSON.stringify(raw)) +} + +const load = () => loadAgentSessionStore(agentSessionStorePath(directory), 'local') + +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-session-read-repair-')) +}) + +afterEach(async () => { + await rm(directory, { recursive: true, force: true }) +}) + +describe('agent session record read repair', () => { + it('repairs a noncanonical name instead of discarding the record around it', async () => { + const owned = await establishOwnedRecord() + await writeStoredName('Fix\nthe lease probe') + + const loaded = await load() + + const record = loaded.state.records.get(SESSION) + expect(record?.conversationName).toBe('Fix the lease probe') + expect(loaded.state.unreadableRecords.has(SESSION)).toBe(false) + // The fields quarantine would have taken with it. + expect(record?.lease).toMatchObject({ claimStatus: 'live', runtimeFence: 1 }) + expect(record?.options).toEqual({ model: 'opus' }) + expect(record?.providerHandleChain).toEqual(owned.providerHandleChain) + }) + + it('drops only the field when the stored name normalizes to nothing', async () => { + await establishOwnedRecord() + await writeStoredName('\u202E\u200B ') + + const loaded = await load() + + const record = loaded.state.records.get(SESSION) + expect(record).toBeDefined() + expect(record?.conversationName).toBeUndefined() + expect(Object.hasOwn(record ?? {}, 'conversationName')).toBe(false) + expect(record?.lease.claimStatus).toBe('live') + expect(record?.options).toEqual({ model: 'opus' }) + }) + + it('drops a name of the wrong type rather than the record', async () => { + await establishOwnedRecord() + await writeStoredName(42) + + const loaded = await load() + + expect(loaded.state.records.get(SESSION)?.conversationName).toBeUndefined() + expect(loaded.state.unreadableRecords.has(SESSION)).toBe(false) + }) + + it('still quarantines the whole record when the lease is structurally invalid', async () => { + await establishOwnedRecord() + await writeStoredName('Fix\nthe lease probe') + await corruptStoredLease() + // No committed copy may vouch for the session, or salvage would mask the quarantine. + await rm(`${agentSessionStorePath(directory)}.bak`, { force: true }) + + const loaded = await load() + + expect(loaded.state.records.has(SESSION)).toBe(false) + expect(loaded.state.unreadableRecords.get(SESSION)?.reason).toBe('current_shape_invalid') + // Quarantined bytes stay verbatim: repair must not edit the row it could not rescue. + expect(loaded.state.unreadableRecords.get(SESSION)?.raw).toMatchObject({ + conversationName: 'Fix\nthe lease probe' + }) + }) + + it('leaves a canonical name alone and asks for no rewrite', async () => { + const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await store.reserveOwner(reserveRequest()) + await store.setConversationName(SESSION, 'Fix the lease probe') + + const loaded = await load() + + expect(loaded.state.records.get(SESSION)?.conversationName).toBe('Fix the lease probe') + expect(loaded.needsRewrite).toBe(false) + }) + + it('marks the store for rewrite without touching the record business timestamp', async () => { + const owned = await establishOwnedRecord() + await writeStoredName('Fix\nthe lease probe') + + const loaded = await load() + expect(loaded.needsRewrite).toBe(true) + expect(loaded.state.records.get(SESSION)?.updatedAt).toBe(owned.updatedAt) + + // Opening the store persists that rewrite, so the next load has nothing left to repair. + await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + const persisted = JSON.parse(await readFile(agentSessionStorePath(directory), 'utf-8')) + expect(persisted.records[SESSION].conversationName).toBe('Fix the lease probe') + expect(persisted.records[SESSION].updatedAt).toBe(owned.updatedAt) + expect((await load()).needsRewrite).toBe(false) + }) +}) diff --git a/src/main/runtime/agent-session-record-read-repair.ts b/src/main/runtime/agent-session-record-read-repair.ts new file mode 100644 index 00000000000..a2f2b9bbf6d --- /dev/null +++ b/src/main/runtime/agent-session-record-read-repair.ts @@ -0,0 +1,72 @@ +/** + * Read one stored agent-session record, repairing recoverable optional metadata before validation. + * + * Quarantine is whole-record: a rejected row loses its lease, its options and its provider-handle + * chain, not only the field that failed. `conversationName` is validated against its canonical + * normalization, so widening the normalizer by one codepoint would retroactively invalidate every + * name already on disk — and take those sessions with it. Repairing the field to canonical form + * keeps the record, and keeps the durable "Orca already named this session" marker that stops a + * later acquisition from spending another model call on a name it already has. + * + * Identity and ownership stay out of reach: an invalid lease, session id or handle chain still + * quarantines the whole record, because nothing here can tell what the right value would have been. + */ + +import { normalizeAgentSessionConversationName } from '../../shared/agent-session-conversation-name' +import { + AGENT_SESSION_RECORD_SCHEMA_VERSION, + isAgentSessionRecord, + type AgentSessionRecord +} from '../../shared/agent-session-record' + +export type AgentSessionRecordReadResult = + | { record: AgentSessionRecord; repaired: boolean } + | { record: null; reason: string } + +/** Repaired shallow copy, or null when the stored field is already canonical or absent. The + * original object is left untouched so a row that fails validation anyway quarantines verbatim. */ +function repairConversationName(value: unknown): Record | null { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return null + } + const stored = value as Record + if (!Object.hasOwn(stored, 'conversationName')) { + return null + } + // Same predicate the validator applies, so repair and validation can never disagree. + const normalized = normalizeAgentSessionConversationName(stored.conversationName) + if (normalized === stored.conversationName) { + return null + } + const repaired = { ...stored } + if (normalized === null) { + delete repaired.conversationName + } else { + repaired.conversationName = normalized + } + return repaired +} + +export function readAgentSessionRecord( + sessionId: string, + value: unknown +): AgentSessionRecordReadResult { + const repaired = repairConversationName(value) + const candidate: unknown = repaired ?? value + const record = isAgentSessionRecord(candidate) ? candidate : null + if (record?.sessionId === sessionId) { + return { record, repaired: repaired !== null } + } + const storedSchemaVersion = + typeof value === 'object' && + value !== null && + (value as { schemaVersion?: unknown }).schemaVersion + return { + record: null, + reason: record + ? 'record_key_session_id_mismatch' + : storedSchemaVersion === AGENT_SESSION_RECORD_SCHEMA_VERSION + ? 'current_shape_invalid' + : 'unsupported_schema' + } +} diff --git a/src/main/runtime/agent-session-record-store-file.ts b/src/main/runtime/agent-session-record-store-file.ts index 7eb3cacdc7c..01b0a59ae78 100644 --- a/src/main/runtime/agent-session-record-store-file.ts +++ b/src/main/runtime/agent-session-record-store-file.ts @@ -15,17 +15,14 @@ import { isAgentSessionOperationRow, type AgentSessionOperationRow } from '../../shared/agent-session-operation-ledger' -import { - AGENT_SESSION_RECORD_SCHEMA_VERSION, - isAgentSessionRecord, - type AgentSessionRecord -} from '../../shared/agent-session-record' +import type { AgentSessionRecord } from '../../shared/agent-session-record' import { copyFileDurable, durableWriteTempPath, renameDurable, writeTempFileDurable } from '../durable-file-write' +import { readAgentSessionRecord } from './agent-session-record-read-repair' import { parseVisibleSessionIds } from './agent-session-visible-tab-index' import { serializeAgentSessionStoreState } from './agent-session-store-serialization' @@ -144,20 +141,13 @@ function parseState( let needsRewrite = false if (typeof file.records === 'object' && file.records !== null) { for (const [sessionId, value] of Object.entries(file.records)) { - const record = isAgentSessionRecord(value) ? value : null - if (record?.sessionId === sessionId) { - state.records.set(sessionId, record) + const read = readAgentSessionRecord(sessionId, value) + if (read.record) { + state.records.set(sessionId, read.record) + // A repaired record must be re-persisted, or the next load repairs it again. + needsRewrite ||= read.repaired && schemaVersion === AGENT_SESSION_STORE_SCHEMA_VERSION } else { - const valueSchemaVersion = - typeof value === 'object' && - value !== null && - (value as { schemaVersion?: unknown }).schemaVersion - const reason = record - ? 'record_key_session_id_mismatch' - : valueSchemaVersion === AGENT_SESSION_RECORD_SCHEMA_VERSION - ? 'current_shape_invalid' - : 'unsupported_schema' - state.unreadableRecords.set(sessionId, { reason, raw: value }) + state.unreadableRecords.set(sessionId, { reason: read.reason, raw: value }) needsRewrite ||= schemaVersion === AGENT_SESSION_STORE_SCHEMA_VERSION } }