diff --git a/docs/public/api.md b/docs/public/api.md index 63675ccf..dace159e 100644 --- a/docs/public/api.md +++ b/docs/public/api.md @@ -309,7 +309,7 @@ doc.schema.accepts("/lists/0/cards/-", candidateCard, "insert"); 큰 문서의 hot path는 document facade인 `doc.patch`, `doc.commit`, `doc.canPatch`를 기준으로 둡니다. 공개 `applyPatch`는 외부 JSON 경계입니다. -Document 내부 state는 신뢰된 document state입니다. schema가 구조만 가진 Zod schema이고 edit가 independent non-root `replace` 또는 array edit에 해당하면 document path는 더 좁은 검증 경로를 쓸 수 있습니다. Refinement, transform, check가 있는 schema는 전체 루트 schema 검증으로 돌아갑니다. +Document 내부 state는 신뢰된 document state입니다. schema가 구조만 가진 Zod schema이고 edit가 non-root `replace` batch(독립 경로와 순차 ancestor/descendant overlap 포함) 또는 지원되는 array edit에 해당하면 document path는 더 좁은 검증 경로를 쓸 수 있습니다. Leading `test` assertion이 붙은 guarded batch도 assertion 확인 뒤 같은 mutation 경로를 사용합니다. Overlapping `replace`의 history inverse도 operation별 이전 값과 undo 순서를 그대로 유지하는 copy-on-write 순회 구현을 사용합니다. Refinement, transform, check가 있는 schema는 전체 루트 schema 검증으로 돌아갑니다. ## 트리 편집 cookbook diff --git a/docs/public/extensions.md b/docs/public/extensions.md index b9ec8cef..75225ba3 100644 --- a/docs/public/extensions.md +++ b/docs/public/extensions.md @@ -251,6 +251,13 @@ if (proposedChanges.canAccept("rename-title").ok) { } ``` +`canAccept`는 현재 상태를 설명하는 capability probe다. 실제 `accept`는 저장된 guard를 +RFC 6902 `test`로 바꾸어 proposed operations 앞에 놓고 하나의 atomic patch로 실행한다. +따라서 schema validation 중 document revision이 바뀌어도 최신 상태에서 guard부터 다시 +확인한다. Document state가 실제 publish되면 생성된 `test` operation도 `lastPatch`, +subscriber, history forward에 나타난다. 같은 proposal id는 `accept` 실행 동안 예약되어 +document subscriber의 재진입이 proposal을 교체하거나 제거하지 못한다. + ## comments ```ts diff --git a/docs/standard/foundation-gate.md b/docs/standard/foundation-gate.md index a3d2227d..d56efb3e 100644 --- a/docs/standard/foundation-gate.md +++ b/docs/standard/foundation-gate.md @@ -108,3 +108,11 @@ replace가 있어도 touched container를 commit당 한 번만 복제한다. 최 경로와 동일하게 유지해야 한다. Plain structural schema의 overlapping replace는 각 value가 target schema에 허용됨을 빠르게 증명할 수 있을 때만 이 bulk 경로로 승격하고, 그 외 배치는 기존 validation/error-order 경로로 되돌린다. + +History inverse 계산도 같은 private sequential-replace COW seam을 재사용한다. 각 write +직전 값을 캡처해 기존 operation별 inverse와 undo 순서를 유지하며, inverse를 상위 path +하나로 압축하거나 public prepared/history 계약을 추가하지 않는다. + +Guarded change가 leading RFC 6902 `test` assertion을 붙여도 plain structural schema에서는 +assertion을 먼저 평가한 뒤 mutation suffix와 inverse를 같은 fast path에 위임한다. Assertion +실패나 지원하지 않는 suffix는 기존 full validation/error-order 경로가 정본이다. diff --git a/docs/standard/json-document-spec.md b/docs/standard/json-document-spec.md index 08040959..020f691a 100644 --- a/docs/standard/json-document-spec.md +++ b/docs/standard/json-document-spec.md @@ -385,7 +385,7 @@ History metadata는 앱이나 adapter가 document change에 붙이는 주석이 큰 문서의 hot path는 document facade인 `doc.patch`, `doc.commit`, `doc.canPatch`에 둔다. 공개 `applyPatch`는 외부 JSON 경계라서 입력 state 전체의 JSON 안전성을 확인한다. `applyPatchToTrustedState`는 호출자가 이미 state JSON 경계를 소유할 때 쓰는 pure core opt-in이다. Operation value와 schema validation은 여전히 실행되며 구조만 가진 schema는 document facade와 같은 trusted fast path를 사용할 수 있다. -빠른 document path는 신뢰된 document state와 구조만 가진 Zod schema에서만 적용된다. 대상 schema는 refinement, transform, check가 없는 object, array, record, scalar validator다. 지원 edit는 independent non-root `replace`, array `add`/`remove`/`copy`/`move`, same-array `add`/`remove` batch다. `refine`, `superRefine`, transform, check가 있으면 의도적으로 전체 루트 schema 검증으로 돌아간다. +빠른 document path는 신뢰된 document state와 구조만 가진 Zod schema에서만 적용된다. 대상 schema는 refinement, transform, check가 없는 object, array, record, scalar validator다. 지원 edit는 non-root `replace` batch(독립 경로와 순차 ancestor/descendant overlap 포함), array `add`/`remove`/`copy`/`move`, same-array `add`/`remove` batch다. Leading `test` assertion 뒤에 이 edit들이 오는 guarded batch도 assertion을 먼저 확인한 뒤 같은 mutation fast path를 사용한다. Overlapping `replace`의 history inverse도 operation별 이전 값과 역순을 유지하면서 같은 private copy-on-write 순회 구현을 사용한다. `refine`, `superRefine`, transform, check가 있으면 의도적으로 전체 루트 schema 검증으로 돌아간다. ```sh npm run perf:core diff --git a/packages/json-document/src/domain/schema/validation/patch.ts b/packages/json-document/src/domain/schema/validation/patch.ts index b698b012..33614ce9 100644 --- a/packages/json-document/src/domain/schema/validation/patch.ts +++ b/packages/json-document/src/domain/schema/validation/patch.ts @@ -71,6 +71,9 @@ export function applyPatchWithLocalSchemaValidation( if (!isPlainStructuralSchema(schema)) return null; const valuesTrusted = options.valuesTrusted === true; + const leadingTests = applyLeadingTestsWithLocalSchemaValidation(schema, state, ops, valuesTrusted); + if (leadingTests) return leadingTests; + const singleReplace = applySingleReplacePatchWithLocalSchemaValidation(schema, state, ops, valuesTrusted); if (singleReplace) return singleReplace; const sameArrayFieldReplace = applySameArrayFieldReplacePatchWithLocalSchemaValidation(schema, state, ops, valuesTrusted); @@ -98,6 +101,37 @@ export function applyPatchWithLocalSchemaValidation( return applySequentialPatchWithLocalSchemaValidation(schema, state, ops, valuesTrusted); } +function applyLeadingTestsWithLocalSchemaValidation( + schema: S, + state: z.output, + operations: ReadonlyArray, + valuesTrusted: boolean, +): ApplyResult | null { + let testCount = 0; + while (testCount < operations.length && operations[testCount]?.op === "test") { + testCount += 1; + } + if (testCount === 0 || testCount === operations.length) return null; + + const tests = operations.slice(0, testCount); + const asserted = valuesTrusted + ? applyAcceptedPatch(state, tests) + : applyTrustedPatch(state, tests); + if (!asserted.result.ok) return failedLocalSchemaValidation(state, asserted.result); + + const mutation = applyPatchWithLocalSchemaValidation( + schema, + state, + operations.slice(testCount), + { valuesTrusted }, + ); + if (mutation === null || !mutation.result.ok) return null; + return okLocalSchemaValidation(mutation.state, [ + ...asserted.applied, + ...mutation.applied, + ]); +} + // schema-인지 trusted-state patch 적용: 먼저 local schema validation 경로를 시도하고, // 다루지 못하는 형태면 foundation 의 trusted-state core 로 위임한다. export function applyPatchToTrustedState( diff --git a/packages/json-document/src/foundation/patch/fast/apply.ts b/packages/json-document/src/foundation/patch/fast/apply.ts index 324c4d07..65048b52 100644 --- a/packages/json-document/src/foundation/patch/fast/apply.ts +++ b/packages/json-document/src/foundation/patch/fast/apply.ts @@ -7,6 +7,7 @@ import { removedRootKeysMatchSuffix, } from "../object.js"; import { validateOperationShape } from "../apply.js"; +import { applySequentialReplacePatch } from "../sequentialReplace.js"; import { applyAppendOnlyAddPatch, applySameArrayStructuralPatch, @@ -14,7 +15,6 @@ import { } from "./array.js"; import { applyIndependentReplacePatch, - applySequentialReplacePatch, applySameArrayElementReplacePatch, applySameArrayFieldReplacePatch, applySameArrayNestedReplacePatch, diff --git a/packages/json-document/src/foundation/patch/fast/replace.ts b/packages/json-document/src/foundation/patch/fast/replace.ts index e53db188..0a1bd6f9 100644 --- a/packages/json-document/src/foundation/patch/fast/replace.ts +++ b/packages/json-document/src/foundation/patch/fast/replace.ts @@ -6,7 +6,6 @@ import { arrayRemoveLocation, arrayFieldText, indexDirection, - numericSegment, parseArrayFieldPath, parseFirstArrayNestedPath, parseKnownArrayNestedIndex, @@ -277,133 +276,6 @@ export function applyIndependentReplacePatch( return { handled: true, state: applyReplaceTree(state, buildReplaceTree(items)), applied: items.map((item) => item.op) }; } -interface SequentialReplaceOperation { - op: Extract; - segments: string[]; -} - -/** - * Applies an overlapping replace batch through a private copy-on-write draft. - * - * Earlier replace strategies cover uniform and independent paths. This fallback - * preserves RFC 6902 operation order for ancestor/descendant and repeated paths - * while cloning each container at most once for the lifetime of the batch. - * The draft is never published on failure, so the caller's state and values stay - * immutable. - */ -export function applySequentialReplacePatch( - state: unknown, - ops: ReadonlyArray, - valuesTrusted = false, -): FastPatchResult { - if (ops.length < 2) return { handled: false }; - - const prepared = new Array(ops.length); - for (let index = 0; index < ops.length; index += 1) { - if (!(index in ops)) return { handled: false }; - const op = ops[index]!; - if (validateOperationShape(op) !== null || op.op !== "replace" || op.path === "") { - return { handled: false }; - } - if (!valuesTrusted && jsonSerializableError(op.value) !== null) return { handled: false }; - const parsed = parseSafe(op.path); - if (!("ok" in parsed)) return { handled: false }; - prepared[index] = { op, segments: parsed.segs }; - } - - const draftContainers = new WeakSet(); - let draft = state; - for (const operation of prepared) { - const replaced = replaceDraftValue(draft, operation.segments, operation.op.value, draftContainers); - if (replaced === null) return { handled: false }; - draft = replaced; - } - - return { - handled: true, - state: draft, - applied: prepared.map((operation) => operation.op), - }; -} - -function replaceDraftValue( - state: unknown, - segments: ReadonlyArray, - value: unknown, - draftContainers: WeakSet, -): unknown | null { - if (segments.length === 0) return null; - const root = ensureDraftContainer(state, draftContainers); - if (root === null) return null; - - let current = root; - for (let index = 0; index < segments.length - 1; index += 1) { - const segment = segments[index]!; - const child = readDraftChild(current, segment); - if (!child.ok) return null; - const childDraft = ensureDraftContainer(child.value, draftContainers); - if (childDraft === null) return null; - if (childDraft !== child.value) writeDraftChild(current, child.key, childDraft); - current = childDraft; - } - - const target = readDraftChild(current, segments[segments.length - 1]!); - if (!target.ok) return null; - writeDraftChild(current, target.key, value); - return root; -} - -type DraftContainer = unknown[] | Record; - -function ensureDraftContainer( - value: unknown, - draftContainers: WeakSet, -): DraftContainer | null { - if (value === null || typeof value !== "object") return null; - if (draftContainers.has(value)) return value as DraftContainer; - const draft: DraftContainer = Array.isArray(value) - ? value.slice() - : { ...(value as Record) }; - draftContainers.add(draft); - return draft; -} - -function readDraftChild( - container: DraftContainer, - segment: string, -): { ok: true; key: number | string; value: unknown } | { ok: false } { - if (Array.isArray(container)) { - const index = numericSegment(segment); - return index === null || index >= container.length - ? { ok: false } - : { ok: true, key: index, value: container[index] }; - } - if (!objectHasOwn.call(container, segment)) return { ok: false }; - return { ok: true, key: segment, value: container[segment] }; -} - -function writeDraftChild( - container: DraftContainer, - key: number | string, - value: unknown, -): void { - if (Array.isArray(container)) { - container[key as number] = value; - return; - } - const property = key as string; - if (property === "__proto__") { - Object.defineProperty(container, property, { - value, - enumerable: true, - configurable: true, - writable: true, - }); - return; - } - container[property] = value; -} - function buildReplaceTree(items: ReadonlyArray<{ segments: string[]; value: unknown }>): ReplaceTree { const root: ReplaceTree = { children: new Map() }; for (const item of items) { diff --git a/packages/json-document/src/foundation/patch/inverse.ts b/packages/json-document/src/foundation/patch/inverse.ts index bb9204a6..d5b35a08 100644 --- a/packages/json-document/src/foundation/patch/inverse.ts +++ b/packages/json-document/src/foundation/patch/inverse.ts @@ -6,6 +6,7 @@ import { cloneTrustedPlainJson } from "../json/trustedClone.js"; import { applyOpRaw } from "./apply.js"; import { getValueAt, resolveAppendPath } from "./container.js"; import { objectHasOwn } from "./object.js"; +import { computeSequentialReplaceInverses } from "./sequentialReplace.js"; import { arrayFieldText, arrayLocation, @@ -84,6 +85,8 @@ export function computeInverses( if (arrayNestedReplace) return arrayNestedReplace; const replaceOnly = computeIndependentReplaceInverses(state, ops); if (replaceOnly) return replaceOnly; + const sequentialReplace = computeSequentialReplaceInverses(state, ops); + if (sequentialReplace !== null) return { ok: true, inverses: sequentialReplace }; const arrayOnly = computeSameArrayStructuralInverses(state, ops); if (arrayOnly) return arrayOnly; diff --git a/packages/json-document/src/foundation/patch/sequentialReplace.ts b/packages/json-document/src/foundation/patch/sequentialReplace.ts new file mode 100644 index 00000000..9bf2cdfe --- /dev/null +++ b/packages/json-document/src/foundation/patch/sequentialReplace.ts @@ -0,0 +1,202 @@ +import { jsonSerializableError } from "../json/serializable.js"; +import { applyOpRaw, validateOperationShape } from "./apply.js"; +import { parseSafe } from "./container.js"; +import type { FastPatchResult, JSONPatchOperation } from "./contract.js"; +import { objectHasOwn } from "./object.js"; +import { numericSegment } from "./path.js"; + +type ReplaceOperation = Extract; + +interface PreparedSequentialReplace { + operation: ReplaceOperation; + segments: string[]; +} + +interface SequentialReplaceRun { + state: unknown; + applied: ReplaceOperation[] | null; + inverses: JSONPatchOperation[] | null; +} + +/** + * Applies a multi-operation, non-root replace batch through one private COW + * draft. Unsupported or invalid batches decline so the reference executor + * remains the owner of observable error details. + */ +export function applySequentialReplacePatch( + state: unknown, + operations: ReadonlyArray, + valuesTrusted = false, +): FastPatchResult { + const run = runSequentialReplaceBatch(state, operations, valuesTrusted, false); + return run === null || run.applied === null + ? { handled: false } + : { handled: true, state: run.state, applied: run.applied }; +} + +/** + * Captures the value immediately before each sequential write and returns + * inverse operations in undo order. A null result delegates to the generic + * inverse path, preserving its existing success and failure semantics. + */ +export function computeSequentialReplaceInverses( + state: unknown, + operations: ReadonlyArray, +): JSONPatchOperation[] | null { + let firstReplace = 0; + while ( + firstReplace < operations.length + && firstReplace in operations + && operations[firstReplace]!.op === "test" + ) { + const asserted = applyOpRaw(state, operations[firstReplace]!); + if ("error" in asserted) return null; + state = asserted.state; + firstReplace += 1; + } + if (firstReplace === operations.length) return null; + + const replaceOperations = firstReplace === 0 + ? operations + : operations.slice(firstReplace); + const run = runSequentialReplaceBatch(state, replaceOperations, false, true); + return run?.inverses ?? null; +} + +function runSequentialReplaceBatch( + state: unknown, + operations: ReadonlyArray, + valuesTrusted: boolean, + captureInverses: boolean, +): SequentialReplaceRun | null { + if (operations.length < 2) return null; + + const prepared = new Array(operations.length); + const applied = captureInverses + ? null + : new Array(operations.length); + for (let index = 0; index < operations.length; index += 1) { + if (!(index in operations)) return null; + const operation = operations[index]!; + if ( + validateOperationShape(operation) !== null + || operation.op !== "replace" + || operation.path === "" + ) { + return null; + } + if (!valuesTrusted && jsonSerializableError(operation.value) !== null) return null; + const parsed = parseSafe(operation.path); + if (!("ok" in parsed)) return null; + prepared[index] = { operation, segments: parsed.segs }; + if (applied !== null) applied[index] = operation; + } + + const inverses = captureInverses + ? new Array(operations.length) + : null; + const draftContainers = new WeakSet(); + let draft = state; + for (let index = 0; index < prepared.length; index += 1) { + const item = prepared[index]!; + const replaced = replaceDraftValue( + draft, + item.segments, + item.operation, + draftContainers, + inverses, + operations.length - index - 1, + ); + if (replaced === null) return null; + draft = replaced; + } + + return { state: draft, applied, inverses }; +} + +function replaceDraftValue( + state: unknown, + segments: ReadonlyArray, + operation: ReplaceOperation, + draftContainers: WeakSet, + inverses: JSONPatchOperation[] | null, + inverseIndex: number, +): unknown | null { + if (segments.length === 0) return null; + const root = ensureDraftContainer(state, draftContainers); + if (root === null) return null; + + let current = root; + for (let index = 0; index < segments.length - 1; index += 1) { + const segment = segments[index]!; + const child = readDraftChild(current, segment); + if (!child.ok) return null; + const childDraft = ensureDraftContainer(child.value, draftContainers); + if (childDraft === null) return null; + if (childDraft !== child.value) writeDraftChild(current, child.key, childDraft); + current = childDraft; + } + + const target = readDraftChild(current, segments[segments.length - 1]!); + if (!target.ok) return null; + if (inverses !== null) { + inverses[inverseIndex] = { + op: "replace", + path: operation.path, + value: target.value, + }; + } + writeDraftChild(current, target.key, operation.value); + return root; +} + +type DraftContainer = unknown[] | Record; + +function ensureDraftContainer( + value: unknown, + draftContainers: WeakSet, +): DraftContainer | null { + if (value === null || typeof value !== "object") return null; + if (draftContainers.has(value)) return value as DraftContainer; + const draft: DraftContainer = Array.isArray(value) + ? value.slice() + : { ...(value as Record) }; + draftContainers.add(draft); + return draft; +} + +function readDraftChild( + container: DraftContainer, + segment: string, +): { ok: true; key: number | string; value: unknown } | { ok: false } { + if (Array.isArray(container)) { + const index = numericSegment(segment); + return index === null || index >= container.length + ? { ok: false } + : { ok: true, key: index, value: container[index] }; + } + if (!objectHasOwn.call(container, segment)) return { ok: false }; + return { ok: true, key: segment, value: container[segment] }; +} + +function writeDraftChild( + container: DraftContainer, + key: number | string, + value: unknown, +): void { + if (Array.isArray(container)) { + container[key as number] = value; + return; + } + const property = key as string; + if (property === "__proto__") { + Object.defineProperty(container, property, { + value, + enumerable: true, + configurable: true, + writable: true, + }); + return; + } + container[property] = value; +} diff --git a/packages/json-document/tests/regression/overlapping-replace-batch.test.ts b/packages/json-document/tests/regression/overlapping-replace-batch.test.ts index 5bea99fe..b8461a81 100644 --- a/packages/json-document/tests/regression/overlapping-replace-batch.test.ts +++ b/packages/json-document/tests/regression/overlapping-replace-batch.test.ts @@ -44,12 +44,13 @@ describe("overlapping replace batch", () => { { op: "replace", path: "/items/0/meta/rank", value: 11 }, { op: "replace", path: "/items/0/meta/tag", value: "final" }, ]; + const inverse: JSONPatchOperation[] = [ + { op: "replace", path: "/items/0/meta/tag", value: "batch" }, + { op: "replace", path: "/items/0/meta/rank", value: 10 }, + { op: "replace", path: "/items/0/meta", value: { tag: "initial", rank: 0 } }, + ]; const observed: ReadonlyArray[] = []; - const unsubscribe = doc.subscribe((patch) => { - observed.push(patch); - expect(doc.value.items[0]?.meta).toEqual({ tag: "final", rank: 11 }); - expect(doc.lastPatch).toEqual(operations); - }); + const unsubscribe = doc.subscribe((patch) => { observed.push(patch); }); expect(doc.patch(operations)).toEqual({ ok: true }); @@ -63,21 +64,52 @@ describe("overlapping replace batch", () => { expect(doc.value.items[1]).toBe(before.items[1]); expect(doc.value.settings).toBe(before.settings); expect(doc.history.undoDepth).toBe(1); + expect(doc.history.redoDepth).toBe(0); - unsubscribe(); expect(doc.undo()).toEqual({ ok: true }); expect(doc.value).toEqual(initial); + expect(doc.lastPatch).toEqual(inverse); + expect(doc.history.undoDepth).toBe(0); + expect(doc.history.redoDepth).toBe(1); expect(doc.redo()).toEqual({ ok: true }); expect(doc.value.items[0]?.meta).toEqual({ tag: "final", rank: 11 }); + expect(doc.lastPatch).toEqual(operations); + expect(doc.history.undoDepth).toBe(1); + expect(doc.history.redoDepth).toBe(0); + expect(observed).toEqual([operations, inverse, operations]); + expect(replacement).toEqual({ tag: "batch", rank: 10 }); + unsubscribe(); }); test("keeps ancestor and descendant operation order observable", () => { const descendantThenAncestor = createDocument(); - expect(descendantThenAncestor.patch([ + const descendantFirst: JSONPatchOperation[] = [ { op: "replace", path: "/items/0/meta/rank", value: 20 }, { op: "replace", path: "/items/0/meta", value: { tag: "ancestor", rank: 21 } }, - ])).toEqual({ ok: true }); + ]; + const descendantFirstInverse: JSONPatchOperation[] = [ + { op: "replace", path: "/items/0/meta", value: { tag: "initial", rank: 20 } }, + { op: "replace", path: "/items/0/meta/rank", value: 0 }, + ]; + const observed: ReadonlyArray[] = []; + const unsubscribe = descendantThenAncestor.subscribe((patch) => { observed.push(patch); }); + expect(descendantThenAncestor.patch(descendantFirst)).toEqual({ ok: true }); expect(descendantThenAncestor.value.items[0]?.meta).toEqual({ tag: "ancestor", rank: 21 }); + expect(descendantThenAncestor.undo()).toEqual({ ok: true }); + expect(descendantThenAncestor.value).toEqual(initial); + expect(descendantThenAncestor.lastPatch).toEqual(descendantFirstInverse); + expect(descendantThenAncestor.redo()).toEqual({ ok: true }); + expect(descendantThenAncestor.lastPatch).toEqual(descendantFirst); + expect(descendantThenAncestor.undo()).toEqual({ ok: true }); + expect(descendantThenAncestor.value).toEqual(initial); + expect(descendantThenAncestor.lastPatch).toEqual(descendantFirstInverse); + expect(observed).toEqual([ + descendantFirst, + descendantFirstInverse, + descendantFirst, + descendantFirstInverse, + ]); + unsubscribe(); const ancestorThenDescendant = createDocument(); expect(ancestorThenDescendant.patch([ @@ -87,6 +119,82 @@ describe("overlapping replace batch", () => { expect(ancestorThenDescendant.value.items[0]?.meta).toEqual({ tag: "ancestor", rank: 22 }); }); + test("keeps leading test assertions in forward history without adding inverses", () => { + const doc = createDocument(); + const operations: JSONPatchOperation[] = [ + { op: "test", path: "/items/0/meta", value: { tag: "initial", rank: 0 } }, + { op: "test", path: "/items/0/meta/rank", value: 0 }, + { op: "replace", path: "/items/0/meta", value: { tag: "guarded", rank: 30 } }, + { op: "replace", path: "/items/0/meta/rank", value: 31 }, + ]; + + expect(doc.patch(operations)).toEqual({ ok: true }); + expect(doc.value.items[0]?.meta).toEqual({ tag: "guarded", rank: 31 }); + expect(doc.lastPatch).toEqual(operations); + expect(doc.undo()).toEqual({ ok: true }); + expect(doc.value).toEqual(initial); + expect(doc.lastPatch).toEqual([ + { op: "replace", path: "/items/0/meta/rank", value: 30 }, + { op: "replace", path: "/items/0/meta", value: { tag: "initial", rank: 0 } }, + ]); + expect(doc.redo()).toEqual({ ok: true }); + expect(doc.value.items[0]?.meta).toEqual({ tag: "guarded", rank: 31 }); + expect(doc.lastPatch).toEqual(operations); + }); + + test("preserves stable failure fields and atomicity for guarded replace batches", () => { + const cases: Array<{ + operations: JSONPatchOperation[]; + expected: Record; + }> = [{ + operations: [ + { op: "test", path: "/items/0/meta/rank", value: 99 }, + { op: "replace", path: "/items/0/meta/rank", value: 1 }, + ], + expected: { + ok: false, + code: "test_failed", + pointer: "/items/0/meta/rank", + }, + }, { + operations: [ + { op: "test", path: "/items/0/meta/tag", value: "initial" }, + { op: "test", path: "/items/0/meta/rank", value: 99 }, + { op: "replace", path: "/items/0/meta/rank", value: 1 }, + ], + expected: { + ok: false, + code: "test_failed", + pointer: "/items/0/meta/rank", + }, + }, { + operations: [ + { op: "test", path: "/items/0/meta/tag", value: "initial" }, + { op: "test", path: "/items/0/meta/rank", value: 0 }, + { op: "replace", path: "/items/0/meta/missing", value: 1 }, + { op: "replace", path: "/items/0/meta/rank", value: 2 }, + ], + expected: { + ok: false, + code: "path_not_found", + pointer: "/items/0/meta/missing", + }, + }]; + + for (const { operations, expected } of cases) { + const doc = createDocument(); + const before = doc.value; + const listener = vi.fn(); + doc.subscribe(listener); + + expect(doc.patch(operations)).toMatchObject(expected); + expect(doc.value).toBe(before); + expect(doc.lastPatch).toEqual([]); + expect(doc.history.undoDepth).toBe(0); + expect(listener).not.toHaveBeenCalled(); + } + }); + test("discards the prepared draft when a later operation fails", () => { const doc = createDocument(); expect(doc.patch({ op: "replace", path: "/settings/active", value: "changed" })).toEqual({ ok: true }); @@ -215,16 +323,34 @@ describe("overlapping replace batch", () => { configurable: true, writable: true, }); - const doc = createJSONDocument(ProtoSchema, { container }, { trustedInitial: true }); - - expect(doc.patch([ - { op: "replace", path: "/container/__proto__", value: { rank: 1 } }, + const doc = createJSONDocument(ProtoSchema, { container }, { history: 10, trustedInitial: true }); + const replacement = { rank: 1 }; + const operations: JSONPatchOperation[] = [ + { op: "replace", path: "/container/__proto__", value: replacement }, { op: "replace", path: "/container/__proto__/rank", value: 2 }, - ])).toEqual({ ok: true }); + ]; + const inverse: JSONPatchOperation[] = [ + { op: "replace", path: "/container/__proto__/rank", value: 1 }, + { op: "replace", path: "/container/__proto__", value: { rank: 0 } }, + ]; + const observed: ReadonlyArray[] = []; + doc.subscribe((patch) => { observed.push(patch); }); + expect(doc.patch(operations)).toEqual({ ok: true }); + + expect(Object.prototype.hasOwnProperty.call(doc.value.container, "__proto__")).toBe(true); + expect(doc.value.container.__proto__).toEqual({ rank: 2 }); + expect(Object.getPrototypeOf(doc.value.container)).toBe(Object.prototype); + expect((Object.prototype as { rank?: number }).rank).toBeUndefined(); + expect(doc.undo()).toEqual({ ok: true }); expect(Object.prototype.hasOwnProperty.call(doc.value.container, "__proto__")).toBe(true); + expect(doc.value.container.__proto__).toEqual({ rank: 0 }); + expect(Object.getPrototypeOf(doc.value.container)).toBe(Object.prototype); + expect(doc.redo()).toEqual({ ok: true }); expect(doc.value.container.__proto__).toEqual({ rank: 2 }); expect(Object.getPrototypeOf(doc.value.container)).toBe(Object.prototype); expect((Object.prototype as { rank?: number }).rank).toBeUndefined(); + expect(replacement).toEqual({ rank: 1 }); + expect(observed).toEqual([operations, inverse, operations]); }); }); diff --git a/packages/proposed-changes/README.md b/packages/proposed-changes/README.md index 283e0ed3..f9614dcc 100644 --- a/packages/proposed-changes/README.md +++ b/packages/proposed-changes/README.md @@ -14,7 +14,7 @@ approval, or document cleanup suggestions. ## Scope - propose schema-safe document patches without mutating the document -- accept an open proposed change through `doc.patch` +- accept an open proposed change through one guarded `doc.patch` batch - reject an open proposed change without document mutation - detect stale changes by comparing guarded target values - restore persisted changes through `createProposedChanges(doc, { initial })` @@ -77,6 +77,30 @@ move/copy -> guard operation.from and parent(operation.path) `canAccept(id)` returns `stale_change` with the changed guard pointer when the guarded document value no longer matches the value captured at proposal time. +Guard comparison follows JSON structural equality: array order matters, while +object member insertion order does not. +`canAccept(id)` is a capability probe, not a reservation. `accept(id)` converts +the saved guards to RFC 6902 `test` operations and executes those assertions +before the proposed operations in the same atomic patch. If schema validation +reenters and changes the document, the core retries the whole batch against the +latest revision, including the guard assertions. + +When the proposed operations publish a document state change, the generated +`test` operations appear before them in `doc.lastPatch`, document subscriber +events, and the history forward patch. Undo contains only mutation inverses; +redo executes the guarded forward patch again. A test-only or otherwise +non-publishing patch does not emit a document subscriber event. + +An id is reserved until its document publication is committed. Reentrant +`accept` or `reject` calls for that id return `not_open`, `remove(id)` returns +`false`, and `load` or `clear` preserves the accepting entry. This keeps +document subscriber reentrancy from removing or replacing the proposal after +its patch publishes. The reservation is released before accepted proposal +listeners run. + +With a strict document, an execution-time guard failure follows the document's +existing error policy and throws `JSONDocumentError`; the guarded batch still +publishes no partial change and the proposal stays open. Audit metadata convention: @@ -105,16 +129,17 @@ editor command / AI result -> host renders current({ status: "all" }) -> host persists snapshot.changes if needed -> canAccept(id) controls disabled/reason UI --> accept(id) applies doc.patch(...) +-> accept(id) applies guard tests + proposed operations in one doc.patch(...) -> reject(id) closes without document mutation ``` ## Contract - Core `canPatch` is enough to reject schema-invalid proposals before storage. -- Core `patch` is enough to accept proposed changes atomically. -- The extension needs local guard values to avoid accepting stale changes that - would otherwise still be patchable. +- Core `patch` is enough to assert saved guard values and apply a proposed + change atomically when both are sent as one JSON Patch batch. +- The extension keeps local guard values and reasserts them during `accept` so + schema-validation reentrancy cannot apply an old proposal to a newer state. - Persistence does not require a core change yet because serialized `ProposedChange` values can be restored at extension level. - `data` remains host-owned metadata. The exported `ProposedChangeAuditData` diff --git a/packages/proposed-changes/src/accept.ts b/packages/proposed-changes/src/accept.ts index 3346df6b..b95432f5 100644 --- a/packages/proposed-changes/src/accept.ts +++ b/packages/proposed-changes/src/accept.ts @@ -1,15 +1,20 @@ import type { + JSONChangeMetadata, JSONDocument, + JSONPatchOperation, + JSONResult, } from "@interactive-os/json-document"; import { cloneJson, copyChange, + copyOperations, } from "./copy.js"; import { capabilityError, notFound, notOpen, + patchError, proposedChangeError, } from "./errors.js"; import type { @@ -18,6 +23,10 @@ import type { ProposedChangeResult, } from "./types.js"; +type GuardedPatchResult = + | { ok: true; result: Extract } + | ProposedChangeError; + export function canAcceptChange( doc: JSONDocument, changes: ReadonlyMap, @@ -31,6 +40,8 @@ export function canAcceptChange( if (stale !== null) return stale; const capability = doc.canPatch(change.operations); + const staleAfterCapability = staleGuard(doc, change); + if (staleAfterCapability !== null) return staleAfterCapability; if (!capability.ok) return capabilityError(id, capability); return { ok: true, change: copyChange(change) }; @@ -46,6 +57,46 @@ export function canCloseChange( return { ok: true, change: copyChange(change) }; } +export function applyGuardedChange( + doc: JSONDocument, + change: ProposedChange, + metadata?: JSONChangeMetadata, +): GuardedPatchResult { + const operations: JSONPatchOperation[] = change.guards.map((guard) => ({ + op: "test", + path: guard.path, + value: cloneJson(guard.value), + })); + operations.push(...copyOperations(change.operations)); + + const result = doc.patch(operations, metadata); + if (result.ok) return { ok: true, result }; + return failedGuard(change, result) ?? patchError(change.id, result); +} + +function failedGuard( + change: ProposedChange, + result: Extract, +): ProposedChangeError | null { + if ( + (result.code !== "test_failed" && result.code !== "path_not_found") + || result.pointer === undefined + ) { + return null; + } + const guard = change.guards.find(({ path }) => path === result.pointer); + if (guard === undefined) return null; + + const missing = result.code === "path_not_found"; + return proposedChangeError( + "stale_change", + missing + ? `proposed change guard path no longer exists: ${guard.path}` + : `proposed change guard changed: ${guard.path}`, + { id: change.id, pointer: guard.path }, + ); +} + function staleGuard( doc: JSONDocument, change: ProposedChange, @@ -58,7 +109,7 @@ function staleGuard( pointer: guard.path, }); } - if (JSON.stringify(read.value) !== JSON.stringify(guard.value)) { + if (!jsonEqual(read.value, guard.value)) { return proposedChangeError("stale_change", `proposed change guard changed: ${guard.path}`, { id: change.id, pointer: guard.path, @@ -67,3 +118,33 @@ function staleGuard( } return null; } + +function jsonEqual(left: unknown, right: unknown): boolean { + if (left === right) return true; + if ( + left === null + || right === null + || typeof left !== "object" + || typeof right !== "object" + || Array.isArray(left) !== Array.isArray(right) + ) { + return false; + } + if (Array.isArray(left)) { + if (left.length !== (right as unknown[]).length) return false; + for (let index = 0; index < left.length; index += 1) { + if (!jsonEqual(left[index], (right as unknown[])[index])) return false; + } + return true; + } + + const leftObject = left as Record; + const rightObject = right as Record; + const keys = Object.keys(leftObject); + if (keys.length !== Object.keys(rightObject).length) return false; + for (const key of keys) { + if (!Object.prototype.hasOwnProperty.call(rightObject, key)) return false; + if (!jsonEqual(leftObject[key], rightObject[key])) return false; + } + return true; +} diff --git a/packages/proposed-changes/src/create.ts b/packages/proposed-changes/src/create.ts index 32fa1713..e0a67d98 100644 --- a/packages/proposed-changes/src/create.ts +++ b/packages/proposed-changes/src/create.ts @@ -4,6 +4,7 @@ import type { } from "@interactive-os/json-document"; import { + applyGuardedChange, canAcceptChange, canCloseChange, } from "./accept.js"; @@ -11,7 +12,7 @@ import { copyChange, } from "./copy.js"; import { - patchError, + proposedChangeError, } from "./errors.js"; import { canProposeChange, @@ -41,6 +42,7 @@ export function createProposedChanges( changes: initial.changes, }; const listeners = new Set(); + const acceptingIds = new Set(); const emitIfChanged = (before: string): void => { const after = snapshotSignature(state.changes); @@ -66,22 +68,34 @@ export function createProposedChanges( emitIfChanged(before); return { ok: true, change: copyChange(change) }; }, - canAccept: (id) => canAcceptChange(doc, state.changes, id), + canAccept: (id) => acceptingIds.has(id) + ? acceptanceInProgress(id, "accept") + : canAcceptChange(doc, state.changes, id), accept(id, metadata?: JSONChangeMetadata) { - const capability = canAcceptChange(doc, state.changes, id); - if (!capability.ok) return capability; + if (acceptingIds.has(id)) return acceptanceInProgress(id, "accept"); + acceptingIds.add(id); + try { + const capability = canAcceptChange(doc, state.changes, id); + if (!capability.ok) return capability; - const result = doc.patch(capability.change.operations, metadata); - if (!result.ok) return patchError(id, result); + const guarded = applyGuardedChange(doc, capability.change, metadata); + if (!guarded.ok) return guarded; - const before = snapshotSignature(state.changes); - const change = state.changes.get(id)!; - change.status = "accepted"; - emitIfChanged(before); - return { ok: true, change: copyChange(change), result }; + const before = snapshotSignature(state.changes); + const change = state.changes.get(id)!; + change.status = "accepted"; + acceptingIds.delete(id); + emitIfChanged(before); + return { ok: true, change: copyChange(change), result: guarded.result }; + } finally { + acceptingIds.delete(id); + } }, - canReject: (id) => canCloseChange(state.changes, id), + canReject: (id) => acceptingIds.has(id) + ? acceptanceInProgress(id, "reject") + : canCloseChange(state.changes, id), reject(id) { + if (acceptingIds.has(id)) return acceptanceInProgress(id, "reject"); const capability = canCloseChange(state.changes, id); if (!capability.ok) return capability; @@ -93,12 +107,16 @@ export function createProposedChanges( }, load(next) { const before = snapshotSignature(state.changes); - const initial = initialChanges(next); + const accepting = [...acceptingIds] + .map((id) => state.changes.get(id)) + .filter((change): change is NonNullable => change !== undefined); + const initial = initialChanges([...next, ...accepting]); state.changes = initial.changes; state.nextId = initial.nextId; emitIfChanged(before); }, remove(id) { + if (acceptingIds.has(id)) return false; const before = snapshotSignature(state.changes); const removed = state.changes.delete(id); if (removed) emitIfChanged(before); @@ -106,7 +124,13 @@ export function createProposedChanges( }, clear() { const before = snapshotSignature(state.changes); - state.changes.clear(); + if (acceptingIds.size === 0) { + state.changes.clear(); + } else { + for (const id of state.changes.keys()) { + if (!acceptingIds.has(id)) state.changes.delete(id); + } + } emitIfChanged(before); }, subscribe(listener) { @@ -117,3 +141,7 @@ export function createProposedChanges( }, }; } + +function acceptanceInProgress(id: string, action: "accept" | "reject") { + return proposedChangeError("not_open", `cannot ${action} change while acceptance is in progress: ${id}`, { id }); +} diff --git a/packages/proposed-changes/tests/proposed-changes.test.ts b/packages/proposed-changes/tests/proposed-changes.test.ts index e7ebc883..19da1240 100644 --- a/packages/proposed-changes/tests/proposed-changes.test.ts +++ b/packages/proposed-changes/tests/proposed-changes.test.ts @@ -1,7 +1,10 @@ import { describe, expect, test } from "vitest"; import { z } from "zod"; -import { createJSONDocument } from "@interactive-os/json-document"; +import { + createJSONDocument, + JSONDocumentError, +} from "@interactive-os/json-document"; import { canAcceptChange, canProposeChange, @@ -64,6 +67,214 @@ describe("@interactive-os/json-document-proposed-changes", () => { }); }); + test("publishes guard assertions and the proposed patch as one history change", () => { + const doc = createJSONDocument(PageSchema, { + title: "Draft", + status: "draft", + sections: [], + }, { history: 10 }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + + const documentEvents: Array<{ + patch: ReadonlyArray; + label: string | undefined; + proposalStatus: string | undefined; + }> = []; + const proposalEvents: unknown[] = []; + doc.subscribe((patch, metadata) => { + documentEvents.push({ + patch, + label: metadata?.label, + proposalStatus: changes.byId("rename")?.status, + }); + }); + changes.subscribe((snapshot) => { proposalEvents.push(snapshot); }); + + expect(changes.accept("rename", { label: "accept rename" })).toMatchObject({ + ok: true, + change: { status: "accepted" }, + }); + const guardedPatch = [ + { op: "test", path: "/title", value: "Draft" }, + { op: "replace", path: "/title", value: "Reviewed" }, + ]; + expect(doc.lastPatch).toEqual(guardedPatch); + expect(doc.history.undoDepth).toBe(1); + expect(documentEvents).toEqual([{ + patch: guardedPatch, + label: "accept rename", + proposalStatus: "open", + }]); + expect(proposalEvents).toHaveLength(1); + expect(proposalEvents[0]).toMatchObject({ open: 0, accepted: 1 }); + + expect(doc.undo()).toEqual({ ok: true }); + expect(doc.value.title).toBe("Draft"); + expect(doc.lastPatch).toEqual([{ + op: "replace", + path: "/title", + value: "Draft", + }]); + expect(changes.byId("rename")).toMatchObject({ status: "accepted" }); + expect(doc.redo()).toEqual({ ok: true }); + expect(doc.value.title).toBe("Reviewed"); + expect(doc.lastPatch).toEqual(guardedPatch); + expect(doc.history.undoDepth).toBe(1); + }); + + test("keeps an accepting proposal reserved during document subscriber reentrancy", () => { + const doc = createPage(); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + let removedDuringAccept: boolean | undefined; + doc.subscribe(() => { + removedDuringAccept = changes.remove("rename"); + }); + + expect(changes.accept("rename")).toMatchObject({ + ok: true, + change: { id: "rename", status: "accepted" }, + }); + expect(removedDuringAccept).toBe(false); + expect(changes.byId("rename")).toMatchObject({ status: "accepted" }); + expect(doc.value.title).toBe("Reviewed"); + }); + + test("does not let a document subscriber reject an accepting proposal", () => { + const doc = createPage(); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + let nestedReject: unknown; + doc.subscribe(() => { + nestedReject = changes.reject("rename"); + }); + + expect(changes.accept("rename")).toMatchObject({ + ok: true, + change: { status: "accepted" }, + }); + expect(nestedReject).toMatchObject({ ok: false, code: "not_open" }); + expect(changes.byId("rename")).toMatchObject({ status: "accepted" }); + }); + + test("does not recursively accept the same proposal from a document subscriber", () => { + const doc = createPage(); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "same-title", + operations: { op: "replace", path: "/title", value: "Draft" }, + })).toMatchObject({ ok: true }); + let nestedAccept: unknown; + doc.subscribe(() => { + nestedAccept = changes.accept("same-title"); + }); + + expect(changes.accept("same-title")).toMatchObject({ + ok: true, + change: { status: "accepted" }, + }); + expect(nestedAccept).toMatchObject({ ok: false, code: "not_open" }); + expect(changes.byId("same-title")).toMatchObject({ status: "accepted" }); + }); + + test("preserves an accepting entry when a document subscriber loads proposals", () => { + const doc = createPage(); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + doc.subscribe(() => { + changes.load([{ + id: "rename", + status: "open", + operations: [{ op: "replace", path: "/status", value: "published" }], + guards: [{ path: "/status", value: "draft" }], + }, { + id: "other", + status: "open", + operations: [{ op: "replace", path: "/status", value: "review" }], + guards: [{ path: "/status", value: "draft" }], + }]); + }); + + expect(changes.accept("rename")).toMatchObject({ + ok: true, + change: { + status: "accepted", + operations: [{ path: "/title", value: "Reviewed" }], + }, + }); + expect(changes.byId("rename")).toMatchObject({ + status: "accepted", + operations: [{ path: "/title", value: "Reviewed" }], + }); + expect(changes.byId("other")).toMatchObject({ status: "open" }); + }); + + test("recomputes generated ids from entries merged by a reentrant load", () => { + const doc = createPage(); + const seed = createProposedChanges(doc); + expect(seed.propose({ + id: "change-100", + operations: { op: "replace", path: "/status", value: "review" }, + })).toMatchObject({ ok: true }); + expect(seed.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + const changes = createProposedChanges(doc, { + initial: seed.current({ status: "all" }).changes, + }); + doc.subscribe(() => { + changes.load([{ + id: "change-1", + status: "open", + operations: [{ op: "replace", path: "/status", value: "published" }], + guards: [{ path: "/status", value: "draft" }], + }]); + }); + + expect(changes.accept("rename")).toMatchObject({ ok: true }); + expect(changes.propose({ + operations: { op: "replace", path: "/status", value: "review" }, + })).toMatchObject({ + ok: true, + change: { id: "change-2" }, + }); + }); + + test("keeps accepting entries when a document subscriber clears proposals", () => { + const doc = createPage(); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + expect(changes.propose({ + id: "other", + operations: { op: "replace", path: "/status", value: "review" }, + })).toMatchObject({ ok: true }); + doc.subscribe(() => { changes.clear(); }); + + expect(changes.accept("rename")).toMatchObject({ + ok: true, + change: { status: "accepted" }, + }); + expect(changes.byId("rename")).toMatchObject({ status: "accepted" }); + expect(changes.byId("other")).toBeNull(); + }); + test("rejects a change without mutating the document", () => { const doc = createPage(); const changes = createProposedChanges(doc); @@ -107,6 +318,205 @@ describe("@interactive-os/json-document-proposed-changes", () => { expect(changes.byId("rename")).toMatchObject({ status: "open" }); }); + test("treats object member order as irrelevant for guard equality", () => { + const Schema = z.object({ + value: z.record(z.string(), z.number()), + }); + const doc = createJSONDocument(Schema, { + value: { a: 1, b: 2 }, + }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "replace-record", + operations: { op: "replace", path: "/value", value: { a: 2, b: 3 } }, + })).toMatchObject({ ok: true }); + doc.load({ value: { b: 2, a: 1 } }, { preserveHistory: true }); + + expect(changes.canAccept("replace-record")).toMatchObject({ ok: true }); + expect(changes.accept("replace-record")).toMatchObject({ ok: true }); + expect(doc.value.value).toEqual({ a: 2, b: 3 }); + }); + + test("does not overwrite a guard that changes during accept validation", () => { + let interfere: (() => void) | undefined; + let armed = false; + let reviewedValidations = 0; + const Schema = z.object({ + title: z.string(), + note: z.string(), + }).superRefine((value) => { + if (!armed || value.title !== "Reviewed") return; + reviewedValidations += 1; + if (reviewedValidations !== 2) return; + armed = false; + interfere?.(); + }); + const doc = createJSONDocument(Schema, { + title: "Draft", + note: "initial", + }, { history: 10 }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + + const documentEvents: ReadonlyArray[] = []; + const proposalEvents: unknown[] = []; + doc.subscribe((patch) => { documentEvents.push(patch); }); + changes.subscribe((snapshot) => { proposalEvents.push(snapshot); }); + interfere = () => { + doc.load({ title: "Edited concurrently", note: "remote" }, { preserveHistory: true }); + }; + armed = true; + + expect(changes.accept("rename", { label: "accept rename" })).toMatchObject({ + ok: false, + code: "stale_change", + pointer: "/title", + }); + expect(doc.value).toEqual({ title: "Edited concurrently", note: "remote" }); + expect(doc.lastPatch).toEqual([{ + op: "replace", + path: "", + value: { title: "Edited concurrently", note: "remote" }, + }]); + expect(doc.history.undoDepth).toBe(0); + expect(changes.byId("rename")).toMatchObject({ status: "open" }); + expect(documentEvents).toEqual([doc.lastPatch]); + expect(proposalEvents).toEqual([]); + }); + + test("reports a proposed-operation failure even when onError changes a guard", () => { + let handlePatchError: (() => void) | undefined; + let armed = false; + let reviewedValidations = 0; + const Schema = z.object({ title: z.string() }).superRefine((value, context) => { + if (!armed || value.title !== "Reviewed") return; + reviewedValidations += 1; + if (reviewedValidations !== 2) return; + context.addIssue({ code: "custom", message: "cannot publish this title" }); + }); + const doc = createJSONDocument(Schema, { title: "Draft" }, { + onError() { handlePatchError?.(); }, + }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + handlePatchError = () => { + doc.load({ title: "Edited by onError" }, { preserveHistory: true }); + }; + armed = true; + + expect(changes.accept("rename")).toMatchObject({ + ok: false, + code: "patch_failed", + result: { code: "schema_violation" }, + }); + expect(doc.value.title).toBe("Edited by onError"); + expect(changes.byId("rename")).toMatchObject({ status: "open" }); + }); + + test("reports the failed guard even when onError restores its value", () => { + let interfere: (() => void) | undefined; + let handlePatchError: (() => void) | undefined; + let armed = false; + let reviewedValidations = 0; + const Schema = z.object({ title: z.string() }).superRefine((value) => { + if (!armed || value.title !== "Reviewed") return; + reviewedValidations += 1; + if (reviewedValidations !== 2) return; + armed = false; + interfere?.(); + }); + const doc = createJSONDocument(Schema, { title: "Draft" }, { + onError() { handlePatchError?.(); }, + }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + interfere = () => { + doc.load({ title: "Edited concurrently" }, { preserveHistory: true }); + }; + handlePatchError = () => { + doc.load({ title: "Draft" }, { preserveHistory: true }); + }; + armed = true; + + expect(changes.accept("rename")).toMatchObject({ + ok: false, + code: "stale_change", + pointer: "/title", + }); + expect(doc.value.title).toBe("Draft"); + expect(changes.byId("rename")).toMatchObject({ status: "open" }); + }); + + test("prefers a newly stale guard over an obsolete capability failure", () => { + let interfere: (() => void) | undefined; + let armed = false; + const Schema = z.object({ title: z.string() }).superRefine((value, context) => { + if (!armed || value.title !== "Reviewed") return; + armed = false; + interfere?.(); + context.addIssue({ code: "custom", message: "obsolete validation failure" }); + }); + const doc = createJSONDocument(Schema, { title: "Draft" }, { history: 10 }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + interfere = () => { + doc.load({ title: "Edited concurrently" }, { preserveHistory: true }); + }; + armed = true; + + expect(changes.accept("rename")).toMatchObject({ + ok: false, + code: "stale_change", + pointer: "/title", + }); + expect(doc.value).toEqual({ title: "Edited concurrently" }); + expect(doc.history.undoDepth).toBe(0); + expect(changes.byId("rename")).toMatchObject({ status: "open" }); + }); + + test("preserves strict execution errors for an atomic guard failure", () => { + let interfere: (() => void) | undefined; + let armed = false; + let reviewedValidations = 0; + const Schema = z.object({ title: z.string() }).superRefine((value) => { + if (!armed || value.title !== "Reviewed") return; + reviewedValidations += 1; + if (reviewedValidations !== 2) return; + armed = false; + interfere?.(); + }); + const doc = createJSONDocument(Schema, { title: "Draft" }, { + history: 10, + strict: true, + }); + const changes = createProposedChanges(doc); + expect(changes.propose({ + id: "rename", + operations: { op: "replace", path: "/title", value: "Reviewed" }, + })).toMatchObject({ ok: true }); + interfere = () => { + doc.load({ title: "Edited concurrently" }, { preserveHistory: true }); + }; + armed = true; + + expect(() => changes.accept("rename")).toThrow(JSONDocumentError); + expect(doc.value).toEqual({ title: "Edited concurrently" }); + expect(doc.history.undoDepth).toBe(0); + expect(changes.byId("rename")).toMatchObject({ status: "open" }); + }); + test("rejects schema-invalid proposals before storing them", () => { const doc = createPage(); const changes = createProposedChanges(doc); diff --git a/scripts/benchmark-core.mjs b/scripts/benchmark-core.mjs index e76c6e57..ceb28eba 100644 --- a/scripts/benchmark-core.mjs +++ b/scripts/benchmark-core.mjs @@ -96,6 +96,7 @@ const batchSize = envNumber("PERF_BATCH", 1000); const individualCount = envNumber("PERF_INDIVIDUAL", 100); const jsonpathRepeats = envNumber("PERF_JSONPATH_REPEATS", 10000); const rounds = envNumber("PERF_ROUNDS", 5); +const overlappingReplaceRounds = Math.max(15, rounds); const forceGc = process.env.PERF_GC === "1"; const runtimeGc = typeof globalThis.gc === "function" ? globalThis.gc.bind(globalThis) : null; @@ -152,6 +153,11 @@ for (const size of sizes) { path: "/items/0/meta/rank", value: index, }); + const guardedOverlappingNestedReplaceOps = [ + { op: "test", path: "/items/0/meta", value: { tag: "tag-0", rank: 0 } }, + { op: "test", path: "/items/0/meta/rank", value: 0 }, + ...overlappingNestedReplaceOps, + ]; const escapedNestedFieldReplaceOps = Array.from({ length: Math.min(batchSize, size) }, (_, index) => ({ op: "replace", path: `/a~1b/${index}/m~0eta/ra~1nk`, @@ -565,6 +571,8 @@ for (const size of sizes) { applyTrustedPatch(state, nestedFieldReplaceOps, { valuesTrusted: true })); bench(`computeInverses nested field replace batch ${nestedFieldReplaceOps.length}`, Math.max(3, Math.ceil(rounds / 2)), () => computeInverses(state, nestedFieldReplaceOps)); + bench(`computeInverses overlapping nested replace batch ${overlappingNestedReplaceOps.length}`, overlappingReplaceRounds, () => + computeInverses(state, overlappingNestedReplaceOps)); bench(`accepted escaped nested field replace batch ${escapedNestedFieldReplaceOps.length}`, Math.max(3, Math.ceil(rounds / 2)), () => applyAcceptedPatch(escapedNestedState, escapedNestedFieldReplaceOps)); bench(`trusted escaped nested field replace batch ${escapedNestedFieldReplaceOps.length}`, Math.max(3, Math.ceil(rounds / 2)), () => @@ -580,10 +588,45 @@ for (const size of sizes) { { let doc; - benchWithSetup(`doc.patch overlapping nested replace batch ${overlappingNestedReplaceOps.length} history=0`, Math.max(3, Math.ceil(rounds / 2)), () => { + benchWithSetup(`doc.patch overlapping nested replace batch ${overlappingNestedReplaceOps.length} history=0`, overlappingReplaceRounds, () => { doc = createJSONDocument(Schema, state, { history: 0, trustedInitial: true }); }, () => doc.patch(overlappingNestedReplaceOps)); } + { + let doc; + benchWithSetup(`doc.patch overlapping nested replace batch ${overlappingNestedReplaceOps.length} history=100`, overlappingReplaceRounds, () => { + doc = createJSONDocument(Schema, state, { history: 100, trustedInitial: true }); + }, () => doc.patch(overlappingNestedReplaceOps)); + } + { + let doc; + benchWithSetup(`doc.patch guarded overlapping nested replace batch ${guardedOverlappingNestedReplaceOps.length} history=0`, overlappingReplaceRounds, () => { + doc = createJSONDocument(Schema, state, { history: 0, trustedInitial: true }); + }, () => doc.patch(guardedOverlappingNestedReplaceOps)); + } + { + let doc; + benchWithSetup(`doc.patch guarded overlapping nested replace batch ${guardedOverlappingNestedReplaceOps.length} history=100`, overlappingReplaceRounds, () => { + doc = createJSONDocument(Schema, state, { history: 100, trustedInitial: true }); + }, () => doc.patch(guardedOverlappingNestedReplaceOps)); + } + { + const doc = createJSONDocument(Schema, state, { history: 100, trustedInitial: true }); + const result = doc.patch(overlappingNestedReplaceOps); + if (!result.ok) throw new Error(`setup overlapping nested replace batch failed: ${JSON.stringify(result)}`); + benchWithSetup(`history undo overlapping nested replace batch ${overlappingNestedReplaceOps.length}`, overlappingReplaceRounds, () => { + if (!doc.history.canUndo) { + const redone = doc.history.redo(); + if (!redone) throw new Error("overlapping nested replace redo setup failed"); + } + }, () => ({ ok: doc.history.undo() })); + benchWithSetup(`history redo overlapping nested replace batch ${overlappingNestedReplaceOps.length}`, overlappingReplaceRounds, () => { + if (!doc.history.canRedo) { + const undone = doc.history.undo(); + if (!undone) throw new Error("overlapping nested replace undo setup failed"); + } + }, () => ({ ok: doc.history.redo() })); + } { const doc = createJSONDocument(EscapedNestedSchema, escapedNestedState, { history: 0 }); bench(`doc.patch escaped nested field replace batch ${escapedNestedFieldReplaceOps.length} history=0`, Math.max(3, Math.ceil(rounds / 2)), () =>