diff --git a/packages/invariants/tests/invariants.spec.ts b/packages/invariants/tests/invariants.spec.ts index 13f908b451..b96d172c22 100644 --- a/packages/invariants/tests/invariants.spec.ts +++ b/packages/invariants/tests/invariants.spec.ts @@ -277,12 +277,15 @@ describe('dev-freeze', () => { // mutable. deepFreeze must descend into the already-frozen object and // freeze the descendant, not short-circuit on the frozen container — // otherwise dev-mode misses exactly the history mutation ADR 0012 catches. + // `append` snapshots `data`, so the freeze applies to the LOGGED clone, not + // the caller's input — read the event back and assert on its data. const innerContent: { type: 'text'; text: string }[] = [{ type: 'text', text: 'inner' }] const block = Object.freeze({ type: 'tool-result' as const, toolCallId: CallId('c1'), content: innerContent, isError: false }) - session.append('user/message', { content: [block], source: { kind: 'user' } }) - expect(Object.isFrozen(block.content)).toBe(true) - expect(Object.isFrozen(block.content[0])).toBe(true) - expect(() => { block.content.push({ type: 'text', text: 'mutation' }) }).toThrow() + const event = session.append('user/message', { content: [block], source: { kind: 'user' } }) + const logged = event.data.content[0] as { content: { type: 'text'; text: string }[] } + expect(Object.isFrozen(logged.content)).toBe(true) + expect(Object.isFrozen(logged.content[0])).toBe(true) + expect(() => { logged.content.push({ type: 'text', text: 'mutation' }) }).toThrow() }) it('terminates on a cyclic event datum (WeakSet guard)', async () => { diff --git a/packages/session/README.md b/packages/session/README.md index 705e3d5b39..7cf443fa71 100644 --- a/packages/session/README.md +++ b/packages/session/README.md @@ -8,7 +8,7 @@ Creates and holds event-sourced `Session` instances. Persistence is intentionall ### Public API -- `ctx.sessions.create(id?: string, options?: { seed?: SessionEvent[]; meta?: { cwd?: string; parentSession?: SessionId } }): Session` — Create a session. `options.seed` replays/forks an existing event log; `options.meta` attaches creation metadata (validated absolute `cwd`, `parentSession` lineage) as the immutable `SessionHeader` (the store fills `version`/`id`/`createdAt`). Disposed with the calling fiber. +- `ctx.sessions.create(id?: string, options?: { seed?: SessionEvent[]; meta?: { cwd?: string; parentSession?: SessionId; createdAt?: number } }): Session` — Create a session. `options.seed` replays/forks an existing event log; `options.meta` attaches creation metadata (validated absolute `cwd`, `parentSession` lineage) as the immutable `SessionHeader`. The store fills `version`/`id` and defaults `createdAt` to now; a caller reconstructing a persisted session passes the original `createdAt` to preserve it. Disposed with the calling fiber. - `ctx.sessions.get(id: string): Session | undefined` - `ctx.sessions.list(): Session[]` diff --git a/packages/session/src/index.ts b/packages/session/src/index.ts index 62cce89c7b..8fe54cb97b 100644 --- a/packages/session/src/index.ts +++ b/packages/session/src/index.ts @@ -90,7 +90,15 @@ export class Session { throw new Error(`seed event "${event.type}" (seq ${event.seq}) carries non-JSON-serializable data`) } }) - this.log = [...seed] + // Deep-clone each seed event, NOT just the array: the seed events and + // their `data` are still owned by the caller (or the source session of a + // fork), so keeping the references would let a post-create mutation of the + // original rewrite this session's durable log — or reintroduce a + // non-JSON-serializable value AFTER the validation above. Snapshotting at + // the boundary makes `session.events` independent and keeps it equal to + // what was validated. Serializability is guaranteed by the check above, so + // structuredClone can never hit a non-cloneable value here. + this.log = seed.map(event => structuredClone(event)) } this.header = header ?? { version: 1, id, createdAt: Date.now() } } @@ -120,7 +128,16 @@ export class Session { if (!isJsonValue(data)) { throw new Error(`session event "${type}" carries non-JSON-serializable data`) } - const event = { type, seq: this.log.length, time: Date.now(), data } as SessionEvent + // Snapshot `data` into the log, NOT the caller's reference: the validation + // above proves it is JSON-serializable AT THIS MOMENT, but the caller still + // owns the object and could mutate it afterwards (before a persistence + // flush, or permanently in the in-memory history) — making `session.events` + // diverge from the value that passed validation, or reintroducing a + // non-serializable value. Cloning here keeps the log equal to what was + // validated. structuredClone is safe because serializability was just + // checked. The returned event carries the SAME snapshot, so a caller reading + // back `event.data` sees the logged value, not its own mutable input. + const event = { type, seq: this.log.length, time: Date.now(), data: structuredClone(data) } as SessionEvent this.log.push(event) this.onAppend?.(event) return event diff --git a/packages/session/src/json.ts b/packages/session/src/json.ts index ebb0c00c64..99fe80ea80 100644 --- a/packages/session/src/json.ts +++ b/packages/session/src/json.ts @@ -22,6 +22,14 @@ * convert lossily. Sparse arrays are rejected too: a hole serializes to `null`, * so `[1, , 3]` would not round-trip. Detects circular references (which would * throw) and reports them as non-serializable rather than propagating the throw. + * + * Scope — matches `JSON.stringify` exactly: only an object's OWN ENUMERABLE + * STRING-keyed properties are inspected (`Object.values`). Symbol-keyed and + * non-enumerable properties are NOT examined, because `JSON.stringify` likewise + * drops them — they never reach the durable form, so a non-serializable value + * hiding under a symbol/non-enumerable key cannot make the round-trip lossy. + * Getters are invoked during the check (again as `JSON.stringify` would), so the + * contract is for plain data records, not objects with side-effecting accessors. */ export function isJsonValue(value: unknown, seen: Set = new Set()): boolean { if (value === null) return true diff --git a/packages/session/tests/session.spec.ts b/packages/session/tests/session.spec.ts index 39d29872ee..50dc9d5907 100644 --- a/packages/session/tests/session.spec.ts +++ b/packages/session/tests/session.spec.ts @@ -139,6 +139,39 @@ describe('Session', () => { const session = new Session(SessionId('seed-ok'), goodSeed) expect(session.events).toHaveLength(3) }) + + it('snapshots the seed: mutating the original after construction does not affect session.events', () => { + const seed = [ + { type: 'turn/start' as const, seq: 0, time: 1, data: { turn: 1, trigger: { kind: 'message' as const, source: { kind: 'user' as const } } } }, + { type: 'user/message' as const, seq: 1, time: 2, data: { content: [{ type: 'text' as const, text: 'original' }], source: { kind: 'user' as const } } }, + { type: 'turn/end' as const, seq: 2, time: 3, data: { turn: 1, reason: { kind: 'completed' as const } } }, + ] as SessionEvent[] + const session = new Session(SessionId('seed-snapshot'), seed) + // Mutate the ORIGINAL seed objects after construction: a shared reference + // would let this rewrite the forked log (or reintroduce non-serializable + // data past validation). The snapshot must shield session.events. + const um = seed[1]! + ;(um.data as { content: { type: 'text'; text: string }[] }).content[0]!.text = 'HACKED' + ;(um.data as Record)['injected'] = 1n // would have failed validation + const logged = session.events[1]! + expect(logged.type === 'user/message' && (logged.data.content[0] as { text: string }).text).toBe('original') + expect((logged.data as Record)['injected']).toBeUndefined() + }) + + it('snapshots append data: mutating the passed object after append does not affect session.events', () => { + const session = new Session(SessionId('append-snapshot')) + const data = { content: [{ type: 'text' as const, text: 'original' }], source: { kind: 'user' as const } } + const event = session.append('user/message', data) + // Mutate the caller's object after append returns. A shared reference would + // make session.events diverge from the value that passed validation. + data.content[0]!.text = 'HACKED' + ;(data as Record)['injected'] = 1n + const logged = session.events[0]! + expect(logged.type === 'user/message' && (logged.data.content[0] as { text: string }).text).toBe('original') + expect((logged.data as Record)['injected']).toBeUndefined() + // The returned event carries the same snapshot, not the caller's input. + expect((event.data.content[0] as { text: string }).text).toBe('original') + }) })