diff --git a/cli/src/codex/session.ts b/cli/src/codex/session.ts index 6c53fb17..c8892419 100644 --- a/cli/src/codex/session.ts +++ b/cli/src/codex/session.ts @@ -102,9 +102,16 @@ export class CodexSession extends AgentSessionBase { this.sessionId = null; this.resetTranscriptPath(); this.client.updateMetadata((metadata: Metadata) => { - const updated = { ...metadata }; - delete updated.codexSessionId; - return updated; + // Explicit-clear sentinel: `null` instructs the hub merge to + // drop `codexSessionId` from the persisted blob. Plain + // `delete` arrives at the hub as an omitted field, which the + // carry-forward path then restores from the prior row — + // defeating the reset. See hub/src/store/sessions.ts + // mergeSessionMetadata. The value is `null` on the wire only; + // MetadataSchema parses `string().optional()`, so the + // post-merge persisted blob carries no key. + const updated: Record = { ...metadata, codexSessionId: null }; + return updated as unknown as Metadata; }); } diff --git a/hub/src/socket/handlers/cli/sessionHandlers.test.ts b/hub/src/socket/handlers/cli/sessionHandlers.test.ts index 54d6c761..428299ce 100644 --- a/hub/src/socket/handlers/cli/sessionHandlers.test.ts +++ b/hub/src/socket/handlers/cli/sessionHandlers.test.ts @@ -6,9 +6,9 @@ import { registerSessionHandlers } from './sessionHandlers' class FakeSocket { readonly roomEvents: Array<{ room: string; event: string; data: unknown }> = [] - private readonly handlers = new Map void>() + private readonly handlers = new Map void) => void>() - on(event: string, handler: (data: unknown) => void): this { + on(event: string, handler: (data: unknown, ack?: (response: unknown) => void) => void): this { this.handlers.set(event, handler) return this } @@ -21,8 +21,8 @@ class FakeSocket { } } - trigger(event: string, data: unknown): void { - this.handlers.get(event)?.(data) + trigger(event: string, data: unknown, ack?: (response: unknown) => void): void { + this.handlers.get(event)?.(data, ack) } } @@ -64,4 +64,60 @@ describe('cli session handlers', () => { expect(socket.roomEvents).toHaveLength(0) expect(webEvents).toHaveLength(0) }) + + it('update-metadata broadcasts the merged value, not the pre-merge payload', () => { + const store = new Store(':memory:') + const session = store.sessions.getOrCreateSession( + 'broadcast-merged', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'broadcast-survives' + }, + null, + 'default' + ) + const socket = new FakeSocket() + + registerSessionHandlers(socket as unknown as CliSocketWithData, { + store, + resolveSessionAccess: () => ({ ok: true, value: session as StoredSession }), + emitAccessError: () => { + throw new Error('unexpected access error') + } + }) + + let ackResponse: unknown = null + socket.trigger( + 'update-metadata', + { + sid: session.id, + expectedVersion: session.metadataVersion, + metadata: { + lifecycleState: 'archived', + archivedBy: 'cli', + archiveReason: 'Session crashed' + } + }, + (response) => { + ackResponse = response + } + ) + + // Ack: success and the version bumps; the persisted value carries the + // merged metadata so other CLIs can update their cache to the truth. + const ack = ackResponse as { result: string; version: number; metadata: unknown } + expect(ack.result).toBe('success') + const ackMetadata = ack.metadata as Record + expect(ackMetadata.cursorSessionId).toBe('broadcast-survives') + expect(ackMetadata.path).toBe('/tmp/project') + + // Broadcast: the room event must carry the same merged value. + const broadcast = socket.roomEvents.find((event) => event.event === 'update') + expect(broadcast).toBeDefined() + const broadcastBody = (broadcast?.data as { body: { metadata: { value: Record } } }).body + expect(broadcastBody.metadata.value.cursorSessionId).toBe('broadcast-survives') + expect(broadcastBody.metadata.value.path).toBe('/tmp/project') + expect(broadcastBody.metadata.value.lifecycleState).toBe('archived') + }) }) diff --git a/hub/src/socket/handlers/cli/sessionHandlers.ts b/hub/src/socket/handlers/cli/sessionHandlers.ts index b2809003..67def89c 100644 --- a/hub/src/socket/handlers/cli/sessionHandlers.ts +++ b/hub/src/socket/handlers/cli/sessionHandlers.ts @@ -202,7 +202,13 @@ export function registerSessionHandlers(socket: CliSocketWithData, deps: Session body: { t: 'update-session' as const, sid, - metadata: { version: result.version, value: metadata }, + // Broadcast the persisted (merged) value, not the pre-merge + // payload — otherwise other CLIs in the session room would + // overwrite their local cache with a tokenless metadata + // snapshot even though the DB row was preserved. + // See store.sessions.mergeSessionMetadata for the merge + // contract. + metadata: { version: result.version, value: result.value }, agentState: null } } diff --git a/hub/src/store/sessions.test.ts b/hub/src/store/sessions.test.ts new file mode 100644 index 00000000..f00a0377 --- /dev/null +++ b/hub/src/store/sessions.test.ts @@ -0,0 +1,759 @@ +import { describe, expect, it } from 'bun:test' +import { Store } from './index' + +function makeStore(): Store { + return new Store(':memory:') +} + +function getMetadata(store: Store, id: string): Record | null { + const row = store.sessions.getSession(id) + return (row?.metadata ?? null) as Record | null +} + +describe('updateSessionMetadata: protocol resume token preservation', () => { + it('preserves cursorSessionId when archive payload omits it (Cursor crash-archive)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-archive-cursor-id', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + cursorSessionId: 'cursor-thread-abc', + cursorSessionProtocol: 'stream-json', + lifecycleState: 'running' + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + lifecycleState: 'archived', + lifecycleStateSince: 2, + archivedBy: 'cli', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + expect(result.result).toBe('success') + + const metadata = getMetadata(store, session.id) + expect(metadata).not.toBeNull() + expect(metadata?.cursorSessionId).toBe('cursor-thread-abc') + expect(metadata?.cursorSessionProtocol).toBe('stream-json') + expect(metadata?.lifecycleState).toBe('archived') + expect(metadata?.archiveReason).toBe('Session crashed') + expect(metadata?.archivedBy).toBe('cli') + }) + + it('preserves codexSessionId when archive payload omits it (Codex generic flavor)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'codex-archive', + { + path: '/tmp/project', + host: 'example', + flavor: 'codex', + codexSessionId: 'codex-thread-1', + lifecycleState: 'running' + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + flavor: 'codex', + lifecycleState: 'archived', + archivedBy: 'cli', + archiveReason: 'User terminated' + }, + session.metadataVersion, + 'default' + ) + expect(result.result).toBe('success') + + const metadata = getMetadata(store, session.id) + expect(metadata?.codexSessionId).toBe('codex-thread-1') + }) + + it.each([ + ['claudeSessionId', 'claude-thread-x'], + ['codexSessionId', 'codex-thread-x'], + ['geminiSessionId', 'gemini-thread-x'], + ['opencodeSessionId', 'opencode-thread-x'], + ['cursorSessionId', 'cursor-thread-x'], + ['kimiSessionId', 'kimi-thread-x'] + ])('preserves %s across an archive metadata replacement', (field, value) => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + `archive-${field}`, + { + path: '/tmp/project', + host: 'example', + [field]: value + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + lifecycleState: 'archived', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + expect(result.result).toBe('success') + + const metadata = getMetadata(store, session.id) + expect(metadata?.[field]).toBe(value) + }) + + it('preserves cursorSessionProtocol independently of cursorSessionId', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-protocol-only', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + cursorSessionProtocol: 'acp' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { path: '/tmp/project', host: 'example' }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) + expect(metadata?.cursorSessionProtocol).toBe('acp') + }) + + it('lets the next write override a flavor session id when it explicitly sets a different value', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-overwrite', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'old-thread' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'new-thread' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) + expect(metadata?.cursorSessionId).toBe('new-thread') + }) + + it('does not invent fields when the prior row had no resume token', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'no-prior-token', + { path: '/tmp/project', host: 'example' }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + lifecycleState: 'archived', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) + expect(metadata).not.toBeNull() + expect('cursorSessionId' in (metadata as Record)).toBe(false) + expect('codexSessionId' in (metadata as Record)).toBe(false) + }) + + it('preserves resume token when CLI sends an empty payload (stale-cache failure mode)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-empty-payload', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'survives-empty-payload' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + lifecycleState: 'archived', + archivedBy: 'cli', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) + expect(metadata?.cursorSessionId).toBe('survives-empty-payload') + expect(metadata?.lifecycleState).toBe('archived') + }) + + it('preserves resume token across multiple consecutive metadata writes', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-multi-write', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'persistent-thread' + }, + null, + 'default' + ) + + const v1 = store.sessions.updateSessionMetadata( + session.id, + { path: '/tmp/project', host: 'example', name: 'renamed' }, + session.metadataVersion, + 'default' + ) + expect(v1.result).toBe('success') + + const v2 = store.sessions.updateSessionMetadata( + session.id, + { path: '/tmp/project', host: 'example', name: 'renamed', tools: ['read_file'] }, + v1.result === 'success' ? v1.version : -1, + 'default' + ) + expect(v2.result).toBe('success') + + const metadata = getMetadata(store, session.id) + expect(metadata?.cursorSessionId).toBe('persistent-thread') + expect(metadata?.name).toBe('renamed') + expect(metadata?.tools).toEqual(['read_file']) + }) + + it('returns version-mismatch unchanged when the expected version is stale', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-version-mismatch', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'stable-id' + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { path: '/tmp/project', host: 'example' }, + session.metadataVersion + 99, + 'default' + ) + expect(result.result).toBe('version-mismatch') + if (result.result === 'version-mismatch') { + const value = result.value as Record | null + expect(value?.cursorSessionId).toBe('stable-id') + } + }) + + it('returns error when the session row does not exist', () => { + const store = makeStore() + const result = store.sessions.updateSessionMetadata( + 'no-such-session', + { path: '/tmp/project', host: 'example' }, + 0, + 'default' + ) + expect(result.result).toBe('error') + }) + + it('archive then read-back ships a payload that legacy resume routing can use', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-roundtrip', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + cursorSessionId: 'legacy-uuid', + lifecycleState: 'running' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + lifecycleState: 'archived', + archiveReason: 'Session crashed', + archivedBy: 'cli' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) + // Legacy routing in cursorProtocol.isLegacyCursorSession() defaults to + // legacy when cursorSessionProtocol is unset and cursorSessionId is + // truthy. Preserving the id alone is enough for resume to route + // correctly even if the protocol marker was never persisted. + expect(metadata?.cursorSessionId).toBe('legacy-uuid') + expect(metadata?.flavor).toBe('cursor') + }) + + // P1 from cold review: a sparse archive payload must result in a + // metadata blob that still parses against MetadataSchema (path/host + // are required). Without these, downstream consumers null-out the + // metadata and resume cannot find the session even though the + // resume token survived in the DB. + it('preserves required path and host when archive payload is sparse (sparse-cache failure mode)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-sparse-archive', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + cursorSessionId: 'parse-required' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + lifecycleState: 'archived', + archivedBy: 'cli', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.path).toBe('/tmp/project') + expect(metadata?.host).toBe('example') + expect(metadata?.cursorSessionId).toBe('parse-required') + expect(metadata?.lifecycleState).toBe('archived') + }) + + it('does not invent path or host when prior had none', () => { + const store = makeStore() + // create with minimal raw metadata (path is technically required by + // the schema, but the store accepts any JSON; this exercises the + // edge case where prior is missing identity fields) + const session = store.sessions.getOrCreateSession( + 'no-prior-identity', + { flavor: 'cursor' }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { lifecycleState: 'archived' }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.lifecycleState).toBe('archived') + expect('path' in (metadata ?? {})).toBe(false) + expect('host' in (metadata ?? {})).toBe(false) + }) + + // P2 from cold review: flavor + machineId are routing fields. Without + // flavor, hub/src/web/routes/sessions.ts and syncEngine fall through + // to the `?? 'claude'` default and ignore the preserved Cursor/Codex + // token. Without machineId, the CLI's resumable listing filters the + // row out of the resume picker. Both must survive sparse archive. + it('preserves flavor and machineId across sparse archive (resume routing)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-routing-survives', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + machineId: 'mach-xyz', + cursorSessionId: 'cursor-thread-routed' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + lifecycleState: 'archived', + archivedBy: 'cli', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.flavor).toBe('cursor') + expect(metadata?.machineId).toBe('mach-xyz') + expect(metadata?.cursorSessionId).toBe('cursor-thread-routed') + }) + + it('does not invent flavor or machineId when prior had none', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'no-prior-routing', + { path: '/tmp/project', host: 'example' }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { lifecycleState: 'archived' }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.lifecycleState).toBe('archived') + expect('flavor' in (metadata ?? {})).toBe(false) + expect('machineId' in (metadata ?? {})).toBe(false) + }) + + it('lets the next write override flavor and machineId when explicitly set', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'override-routing', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + machineId: 'mach-old' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + flavor: 'codex', + machineId: 'mach-new' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.flavor).toBe('codex') + expect(metadata?.machineId).toBe('mach-new') + }) + + // P2 from cold review: cursorSessionProtocol must NOT carry over + // when the next write explicitly sets a different cursorSessionId. + // The protocol is tied to the id, and a different id may use a + // different protocol (e.g. legacy stream-json id under an old + // ACP marker would be misrouted). + it('drops cursorSessionProtocol when a new cursorSessionId is written', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-protocol-pair-drop', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'old-id', + cursorSessionProtocol: 'acp' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'new-id' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.cursorSessionId).toBe('new-id') + expect(metadata?.cursorSessionProtocol).toBeUndefined() + }) + + it('preserves cursorSessionProtocol when neither id nor protocol is in the next write', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-protocol-pair-preserve', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'stable-id', + cursorSessionProtocol: 'acp' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + lifecycleState: 'archived', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.cursorSessionId).toBe('stable-id') + expect(metadata?.cursorSessionProtocol).toBe('acp') + }) + + it('respects an explicit cursorSessionProtocol on the next write even when the id is unchanged', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-protocol-pair-explicit', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'stable-id' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + cursorSessionProtocol: 'stream-json' + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata?.cursorSessionId).toBe('stable-id') + expect(metadata?.cursorSessionProtocol).toBe('stream-json') + }) + + // P2 from cold review: the broadcast on a successful update must + // ship the merged value so other CLIs in the session room update + // their local cache to the persisted state. This is enforced in the + // socket handler (see hub/src/socket/handlers/cli/sessionHandlers.ts); + // the store-level guarantee here is that result.value reflects the + // merged state and not the pre-merge input. + it('returns the merged value in the success ack, not the pre-merge input', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'cursor-ack-merged', + { + path: '/tmp/project', + host: 'example', + cursorSessionId: 'should-survive-ack' + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { + lifecycleState: 'archived', + archivedBy: 'cli', + archiveReason: 'Session crashed' + }, + session.metadataVersion, + 'default' + ) + + expect(result.result).toBe('success') + if (result.result === 'success') { + const value = result.value as Record | null + expect(value?.path).toBe('/tmp/project') + expect(value?.host).toBe('example') + expect(value?.cursorSessionId).toBe('should-survive-ack') + expect(value?.lifecycleState).toBe('archived') + } + }) + + // Upstream cold-review (Major): preserve-on-omit must not block + // intentional clears. `cli/src/codex/session.ts resetCodexThread()` + // is the existing site that needs to drop `codexSessionId` (called + // from /clear in codexRemoteLauncher.ts). The explicit-clear + // sentinel: `null` in `next` means "drop this field entirely from + // the merged blob" (key removed, not stored as null) — distinct + // from omitted (`undefined`) which carries forward. + it('drops a carry-forward field when next sets it to null (explicit clear)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'codex-explicit-clear', + { + path: '/tmp/project', + host: 'example', + flavor: 'codex', + codexSessionId: 'old-thread' + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + flavor: 'codex', + codexSessionId: null + }, + session.metadataVersion, + 'default' + ) + + expect(result.result).toBe('success') + const metadata = getMetadata(store, session.id) as Record | null + expect(metadata).not.toBeNull() + expect('codexSessionId' in (metadata ?? {})).toBe(false) + expect(metadata?.flavor).toBe('codex') + }) + + it('treats null as clear for any carry-forward field, independently of others', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'multi-token-explicit-clear', + { + path: '/tmp/project', + host: 'example', + flavor: 'cursor', + cursorSessionId: 'cursor-keep', + codexSessionId: 'codex-clear-me' + }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + codexSessionId: null + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect('codexSessionId' in (metadata ?? {})).toBe(false) + expect(metadata?.cursorSessionId).toBe('cursor-keep') + expect(metadata?.flavor).toBe('cursor') + }) + + it('null on a never-set field is a no-op (does not introduce the key)', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'null-on-absent', + { path: '/tmp/project', host: 'example' }, + null, + 'default' + ) + + store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + codexSessionId: null + }, + session.metadataVersion, + 'default' + ) + + const metadata = getMetadata(store, session.id) as Record | null + expect('codexSessionId' in (metadata ?? {})).toBe(false) + }) + + it('explicit clear leaves the merged value in the success ack', () => { + const store = makeStore() + const session = store.sessions.getOrCreateSession( + 'explicit-clear-ack', + { + path: '/tmp/project', + host: 'example', + codexSessionId: 'thread-x' + }, + null, + 'default' + ) + + const result = store.sessions.updateSessionMetadata( + session.id, + { + path: '/tmp/project', + host: 'example', + codexSessionId: null + }, + session.metadataVersion, + 'default' + ) + + expect(result.result).toBe('success') + if (result.result === 'success') { + const value = result.value as Record | null + expect('codexSessionId' in (value ?? {})).toBe(false) + expect(value?.path).toBe('/tmp/project') + } + }) +}) diff --git a/hub/src/store/sessions.ts b/hub/src/store/sessions.ts index 82264b83..44921403 100644 --- a/hub/src/store/sessions.ts +++ b/hub/src/store/sessions.ts @@ -5,6 +5,130 @@ import type { StoredSession, VersionedUpdateResult } from './types' import { safeJsonParse } from './json' import { updateVersionedField } from './versionedUpdates' +// Carry-forward fields that the hub preserves across any metadata +// replacement when the incoming write omits them. +// +// The CLI's archive transition (cli/src/agent/runnerLifecycle.ts +// archiveAndClose) spreads `currentMetadata` from the session client's +// local cache; if that cache is `null` (e.g. the row's metadata failed +// Zod parse at bootstrap and got nulled out in cli/src/api/api.ts) or +// stale, the resulting payload is sparse and the unconditional REPLACE +// in updateSessionMetadata wipes whatever it omits. That breaks resume +// even though the on-disk chat data still exists. +// +// Three preservation tiers cover the failure modes: +// +// - PARSE_IDENTITY_FIELDS: required by MetadataSchema in +// shared/src/schemas.ts. Without these, hub session cache and CLI +// getSession reject the row with safeParse → metadata becomes null +// downstream and resume cannot find a path even when the resume +// token survived. +// +// - ROUTING_FIELDS: flavor + machineId. `flavor` is what +// hub/src/web/routes/sessions.ts and hub/src/sync/syncEngine.ts use +// to pick which session id field to read; if it's dropped, the +// `?? 'claude'` fallback misroutes a Cursor/Codex/Gemini session as +// Claude and the preserved token is ignored. `machineId` is the +// filter the CLI's resumable listing uses to scope rows to the +// current host; without it the row drops out of the resume picker. +// +// - SIMPLE_RESUME_TOKENS: flavor-specific resume identifiers that are +// write-once-keep semantics. Mirror of pickExistingSessionMetadata +// in cli/src/agent/sessionFactory.ts. +// +// `cursorSessionProtocol` is paired with `cursorSessionId`: protocol is +// tied to a specific chat id, so a write that explicitly sets a new +// `cursorSessionId` must drop a stale prior protocol. Handled in +// preserveCursorProtocolPair below. +// +// Explicit-clear sentinel: when `next` sets a carry-forward field to +// `null`, the merge drops the key entirely from the output (the +// resulting blob has neither the prior value nor `null`). This lets +// callers intentionally remove a preserved field — e.g. +// `cli/src/codex/session.ts` `resetCodexThread()` clears the codex +// thread id with `codexSessionId: null` so a `/clear` command actually +// drops the persisted thread. `undefined` (key missing from `next`) +// continues to mean "carry forward". +const PARSE_IDENTITY_FIELDS = ['path', 'host'] as const + +const ROUTING_FIELDS = ['flavor', 'machineId'] as const + +const SIMPLE_RESUME_TOKENS = [ + 'claudeSessionId', + 'codexSessionId', + 'geminiSessionId', + 'opencodeSessionId', + 'cursorSessionId', + 'kimiSessionId' +] as const + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function carryForwardIfMissing( + prior: Record, + next: Record, + merged: Record | null, + fields: ReadonlyArray +): Record | null { + let result = merged + for (const field of fields) { + // Explicit-clear sentinel: `null` in next means "drop this field". + // Strip it from the merged output so the persisted blob stays + // schema-clean (MetadataSchema fields are `string().optional()` + // — string|undefined, not nullable). + if (next[field] === null) { + if (result === null) { + result = { ...next } + } + delete result[field] + continue + } + if (next[field] === undefined && prior[field] !== undefined) { + if (result === null) { + result = { ...next } + } + result[field] = prior[field] + } + } + return result +} + +function preserveCursorProtocolPair( + prior: Record, + next: Record, + merged: Record | null +): Record | null { + // If next explicitly sets cursorSessionId, the protocol is tied to + // the new id — never carry over the prior protocol. The next write + // can include its own cursorSessionProtocol if it knows the protocol. + if (next.cursorSessionId !== undefined) { + return merged + } + // Otherwise next is silent on the id (and possibly the protocol); + // carry over the prior protocol so it stays paired with the prior id + // (which is preserved via SIMPLE_RESUME_TOKENS above). + if (next.cursorSessionProtocol === undefined && prior.cursorSessionProtocol !== undefined) { + const result = merged ?? { ...next } + result.cursorSessionProtocol = prior.cursorSessionProtocol + return result + } + return merged +} + +export function mergeSessionMetadata(prior: unknown, next: unknown): unknown { + if (!isPlainObject(prior) || !isPlainObject(next)) { + return next + } + let merged: Record | null = null + merged = carryForwardIfMissing(prior, next, merged, PARSE_IDENTITY_FIELDS) + merged = carryForwardIfMissing(prior, next, merged, ROUTING_FIELDS) + merged = carryForwardIfMissing(prior, next, merged, SIMPLE_RESUME_TOKENS) + merged = preserveCursorProtocolPair(prior, next, merged) + return merged ?? next +} + type DbSessionRow = { id: string tag: string | null @@ -128,29 +252,42 @@ export function updateSessionMetadata( const now = Date.now() const touchUpdatedAt = options?.touchUpdatedAt !== false - return updateVersionedField({ - db, - table: 'sessions', - id, - namespace, - field: 'metadata', - versionField: 'metadata_version', - expectedVersion, - value: metadata, - encode: (value) => { - const json = JSON.stringify(value) - return json === undefined ? null : json - }, - decode: safeJsonParse, - setClauses: [ - 'updated_at = CASE WHEN @touch_updated_at = 1 THEN @updated_at ELSE updated_at END', - 'seq = seq + 1' - ], - params: { - updated_at: now, - touch_updated_at: touchUpdatedAt ? 1 : 0 - } - }) + try { + return db.transaction((): VersionedUpdateResult => { + const priorRow = db.prepare( + 'SELECT metadata FROM sessions WHERE id = ? AND namespace = ?' + ).get(id, namespace) as { metadata: string | null } | undefined + + const prior = priorRow ? safeJsonParse(priorRow.metadata) : null + const merged = mergeSessionMetadata(prior, metadata) + + return updateVersionedField({ + db, + table: 'sessions', + id, + namespace, + field: 'metadata', + versionField: 'metadata_version', + expectedVersion, + value: merged, + encode: (value) => { + const json = JSON.stringify(value) + return json === undefined ? null : json + }, + decode: safeJsonParse, + setClauses: [ + 'updated_at = CASE WHEN @touch_updated_at = 1 THEN @updated_at ELSE updated_at END', + 'seq = seq + 1' + ], + params: { + updated_at: now, + touch_updated_at: touchUpdatedAt ? 1 : 0 + } + }) + })() + } catch { + return { result: 'error' } + } } export function updateSessionAgentState(