mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(invariants): deepFreeze walks already-frozen objects' descendants
Session.append accepts event data from arbitrary plugins/tools, so a caller can pass a SHALLOW-frozen object with mutable descendants. The old Object.isFrozen early-return skipped such an object entirely, leaving its descendants mutable in the log — exactly the history mutation ADR 0012 means to catch. Now always descend, tracking visited objects in a WeakSet for cycle-termination and idempotence. Addresses PR review finding.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user