Skip to content

Commit cd99cfb

Browse files
fix(hub): preserve session metadata across archive transitions (#825)
* fix(hub): preserve flavor session ids in metadata across archive transitions When a session ends (terminate, crash, local-launch failure, handoff), the runner's archive write replaces sessions.metadata wholesale. If the CLI's locally cached Metadata is null (e.g. Zod parse failed at bootstrap and api.ts nulled it out) or stale, the spread in archiveAndClose ships a sparse blob and the resume token (cursorSessionId, codexSessionId, claudeSessionId, etc.) gets cleared from the row even though the on-disk chat data is still intact. Fix at the hub layer because update-metadata is the single chokepoint for every metadata write surface (CLI, web, future): in the store-level updateSessionMetadata, read the prior row's metadata inside a transaction and carry forward a small allowlist of flavor resume tokens when the incoming write omits them. Explicit overwrites still win. The allowlist mirrors pickExistingSessionMetadata in sessionFactory.ts which already preserves the same fields on bootstrap. Closes #820 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(hub): address cold-review findings on metadata merge Three bot findings on the initial patch: 1. (P1) Sparse archive payloads still resulted in metadata blobs that failed MetadataSchema parse downstream — required `path`/`host` were not in the carry-forward set, so even though the resume token survived, hub session cache and CLI getSession nulled-out the row and resume_unavailable came back. Add PARSE_IDENTITY_FIELDS = `path`, `host` to the carry-forward. 2. (P2) Preserving `cursorSessionProtocol` whenever it was omitted carried a stale protocol over to a freshly written `cursorSessionId`, misrouting a future remote resume. Pair-aware logic: drop the prior protocol when next sets a new id; preserve the protocol only when next is silent on both id and protocol. 3. (P2) The successful update-metadata broadcast emitted the pre-merge payload to other CLIs in the session room, so even though the DB row was preserved, peer caches diverged. Switch the broadcast value to `result.value` (the persisted merged value) so live caches stay in sync with the truth. Refactor preserveProtocolResumeFields into mergeSessionMetadata with two tiers (PARSE_IDENTITY_FIELDS + SIMPLE_RESUME_TOKENS) plus the cursor pair handler. 6 new tests cover the regressions; existing 16 still pass plus 1 new socket-level test for the broadcast. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(hub): preserve flavor + machineId across sparse metadata merges Bot P2 on the prior fix: PARSE_IDENTITY_FIELDS (path, host) made the blob parseable and SIMPLE_RESUME_TOKENS preserved the chat-id, but flavor and machineId were still being dropped by sparse archive payloads. Consequences: - flavor: hub/src/web/routes/sessions.ts and sync/syncEngine.ts read `metadata?.flavor ?? 'claude'` to pick which session id field to resume. With flavor missing, a Cursor/Codex/Gemini session was routed as Claude and the preserved cursorSessionId was ignored. - machineId: telegram/bot.ts and the CLI's resumable listing read `metadata?.machineId` to scope sessions to the current host. With machineId missing, the row dropped out of the resume picker. Add a third carry-forward tier ROUTING_FIELDS = [flavor, machineId] between PARSE_IDENTITY_FIELDS and SIMPLE_RESUME_TOKENS in mergeSessionMetadata. 3 new tests cover preservation, no-invention, and explicit override. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(hub,cli): support explicit-clear sentinel for carry-forward fields Upstream cold-review (Major): the carry-forward semantics introduced in the prior commits ("omit field → preserve from prior") collide with cli/src/codex/session.ts resetCodexThread(), which intentionally clears codexSessionId by deleting it from the metadata blob before calling updateMetadata. With omit-as-preserve, the cleared id was restored from the prior row and /clear on a Codex session no longer dropped the persisted thread. Add an explicit-clear sentinel: when next sets a carry-forward field to `null`, the merge drops the key entirely from the persisted blob (key removed; not stored as null since MetadataSchema fields are `string().optional()`). `undefined` (key missing from next) keeps its "carry forward" meaning. The two semantics now compose cleanly: - next.field = "x" → next wins (caller sets a new value) - next.field = null → drop the field (caller intentionally clears) - next omits field → carry forward prior (caller didn't touch it) Update resetCodexThread() to send `codexSessionId: null` so the reset actually drops the persisted thread under the new merge. 4 new hub tests cover: explicit clear of a single token, clear-one- preserve-others independence, no-op clear on a never-set field, and the success-ack value reflects the cleared blob. cli/src/codex tests (224/224) and hub suite (301/301) green; bun typecheck clean. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent edc3acc commit cd99cfb

5 files changed

Lines changed: 996 additions & 31 deletions

File tree

cli/src/codex/session.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,16 @@ export class CodexSession extends AgentSessionBase<EnhancedMode> {
102102
this.sessionId = null;
103103
this.resetTranscriptPath();
104104
this.client.updateMetadata((metadata: Metadata) => {
105-
const updated = { ...metadata };
106-
delete updated.codexSessionId;
107-
return updated;
105+
// Explicit-clear sentinel: `null` instructs the hub merge to
106+
// drop `codexSessionId` from the persisted blob. Plain
107+
// `delete` arrives at the hub as an omitted field, which the
108+
// carry-forward path then restores from the prior row —
109+
// defeating the reset. See hub/src/store/sessions.ts
110+
// mergeSessionMetadata. The value is `null` on the wire only;
111+
// MetadataSchema parses `string().optional()`, so the
112+
// post-merge persisted blob carries no key.
113+
const updated: Record<string, unknown> = { ...metadata, codexSessionId: null };
114+
return updated as unknown as Metadata;
108115
});
109116
}
110117

hub/src/socket/handlers/cli/sessionHandlers.test.ts

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import { registerSessionHandlers } from './sessionHandlers'
66

77
class FakeSocket {
88
readonly roomEvents: Array<{ room: string; event: string; data: unknown }> = []
9-
private readonly handlers = new Map<string, (data: unknown) => void>()
9+
private readonly handlers = new Map<string, (data: unknown, ack?: (response: unknown) => void) => void>()
1010

11-
on(event: string, handler: (data: unknown) => void): this {
11+
on(event: string, handler: (data: unknown, ack?: (response: unknown) => void) => void): this {
1212
this.handlers.set(event, handler)
1313
return this
1414
}
@@ -21,8 +21,8 @@ class FakeSocket {
2121
}
2222
}
2323

24-
trigger(event: string, data: unknown): void {
25-
this.handlers.get(event)?.(data)
24+
trigger(event: string, data: unknown, ack?: (response: unknown) => void): void {
25+
this.handlers.get(event)?.(data, ack)
2626
}
2727
}
2828

@@ -64,4 +64,60 @@ describe('cli session handlers', () => {
6464
expect(socket.roomEvents).toHaveLength(0)
6565
expect(webEvents).toHaveLength(0)
6666
})
67+
68+
it('update-metadata broadcasts the merged value, not the pre-merge payload', () => {
69+
const store = new Store(':memory:')
70+
const session = store.sessions.getOrCreateSession(
71+
'broadcast-merged',
72+
{
73+
path: '/tmp/project',
74+
host: 'example',
75+
cursorSessionId: 'broadcast-survives'
76+
},
77+
null,
78+
'default'
79+
)
80+
const socket = new FakeSocket()
81+
82+
registerSessionHandlers(socket as unknown as CliSocketWithData, {
83+
store,
84+
resolveSessionAccess: () => ({ ok: true, value: session as StoredSession }),
85+
emitAccessError: () => {
86+
throw new Error('unexpected access error')
87+
}
88+
})
89+
90+
let ackResponse: unknown = null
91+
socket.trigger(
92+
'update-metadata',
93+
{
94+
sid: session.id,
95+
expectedVersion: session.metadataVersion,
96+
metadata: {
97+
lifecycleState: 'archived',
98+
archivedBy: 'cli',
99+
archiveReason: 'Session crashed'
100+
}
101+
},
102+
(response) => {
103+
ackResponse = response
104+
}
105+
)
106+
107+
// Ack: success and the version bumps; the persisted value carries the
108+
// merged metadata so other CLIs can update their cache to the truth.
109+
const ack = ackResponse as { result: string; version: number; metadata: unknown }
110+
expect(ack.result).toBe('success')
111+
const ackMetadata = ack.metadata as Record<string, unknown>
112+
expect(ackMetadata.cursorSessionId).toBe('broadcast-survives')
113+
expect(ackMetadata.path).toBe('/tmp/project')
114+
115+
// Broadcast: the room event must carry the same merged value.
116+
const broadcast = socket.roomEvents.find((event) => event.event === 'update')
117+
expect(broadcast).toBeDefined()
118+
const broadcastBody = (broadcast?.data as { body: { metadata: { value: Record<string, unknown> } } }).body
119+
expect(broadcastBody.metadata.value.cursorSessionId).toBe('broadcast-survives')
120+
expect(broadcastBody.metadata.value.path).toBe('/tmp/project')
121+
expect(broadcastBody.metadata.value.lifecycleState).toBe('archived')
122+
})
67123
})

hub/src/socket/handlers/cli/sessionHandlers.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,13 @@ export function registerSessionHandlers(socket: CliSocketWithData, deps: Session
202202
body: {
203203
t: 'update-session' as const,
204204
sid,
205-
metadata: { version: result.version, value: metadata },
205+
// Broadcast the persisted (merged) value, not the pre-merge
206+
// payload — otherwise other CLIs in the session room would
207+
// overwrite their local cache with a tokenless metadata
208+
// snapshot even though the DB row was preserved.
209+
// See store.sessions.mergeSessionMetadata for the merge
210+
// contract.
211+
metadata: { version: result.version, value: result.value },
206212
agentState: null
207213
}
208214
}

0 commit comments

Comments
 (0)