diff --git a/packages/session-persistence-jsonl/src/index.ts b/packages/session-persistence-jsonl/src/index.ts index 8f45dccd6a..47d593d3c0 100644 --- a/packages/session-persistence-jsonl/src/index.ts +++ b/packages/session-persistence-jsonl/src/index.ts @@ -23,7 +23,7 @@ import { Context } from 'cordis' import z from 'schemastery' -import { open, mkdir, readFile, readdir, rename, link, rm, writeFile, truncate } from 'node:fs/promises' +import { open, mkdir, readFile, readdir, rename, link, rm, truncate } from 'node:fs/promises' import { resolve } from 'node:path' import { randomBytes } from 'node:crypto' import { SessionPersistence } from '@deepseek-ai/dsh-session-persistence' @@ -103,6 +103,19 @@ function assertSerializable(events: readonly SessionEvent[]): void { } } +/** + * Whether `error` is a "no such file/directory" (`ENOENT`) failure — the ONLY + * filesystem error that legitimately means "this session/root is absent" for a + * durable backend. Any OTHER error (`EACCES`, `ENOTDIR`, transient I/O) must + * surface rather than be silently reported as absence: masking it would let + * `list()` report no sessions, `load()` report "not found", and collision + * checks proceed under a false absence assumption — all unsafe for durable + * persistence. (A NodeJS filesystem rejection carries a string `code`.) + */ +function isENOENT(error: unknown): boolean { + return (error as NodeJS.ErrnoException | null)?.code === 'ENOENT' +} + /** * The JSONL persistence backend. Load as a plugin; it registers as * `ctx.sessionPersistence` and installs the write-path listeners. @@ -504,7 +517,17 @@ export class SessionPersistenceJsonl extends SessionPersistence { ...meta.firstPrompt !== undefined ? { firstPrompt: meta.firstPrompt } : {}, } const tmp = `${path}.${randomBytes(6).toString('hex')}.tmp` - await writeFile(tmp, JSON.stringify(summary), { mode: 0o600 }) + // Exclusive owner-only create ('wx', 0o600), matching the log-materialization + // temp write: the sidecar can carry user data (title/firstPrompt), so a + // predictable/pre-existing temp path must never be silently truncated and + // followed (symlink race / disclosure). The random suffix already makes a + // collision unlikely; 'wx' makes reuse an error rather than a clobber. + const handle = await open(tmp, 'wx', 0o600) + try { + await handle.writeFile(JSON.stringify(summary)) + } finally { + await handle.close() + } await rename(tmp, path) } @@ -550,8 +573,13 @@ export class SessionPersistenceJsonl extends SessionPersistence { try { const entries = await readdir(this.root, { withFileTypes: true }) return entries.filter(e => e.isDirectory()).map(e => `${this.root}/${e.name}`) - } catch { - return [] // root does not exist yet → no sessions + } catch (error) { + // ENOENT = the root has not been created yet → genuinely no sessions. + // Any other error (EACCES, ENOTDIR, transient I/O) must NOT be reported + // as "no sessions" — a durable backend cannot silently pretend persisted + // state is absent on a storage fault. + if (isENOENT(error)) return [] + throw error } } @@ -565,8 +593,12 @@ export class SessionPersistenceJsonl extends SessionPersistence { const handle = await open(path, 'r') await handle.close() return true - } catch { - return false + } catch (error) { + // Only ENOENT means absent. A permission/I/O error must surface, not be + // collapsed to `false` — otherwise load() reports "not found" and + // collision checks proceed under a false absence assumption. + if (isENOENT(error)) return false + throw error } } diff --git a/packages/session-persistence-jsonl/tests/jsonl.spec.ts b/packages/session-persistence-jsonl/tests/jsonl.spec.ts index c63d05226d..a91e6f9138 100644 --- a/packages/session-persistence-jsonl/tests/jsonl.spec.ts +++ b/packages/session-persistence-jsonl/tests/jsonl.spec.ts @@ -1014,6 +1014,36 @@ describe('SessionPersistenceJsonl: edge cases', () => { await ctx2.fiber.dispose() }) + it('list surfaces a non-ENOENT root error (ENOTDIR) instead of reporting no sessions', async () => { + // A durable backend must NOT collapse a storage fault to "no sessions". Point + // the root at a regular FILE: readdir then fails with ENOTDIR, which must + // propagate rather than be swallowed as an empty listing. + const filePath = join(root, 'not-a-dir') + await writeFile(filePath, 'x') + const ctx2 = new Context() + await ctx2.plugin(SessionStore) + await ctx2.plugin(SessionPersistenceJsonl, { root: filePath }) + await expect(ctx2.sessionPersistence.list()).rejects.toThrow(/ENOTDIR/) + await ctx2.fiber.dispose() + }) + + it('exists() surfaces a non-ENOENT lookup error (ENOTDIR) instead of reporting absent', async () => { + // Same contract on the existence path: a non-ENOENT error from the per-id + // open() must surface, not be collapsed to "not found" (which would let a + // collision check proceed under a false absence assumption). A LAZY session + // (created, never appended) keeps its cwd in state, so has() reaches + // findLog(id, cwd) → exists(logPath). Make that cwd's bucket DIRECTORY a + // regular file: open()ing `bucket/.jsonl` under it then fails ENOTDIR. + const cwd = '/x' + const ctx2 = new Context() + await ctx2.plugin(SessionStore) + await ctx2.plugin(SessionPersistenceJsonl, { root }) + await ctx2.sessionPersistence.create(meta('exists-fault', cwd)) // lazy: no bucket yet + await writeFile(sessionDir(root, cwd), 'x') // bucket path is now a FILE + await expect(ctx2.sessionPersistence.has(SessionId('exists-fault'))).rejects.toThrow(/ENOTDIR/) + await ctx2.fiber.dispose() + }) + it('append() to a disk-only session adopts it and repairs a crash tail', async () => { // Persist a session, then corrupt its tail, all through ONE backend. const m = meta('disk-append', '/d') diff --git a/packages/session-persistence/tests/contract.ts b/packages/session-persistence/tests/contract.ts index 94f2ab9e4e..3c4e739b60 100644 --- a/packages/session-persistence/tests/contract.ts +++ b/packages/session-persistence/tests/contract.ts @@ -120,13 +120,31 @@ export function runPersistenceContract(name: string, make: () => Promise { const { persistence, dispose } = await make() try { - const m = meta('s5') - await persistence.create(m) - // A plugin-added event carrying a BigInt (not JSON-serializable). - const bad = [ - { type: 'user/message', seq: 0, time: 1, data: { content: [{ type: 'text', text: 'x' }], source: { kind: 'user' }, extra: 1n } }, - ] as unknown as SessionEvent[] - await expect(persistence.append(m.id, bad)).rejects.toThrow(/user\/message/) + // Every value `isJsonValue` rejects must be rejected by the backend, not + // just BigInt — otherwise a backend could pass this contract while still + // accepting values that corrupt the durable round-trip. Each is a + // plugin-added `extra` field on a single user/message (seq 0). + const cyclic: Record = { type: 'text', text: 'x' } + cyclic['self'] = cyclic + const badValues: unknown[] = [ + 1n, // BigInt + undefined, // dropped by JSON.stringify + Infinity, // → null + () => 0, // function + Symbol('s'), // symbol + new Map(), // exotic object + cyclic, // circular ref + ] + for (const [i, bad] of badValues.entries()) { + // A fresh session per value isolates each rejection (a rejected append + // must leave no state behind, but isolating keeps the assertion clean). + const mi = meta(`s5-${i}`) + await persistence.create(mi) + const events = [ + { type: 'user/message', seq: 0, time: 1, data: { content: [{ type: 'text', text: 'x' }], source: { kind: 'user' }, extra: bad } }, + ] as unknown as SessionEvent[] + await expect(persistence.append(mi.id, events)).rejects.toThrow(/user\/message/) + } } finally { await dispose() } diff --git a/packages/session-persistence/tests/persistence.spec.ts b/packages/session-persistence/tests/persistence.spec.ts index 0ce720cd51..9d211fff52 100644 --- a/packages/session-persistence/tests/persistence.spec.ts +++ b/packages/session-persistence/tests/persistence.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { Context } from 'cordis' -import { SessionId } from '@deepseek-ai/dsh-session' +import { SessionId, isJsonValue } from '@deepseek-ai/dsh-session' import type { SessionEvent, SessionMeta, SessionSummary } from '@deepseek-ai/dsh-session' import { SessionPersistence } from '../src/index.ts' import { runPersistenceContract, meta, oneTurnLog } from './contract.ts' @@ -30,7 +30,7 @@ class MemoryPersistence extends SessionPersistence { for (let i = 0; i < events.length; i++) { const e = events[i]! if (e.seq !== nextSeq + i) throw new Error(`non-contiguous seq in batch for "${id}" at index ${i}`) - if (containsNonSerializable(e.data)) { + if (!isJsonValue(e.data)) { throw new Error(`event "${e.type}" carries non-JSON-serializable data`) } } @@ -68,15 +68,6 @@ class MemoryPersistence extends SessionPersistence { } } -/** Detect BigInt (and other JSON-hostile values) in event data. */ -function containsNonSerializable(value: unknown): boolean { - if (typeof value === 'bigint' || typeof value === 'function' || typeof value === 'symbol') return true - if (value && typeof value === 'object') { - return Object.values(value).some(containsNonSerializable) - } - return false -} - // Run the shared contract against the in-memory backend. runPersistenceContract('memory', async () => { const ctx = new Context()