From 711245821bf51ddc786e5fdd96b16a85dd15e799 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 19 Jun 2026 01:31:56 +0800 Subject: [PATCH] Handle corrupt JSONL sidecars during list --- packages/invariants/src/index.ts | 4 ++++ .../session-persistence-jsonl/src/index.ts | 23 ++++++++++++++----- .../tests/jsonl.spec.ts | 21 +++++++++++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/packages/invariants/src/index.ts b/packages/invariants/src/index.ts index 254b1282c7..b201e1ae43 100644 --- a/packages/invariants/src/index.ts +++ b/packages/invariants/src/index.ts @@ -118,6 +118,9 @@ function checkEvent(trace: SessionTrace, event: SessionEvent): void { if (trace.openTurn !== null) { throw new InvariantError(`turn/start ${event.data.turn} while turn ${trace.openTurn} is still open`) } + // Current sessions replay full logs, so numbering starts at 1 and remains + // contiguous. If a future compaction/fork stores a partial log, it must + // seed `nextTurn` from retained metadata before this check runs. if (event.data.turn !== trace.nextTurn) { throw new InvariantError(`turn/start expected turn ${trace.nextTurn}, got ${event.data.turn}`) } @@ -143,6 +146,7 @@ function checkEvent(trace: SessionTrace, event: SessionEvent): void { if (trace.openStep !== null) { throw new InvariantError(`step/start ${event.data.step} while step ${trace.openStep} is still open`) } + // Steps are checked under the same full-log assumption as turns above. if (event.data.step !== trace.nextStep) { throw new InvariantError(`step/start expected step ${trace.nextStep} in turn ${event.data.turn}, got ${event.data.step}`) } diff --git a/packages/session-persistence-jsonl/src/index.ts b/packages/session-persistence-jsonl/src/index.ts index a2f63470cd..f68dde5a0c 100644 --- a/packages/session-persistence-jsonl/src/index.ts +++ b/packages/session-persistence-jsonl/src/index.ts @@ -354,7 +354,7 @@ export class SessionPersistenceJsonl extends SessionPersistence { if (first === undefined) continue // empty/half-written file const meta = parseHeaderMeta(first) if (meta === undefined) continue // not a session header - const summary = await this.readSidecar(meta.id, meta.cwd) + const summary = await this.readSidecarForList(meta.id, meta.cwd) metas.push({ ...meta, ...summary }) } } @@ -600,11 +600,9 @@ export class SessionPersistenceJsonl extends SessionPersistence { } /** - * Read the mutable-summary sidecar, or `undefined` if it is absent/unreadable - * (a session that has never been `update()`d, or a failed sidecar write). The - * caller keeps the header-derived `updatedAt` (the session's createdAt) in - * that case rather than overlaying `0` — reporting an active session as - * updated at the Unix epoch would be wrong. + * Read the mutable-summary sidecar, or `undefined` if it is absent (a session + * that has never been `update()`d). Non-ENOENT failures surface on strict + * load/adopt paths so corrupt metadata does not masquerade as a clean default. */ private async readSidecar(id: SessionId, cwd: string | undefined): Promise { try { @@ -616,6 +614,19 @@ export class SessionPersistenceJsonl extends SessionPersistence { } } + /** + * Best-effort summary read for list(): a corrupt sidecar should degrade one + * row to header metadata, not hide every session from a picker. + */ + private async readSidecarForList(id: SessionId, cwd: string | undefined): Promise { + try { + return await this.readSidecar(id, cwd) + } catch (error: unknown) { + this.ctx.logger.warn(`session-persistence-jsonl: ignoring unreadable summary for session "${id}" while listing: ${String(error)}`) + return undefined + } + } + // --- discovery helpers --- /** Find a session's log file across cwd buckets (when cwd is unknown). */ diff --git a/packages/session-persistence-jsonl/tests/jsonl.spec.ts b/packages/session-persistence-jsonl/tests/jsonl.spec.ts index 08fde20e9d..8257a9853c 100644 --- a/packages/session-persistence-jsonl/tests/jsonl.spec.ts +++ b/packages/session-persistence-jsonl/tests/jsonl.spec.ts @@ -765,6 +765,27 @@ describe('SessionPersistenceJsonl: edge cases', () => { expect(ids).toEqual(['p1', 'p2', 'p3']) }) + it('list tolerates one corrupt sidecar and still returns other sessions', async () => { + const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => undefined) + const bad = meta('bad-list-summary', '/proj') + await ctx.sessionPersistence.create(bad) + await ctx.sessionPersistence.append(bad.id, oneTurnLog()) + await ctx.sessionPersistence.update(bad.id, { title: 'hidden by corrupt sidecar' }) + await writeFile(sidecarPath(root, '/proj', bad.id), '{not json') + const good = meta('good-list-summary', '/proj') + await ctx.sessionPersistence.create(good) + await ctx.sessionPersistence.append(good.id, oneTurnLog()) + await ctx.sessionPersistence.update(good.id, { title: 'visible' }) + + const listed = await ctx.sessionPersistence.list() + + const badListed = listed.find(m => m.id === bad.id) + expect(badListed).toMatchObject({ id: bad.id }) + expect(badListed).not.toHaveProperty('title') + expect(listed.find(m => m.id === good.id)).toMatchObject({ id: good.id, title: 'visible' }) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('bad-list-summary')) + }) + it('list on an empty root returns nothing', async () => { expect(await ctx.sessionPersistence.list()).toEqual([]) })