diff --git a/libs/ag-ui/src/lib/internal/apply-patch.spec.ts b/libs/ag-ui/src/lib/internal/apply-patch.spec.ts index 8cb30fca3..c06c2b9c5 100644 --- a/libs/ag-ui/src/lib/internal/apply-patch.spec.ts +++ b/libs/ag-ui/src/lib/internal/apply-patch.spec.ts @@ -158,3 +158,167 @@ describe('applyPatch — move/copy/test', () => { ).toThrowError(/'test' op failed/); }); }); + +describe('applyPatch — existing own paths', () => { + it.each([ + { op: 'replace', path: '/missing', value: 2 }, + { op: 'copy', from: '/missing', path: '/copy' }, + { op: 'move', from: '/missing', path: '/moved' }, + { op: 'test', path: '/missing', value: undefined }, + { op: 'copy', from: '/toString', path: '/copy' }, + { op: 'remove', path: '/constructor' }, + ] satisfies JsonPatchOp[])('rejects absent or inherited members: $op $path', (op) => { + expect(() => applyPatch({ a: 1 }, [op])).toThrow(); + }); + + it.each(['constructor', 'toString', '__proto__'])('never traverses inherited %s', (key) => { + const input = { nested: {} }; + for (const op of [ + { op: 'add', path: `/nested/${key}/field`, value: 1 }, + { op: 'replace', path: `/nested/${key}`, value: 1 }, + { op: 'remove', path: `/nested/${key}` }, + { op: 'copy', from: `/nested/${key}`, path: '/copy' }, + ] satisfies JsonPatchOp[]) { + expect(() => applyPatch(input, [op])).toThrow(); + } + }); + + it('distinguishes an own undefined value from an absent member', () => { + const input = Object.freeze({ present: undefined }); + expect(applyPatch(input, [{ op: 'test', path: '/present', value: undefined }])).toBe(input); + expect(applyPatch(input, [{ op: 'replace', path: '/present', value: 1 }])).toEqual({ present: 1 }); + expect(applyPatch(input, [{ op: 'remove', path: '/present' }])).toEqual({}); + const copied = applyPatch(input, [{ op: 'copy', from: '/present', path: '/copy' }]); + expect(Object.hasOwn(copied, 'copy')).toBe(true); + expect(copied).toEqual({ present: undefined, copy: undefined }); + expect(applyPatch(input, [{ op: 'move', from: '/present', path: '/moved' }])).toEqual({ moved: undefined }); + }); + + it.each(['missing', 'scalar', 'nil', 'present'])('rejects non-container or missing intermediate %s', (key) => { + const input = { scalar: 1, nil: null, present: undefined }; + expect(() => applyPatch(input, [{ op: 'add', path: `/${key}/child`, value: 1 }])).toThrow(); + }); +}); + +describe('applyPatch — array path bounds', () => { + it.each(['1', '2', '0'])('rejects missing array source or target index %s', (index) => { + // Index 0 is a hole; 1 is the end; 2 exceeds the end. + const input = { items: Object.freeze(new Array(1)) }; + for (const op of [ + { op: 'copy', from: `/items/${index}`, path: '/copy' }, + { op: 'move', from: `/items/${index}`, path: '/moved' }, + { op: 'test', path: `/items/${index}`, value: undefined }, + { op: 'replace', path: `/items/${index}`, value: 2 }, + { op: 'remove', path: `/items/${index}` }, + { op: 'add', path: `/items/${index}/child`, value: 2 }, + ] satisfies JsonPatchOp[]) { + expect(() => applyPatch(input, [op])).toThrow(); + } + expect(Object.hasOwn(input.items, 0)).toBe(false); + }); + + it.each(['-', '-1', '01', '1.0', '1e0'])('rejects non-element index %s for reads', (index) => { + expect(() => applyPatch([1, 2], [{ op: 'copy', from: `/${index}`, path: '/0' }])).toThrow(); + }); + + it('accepts own undefined array elements', () => { + expect(applyPatch([undefined], [{ op: 'test', path: '/0', value: undefined }])).toEqual([undefined]); + expect(applyPatch([undefined], [{ op: 'replace', path: '/0', value: 2 }])).toEqual([2]); + expect(applyPatch([undefined], [{ op: 'remove', path: '/0' }])).toEqual([]); + }); + + it.each(['2', '-'])('inserts at the array end using %s', (index) => { + expect(applyPatch([1, 2], [{ op: 'add', path: `/${index}`, value: 3 }])).toEqual([1, 2, 3]); + }); + + it('rejects add beyond the array end', () => { + expect(() => applyPatch([1, 2], [{ op: 'add', path: '/3', value: 3 }])).toThrow(); + }); + + it('preserves array move shifts and nested traversal', () => { + expect(applyPatch(['a', 'b', 'c'], [{ op: 'move', from: '/0', path: '/2' }])).toEqual(['b', 'c', 'a']); + const input = [{ value: 1 }]; + expect(applyPatch(input, [{ op: 'replace', path: '/0/value', value: 2 }])).toEqual([{ value: 2 }]); + expect(applyPatch(input, [{ op: 'copy', from: '/0', path: '/1' }])).toEqual([{ value: 1 }, { value: 1 }]); + }); +}); + +describe('applyPatch — safe own data keys', () => { + it.each(['__proto__', 'constructor', 'prototype'])('adds and operates on own %s data safely', (key) => { + const globalDescriptors = Object.getOwnPropertyDescriptors(Object.prototype); + const input = Object.freeze({}); + const added = applyPatch>(input, [{ op: 'add', path: `/${key}`, value: { value: 1 } }]); + expect(Object.hasOwn(added, key)).toBe(true); + expect(Object.getPrototypeOf(added)).toBe(Object.prototype); + expect(added[key]).toEqual({ value: 1 }); + expect(Object.getPrototypeOf(input)).toBe(Object.prototype); + expect(Object.hasOwn(input, key)).toBe(false); + const updated = applyPatch(added, [ + { op: 'test', path: `/${key}/value`, value: 1 }, + { op: 'replace', path: `/${key}/value`, value: 2 }, + { op: 'copy', from: `/${key}`, path: '/copied' }, + { op: 'remove', path: `/${key}/value` }, + ]); + expect(updated[key]).toEqual({}); + expect(updated['copied']).toEqual({ value: 2 }); + expect(Object.getPrototypeOf(updated)).toBe(Object.prototype); + expect(applyPatch(updated, [{ op: 'replace', path: `/${key}`, value: 3 }])[key]).toBe(3); + expect(Object.hasOwn(applyPatch(updated, [{ op: 'remove', path: `/${key}` }]), key)).toBe(false); + expect(Object.getOwnPropertyDescriptors(Object.prototype)).toEqual(globalDescriptors); + }); + + it('accepts explicit own special keys from null-prototype input', () => { + const input = Object.freeze(Object.assign(Object.create(null), JSON.parse('{"__proto__":{"value":1}}'))); + const out = applyPatch(input, [{ op: 'replace', path: '/__proto__/value', value: 2 }]); + expect(Object.getPrototypeOf(input)).toBe(null); + expect(Object.hasOwn(out, '__proto__')).toBe(true); + expect(Object.getPrototypeOf(out)).toBe(Object.prototype); + expect(out['__proto__']).toEqual({ value: 2 }); + expect(input['__proto__']).toEqual({ value: 1 }); + }); +}); + +describe('applyPatch — ownership and atomic failure', () => { + it.each([ + { op: 'replace', path: '/missing', value: 2 }, + { op: 'copy', from: '/missing', path: '/copy' }, + { op: 'move', from: '/missing', path: '/moved' }, + ] satisfies JsonPatchOp[])('leaves frozen input and inserted payload unchanged after failing $op', (invalid) => { + const items = Object.freeze([1, 2]); + const branch = Object.freeze({ value: 1 }); + const input = Object.freeze({ items, branch }); + const payload = Object.freeze({ nested: Object.freeze({ value: 1 }) }); + expect(() => applyPatch(input, [ + { op: 'replace', path: '/items/0', value: 9 }, + { op: 'add', path: '/added', value: payload }, + { op: 'replace', path: '/added/nested/value', value: 9 }, + invalid, + ])).toThrow(); + expect(input).toEqual({ items: [1, 2], branch: { value: 1 } }); + expect(input.items).toBe(items); + expect(input.branch).toBe(branch); + expect(payload.nested.value).toBe(1); + }); + + it('shares untouched branches and detaches inserted mutable values', () => { + const input = Object.freeze({ changed: Object.freeze({ value: 1 }), untouched: Object.freeze({ value: 2 }) }); + const payload = { nested: { value: 3 } }; + const out = applyPatch>(input, [ + { op: 'replace', path: '/changed/value', value: 4 }, + { op: 'add', path: '/added', value: payload }, + ]); + expect(out['untouched']).toBe(input.untouched); + expect(out['changed']).not.toBe(input.changed); + expect(out['added']).not.toBe(payload); + payload.nested.value = 5; + expect(out['added']).toEqual({ nested: { value: 3 } }); + }); + + it('preserves root replacement and rejects root removal', () => { + const value = { nested: { value: 2 } }; + const out = applyPatch({ old: 1 }, [{ op: 'replace', path: '', value }]); + expect(out).toEqual(value); + expect(out).not.toBe(value); + expect(() => applyPatch(out, [{ op: 'remove', path: '' }])).toThrow('Cannot remove root'); + }); +}); diff --git a/libs/ag-ui/src/lib/internal/apply-patch.ts b/libs/ag-ui/src/lib/internal/apply-patch.ts index ee337209e..bf56fe2a3 100644 --- a/libs/ag-ui/src/lib/internal/apply-patch.ts +++ b/libs/ag-ui/src/lib/internal/apply-patch.ts @@ -15,8 +15,9 @@ export interface JsonPatchOp { * new document. The input is not mutated. * * Operations apply in order; if any operation fails (invalid path, failed - * test, etc.) the whole patch throws — matching `fast-json-patch`'s - * `validate: false` behavior used by the reducer. + * test, etc.) the whole patch throws without changing the input. Paths must + * resolve through existing own members; only a final add may create a member + * or insert at the array end. Root add/replace is supported, root remove is not. */ export function applyPatch(target: T, ops: readonly JsonPatchOp[]): T { let current: unknown = target; @@ -28,8 +29,8 @@ export function applyPatch(target: T, ops: readonly JsonPatchOp[]): T { function applyOne(doc: unknown, op: JsonPatchOp): unknown { switch (op.op) { - case 'add': return setAt(doc, parsePointer(op.path), op.value, /*replaceArrayDash*/ true); - case 'replace': return setAt(doc, parsePointer(op.path), op.value, /*replaceArrayDash*/ false); + case 'add': return setAt(doc, parsePointer(op.path), op.value, /*add*/ true); + case 'replace': return setAt(doc, parsePointer(op.path), op.value, /*add*/ false); case 'remove': return removeAt(doc, parsePointer(op.path)); case 'move': { if (op.from == null) throw new Error("'move' op requires 'from'"); @@ -85,10 +86,11 @@ function getAt(doc: unknown, tokens: readonly string[]): unknown { function stepInto(node: unknown, token: string): unknown { if (Array.isArray(node)) { - const i = parseArrayIndex(token, node.length); + const i = existingArrayIndex(node, token); return node[i]; } if (node !== null && typeof node === 'object') { + if (!Object.hasOwn(node, token)) throw new Error(`Cannot read non-existent key "${token}"`); return (node as Record)[token]; } throw new Error(`Cannot traverse non-container at token "${token}"`); @@ -98,7 +100,7 @@ function setAt( doc: unknown, tokens: readonly string[], value: unknown, - allowArrayAppend: boolean, + add: boolean, ): unknown { if (tokens.length === 0) { // Replace root. @@ -106,36 +108,35 @@ function setAt( } const [head, ...rest] = tokens; if (Array.isArray(doc)) { + const i = add && rest.length === 0 + ? head === '-' ? doc.length : parseArrayIndex(head!, doc.length) + : existingArrayIndex(doc, head!); const arr = doc.slice(); - const i = head === '-' && allowArrayAppend ? arr.length : parseArrayIndex(head!, arr.length + (allowArrayAppend ? 1 : 0)); if (rest.length === 0) { - if (allowArrayAppend) { + if (add) { // RFC-6902 add: insert at index, shifting elements right arr.splice(i, 0, structuredCloneSafe(value)); } else { // replace: overwrite at index - if (i >= arr.length) throw new Error(`Cannot replace beyond array length at "/${tokens.join('/')}"`); arr[i] = structuredCloneSafe(value); } } else { - if (i >= arr.length) throw new Error(`Cannot descend into non-existent array index ${i}`); - arr[i] = setAt(arr[i], rest, value, allowArrayAppend); + arr[i] = setAt(arr[i], rest, value, add); } return arr; } if (doc === null || typeof doc !== 'object') { throw new Error(`Cannot descend into non-container at "/${tokens.join('/')}"`); } - const obj = { ...(doc as Record) }; + const obj = doc as Record; + if ((!add || rest.length > 0) && !Object.hasOwn(obj, head!)) { + throw new Error(`Cannot update missing path "/${tokens.join('/')}"`); + } if (rest.length === 0) { - obj[head!] = structuredCloneSafe(value); - } else { - if (!(head! in obj)) { - throw new Error(`Cannot descend into missing path "/${tokens.join('/')}"`); - } - obj[head!] = setAt(obj[head!], rest, value, allowArrayAppend); + // Computed properties create own data even for the key "__proto__". + return { ...obj, [head!]: structuredCloneSafe(value) }; } - return obj; + return { ...obj, [head!]: setAt(obj[head!], rest, value, add) }; } function removeAt(doc: unknown, tokens: readonly string[]): unknown { @@ -144,9 +145,8 @@ function removeAt(doc: unknown, tokens: readonly string[]): unknown { } const [head, ...rest] = tokens; if (Array.isArray(doc)) { + const i = existingArrayIndex(doc, head!); const arr = doc.slice(); - const i = parseArrayIndex(head!, arr.length); - if (i >= arr.length) throw new Error(`Cannot remove non-existent array index ${i}`); if (rest.length === 0) { arr.splice(i, 1); } else { @@ -157,15 +157,20 @@ function removeAt(doc: unknown, tokens: readonly string[]): unknown { if (doc === null || typeof doc !== 'object') { throw new Error(`Cannot remove from non-container at token "${head}"`); } - const obj = { ...(doc as Record) }; + if (!Object.hasOwn(doc, head!)) throw new Error(`Cannot remove non-existent key "${head}"`); + const obj = doc as Record; if (rest.length === 0) { - if (!(head! in obj)) throw new Error(`Cannot remove non-existent key "${head}"`); - delete obj[head!]; - } else { - if (!(head! in obj)) throw new Error(`Cannot descend into missing path "${head}"`); - obj[head!] = removeAt(obj[head!], rest); + const out = { ...obj }; + delete out[head!]; + return out; } - return obj; + return { ...obj, [head!]: removeAt(obj[head!], rest) }; +} + +function existingArrayIndex(array: readonly unknown[], token: string): number { + const i = parseArrayIndex(token, array.length - 1); + if (!Object.hasOwn(array, i)) throw new Error(`Cannot access non-existent array index ${i}`); + return i; } function parseArrayIndex(token: string, lengthBound: number): number { diff --git a/libs/ag-ui/src/lib/reducer.spec.ts b/libs/ag-ui/src/lib/reducer.spec.ts index 0a665e2da..e589a6928 100644 --- a/libs/ag-ui/src/lib/reducer.spec.ts +++ b/libs/ag-ui/src/lib/reducer.spec.ts @@ -494,6 +494,39 @@ describe('reduceEvent', () => { expect(store.state()).toEqual({ a: 2, b: 3 }); }); + it.each([ + { op: 'replace', path: '/missing', value: 2 }, + { op: 'copy', from: '/missing', path: '/copy' }, + ])('STATE_DELTA rejects a failed $op batch before publishing state or citations', (invalid) => { + const store = makeStore(); + store.messages.set([{ id: 'm1', role: 'assistant', content: 'answer', delivery: staticDelivery('m1') }]); + reduceEvent({ type: 'STATE_SNAPSHOT', snapshot: { + citations: { m1: [{ id: 'prior', title: 'Prior citation' }] }, + } } as never, store); + const state = store.state(); + const messages = store.messages(); + const citations = messages[0].citations; + expect(() => reduceEvent({ type: 'STATE_DELTA', delta: [ + { op: 'replace', path: '/citations/m1', value: [{ id: 'next', title: 'Unpublished citation' }] }, + invalid, + ] } as never, store)).toThrow(); + expect(store.state()).toBe(state); + expect(store.messages()).toBe(messages); + expect(store.messages()[0].citations).toBe(citations); + expect(citations?.[0].id).toBe('prior'); + expect(state['citations']).toEqual({ m1: [{ id: 'prior', title: 'Prior citation' }] }); + }); + + it('STATE_DELTA publishes an own __proto__ data member with a safe prototype', () => { + const store = makeStore(); + reduceEvent({ type: 'STATE_DELTA', delta: [ + { op: 'add', path: '/__proto__', value: { label: 'data' } }, + ] } as never, store); + expect(Object.hasOwn(store.state(), '__proto__')).toBe(true); + expect(store.state()['__proto__']).toEqual({ label: 'data' }); + expect(Object.getPrototypeOf(store.state())).toBe(Object.prototype); + }); + it('MESSAGES_SNAPSHOT replaces messages wholesale', () => { const store = makeStore(); store.deliveryRun = null; @@ -910,6 +943,44 @@ describe('ACTIVITY events (F5 subagent activities)', () => { expect(store.activities().size).toBe(0); }); + it.each([ + { op: 'replace', path: '/missing', value: 2 }, + { op: 'copy', from: '/missing', path: '/copy' }, + ])('ACTIVITY_DELTA retains the original content and membership after a failed $op batch', (invalid) => { + const store = makeStore(); + const content = Object.freeze({ text: 'prior', items: Object.freeze([1, 2]) }); + reduceEvent({ type: 'ACTIVITY_SNAPSHOT', messageId: 'tc-1', activityType: 'subagent', content } as never, store); + const activities = store.activities(); + const entry = activities.get('tc-1'); + const warning = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + expect(() => reduceEvent({ type: 'ACTIVITY_DELTA', messageId: 'tc-1', patch: [ + { op: 'replace', path: '/text', value: 'unpublished' }, + { op: 'replace', path: '/items/0', value: 9 }, + invalid, + ] } as never, store)).not.toThrow(); + expect(entry?.content()).toBe(content); + expect(entry?.content()).toEqual({ text: 'prior', items: [1, 2] }); + expect(store.activities()).toBe(activities); + expect(store.activities().get('tc-1')).toBe(entry); + expect(warning).toHaveBeenCalledExactlyOnceWith('[ag-ui] dropping malformed ACTIVITY_DELTA patch', expect.any(Error)); + } finally { + warning.mockRestore(); + } + }); + + it('ACTIVITY_DELTA publishes an own __proto__ data member with a safe prototype', () => { + const store = makeStore(); + reduceEvent({ type: 'ACTIVITY_SNAPSHOT', messageId: 'tc-1', activityType: 'subagent', content: {} } as never, store); + reduceEvent({ type: 'ACTIVITY_DELTA', messageId: 'tc-1', patch: [ + { op: 'add', path: '/__proto__', value: { label: 'data' } }, + ] } as never, store); + const content = store.activities().get('tc-1')?.content() ?? {}; + expect(Object.hasOwn(content, '__proto__')).toBe(true); + expect(content['__proto__']).toEqual({ label: 'data' }); + expect(Object.getPrototypeOf(content)).toBe(Object.prototype); + }); + it('two concurrent subagents are keyed independently', () => { const store = makeStore(); reduceEvent({ type: 'ACTIVITY_SNAPSHOT', messageId: 'a', activityType: 'subagent', content: { text: '' } } as any, store); diff --git a/scripts/react-parity/baseline.json b/scripts/react-parity/baseline.json index 49aa81384..cc9250174 100644 --- a/scripts/react-parity/baseline.json +++ b/scripts/react-parity/baseline.json @@ -1,11 +1,11 @@ { "schemaVersion": 1, - "baselineHead": "3e7da956bd7e12927412a7a533045f69fcf5c27b", + "baselineHead": "9a87f53641064041cae89e1e83fd2da071440e5b", "sourceState": { - "modified": [], - "untracked": [ - "libs/ag-ui/src/runtime/transcript.ts" - ] + "modified": [ + "libs/ag-ui/src/lib/internal/apply-patch.ts" + ], + "untracked": [] }, "scope": { "libraries": [ @@ -10536,7 +10536,7 @@ "id": "source:libs/ag-ui/src/lib/internal/apply-patch.ts", "kind": "source", "path": "libs/ag-ui/src/lib/internal/apply-patch.ts", - "sha256": "6a348d876814bc7f1ec10c698bcbca11eb5e224866f0df55546f4e3481d2af1f" + "sha256": "7d241f7d6cfbffbb4a562c5e73ff660c628e11e34aa59229dba771dc14c68427" }, { "id": "source:libs/ag-ui/src/lib/interrupt-persistence.ts",