Handle corrupt JSONL sidecars during list
This commit is contained in:
@@ -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}`)
|
||||
}
|
||||
|
||||
@@ -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<SessionSummary | undefined> {
|
||||
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<SessionSummary | undefined> {
|
||||
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). */
|
||||
|
||||
@@ -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([])
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user