Merge branch 'feat/rfc005-dev-invariants' into feat/rfc001-property-tests

packages/invariants/src/index.ts

@@ -66,19 +66,23 @@ interface SessionTrace {
 /**
  * Deep-freeze a value and everything reachable from it.
  *
- * Sound because this is only ever called top-down on event objects we just
- * appended: by the time a node is frozen, this same walk has already frozen
- * its descendants, so a frozen node implies frozen descendants — skipping it
- * is correct and avoids re-walking on HMR replay. (We never pass an
- * externally shallow-frozen object, which is the only input that would make
- * the early-return unsound.)
+ * Walks every object's own properties even when the object itself is already
+ * frozen: `Session.append()` accepts event data from arbitrary plugins/tools,
+ * so a caller can hand us a SHALLOW-frozen object whose descendants are still
+ * mutable. Skipping an already-frozen node (the obvious idempotence shortcut)
+ * would leave exactly the kind of mutable history ADR 0012 means to catch. A
+ * `WeakSet` of visited objects keeps it terminating on cycles and avoids
+ * re-walking shared subtrees / already-processed seed events.
  */
-function deepFreeze(value: unknown): void {
+function deepFreeze(value: unknown, seen: WeakSet<object> = new WeakSet()): void {
   if (value === null || typeof value !== 'object') return
-  if (Object.isFrozen(value)) return
+  if (seen.has(value)) return
+  seen.add(value)
+  // Freeze the node (no-op if a caller pre-froze it), then ALWAYS descend —
+  // a frozen container can still hold mutable children.
   Object.freeze(value)
   for (const key of Object.keys(value)) {
-    deepFreeze((value as Record<string, unknown>)[key])
+    deepFreeze((value as Record<string, unknown>)[key], seen)
   }
 }
 

packages/invariants/tests/invariants.spec.ts

@@ -222,16 +222,29 @@ describe('dev-freeze', () => {
     expect(Object.isFrozen(session.events[0])).toBe(true)
   })
 
-  it('is idempotent over already-frozen sub-structures', async () => {
+  it('freezes mutable descendants of a shallow-frozen event datum', async () => {
     const { ctx } = await setup()
     const session = ctx.sessions.create()
-    // Pre-freeze a block before appending; deepFreeze must short-circuit on it
-    // (the already-frozen guard) while still freezing the enclosing event.
-    const block = Object.freeze({ type: 'text' as const, text: 'pre-frozen' })
-    const event = session.append('user/message', { content: [block], source: { kind: 'user' } })
-    expect(Object.isFrozen(event)).toBe(true)
-    expect(Object.isFrozen(event.data.content)).toBe(true)
-    expect(Object.isFrozen(event.data.content[0])).toBe(true)
+    // A caller hands in a SHALLOW-frozen block whose nested array is still
+    // 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.
+    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()
+  })
+
+  it('terminates on a cyclic event datum (WeakSet guard)', async () => {
+    const { ctx } = await setup()
+    const session = ctx.sessions.create()
+    // A self-referential structure must not loop forever.
+    const cyclic: Record<string, unknown> = { type: 'text', text: 'x' }
+    cyclic['self'] = cyclic
+    expect(() => session.append('user/message', { content: [cyclic as never], source: { kind: 'user' } })).not.toThrow()
+    expect(Object.isFrozen(cyclic)).toBe(true)
   })
 })