Skip to content

Commit 16f2ea2

Browse files
committed
fix(super-editor): throw REVISION_MISMATCH on metadata dry-run (SD-3104)
Follow-up on the previous dry-run fix. Returning a structured { success: false, failure: { code: 'REVISION_MISMATCH' } } on the attach/remove dry-run paths was shape-inconsistent: the live paths and the metadata.update dry-run all reach checkRevision via executeDomainCommand or executeOutOfBandMutation, both of which throw PlanError. A consumer wrapping these calls in one try/catch couldn't treat the two failure modes uniformly. Calls checkRevision directly in the attach + remove dry-run early-return paths so they throw on stale revisions, matching every other path. Drops the local REVISION_MISMATCH from the FailureCode union and the revisionMismatchFailure helper. Updates the two new unit tests to assert the throw. Also declares REVISION_MISMATCH on metadata.attach/update/remove throws arrays in operation-definitions, which the contract was missing.
1 parent fbd1b16 commit 16f2ea2

7 files changed

Lines changed: 35 additions & 51 deletions

File tree

apps/docs/document-api/reference/_generated-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1077,5 +1077,5 @@
10771077
}
10781078
],
10791079
"marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}",
1080-
"sourceHash": "97cf1fa7f05b40382d9839c0b60c7b97ccb7fddec642bc62383946ca76a75ee8"
1080+
"sourceHash": "30a98282620ba67a1197b0a2e228c6dd7203c4692c0ca15e56528855f02f38d1"
10811081
}

apps/docs/document-api/reference/metadata/attach.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ Returns an AnchoredMetadataAttachResult with the assigned id and the backing Sto
9595
- `INVALID_TARGET`
9696
- `INVALID_INPUT`
9797
- `CAPABILITY_UNAVAILABLE`
98+
- `REVISION_MISMATCH`
9899

99100
## Non-applied failure codes
100101

apps/docs/document-api/reference/metadata/remove.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ Returns an AnchoredMetadataMutationResult with the removed entry id on success o
6969
- `TARGET_NOT_FOUND`
7070
- `INVALID_TARGET`
7171
- `CAPABILITY_UNAVAILABLE`
72+
- `REVISION_MISMATCH`
7273

7374
## Non-applied failure codes
7475

apps/docs/document-api/reference/metadata/update.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ Returns an AnchoredMetadataMutationResult with the entry id on success or a fail
7272
- `INVALID_TARGET`
7373
- `INVALID_INPUT`
7474
- `CAPABILITY_UNAVAILABLE`
75+
- `REVISION_MISMATCH`
7576

7677
## Non-applied failure codes
7778

packages/document-api/src/contract/operation-definitions.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6363,7 +6363,7 @@ export const OPERATION_DEFINITIONS = {
63636363
supportsDryRun: true,
63646364
supportsTrackedMode: false,
63656365
possibleFailureCodes: ['TARGET_NOT_FOUND', 'INVALID_TARGET', 'INVALID_INPUT'],
6366-
throws: T_REF_INSERT,
6366+
throws: [...T_REF_INSERT, 'REVISION_MISMATCH'],
63676367
}),
63686368
referenceDocPath: 'metadata/attach.mdx',
63696369
referenceGroup: 'metadata',
@@ -6403,7 +6403,7 @@ export const OPERATION_DEFINITIONS = {
64036403
supportsDryRun: true,
64046404
supportsTrackedMode: false,
64056405
possibleFailureCodes: ['TARGET_NOT_FOUND', 'INVALID_INPUT'],
6406-
throws: T_REF_MUTATION,
6406+
throws: [...T_REF_MUTATION, 'REVISION_MISMATCH'],
64076407
}),
64086408
referenceDocPath: 'metadata/update.mdx',
64096409
referenceGroup: 'metadata',
@@ -6419,7 +6419,7 @@ export const OPERATION_DEFINITIONS = {
64196419
supportsDryRun: true,
64206420
supportsTrackedMode: false,
64216421
possibleFailureCodes: ['TARGET_NOT_FOUND'],
6422-
throws: T_REF_MUTATION_REMOVE,
6422+
throws: [...T_REF_MUTATION_REMOVE, 'REVISION_MISMATCH'],
64236423
}),
64246424
referenceDocPath: 'metadata/remove.mdx',
64256425
referenceGroup: 'metadata',

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/anchored-metadata-wrappers.test.ts

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -232,23 +232,22 @@ describe('anchored metadata wrappers', () => {
232232
expect(metadataListWrapper(editor).total).toBe(0);
233233
});
234234

235-
it('attach dry-run reports REVISION_MISMATCH when expectedRevision is stale', () => {
235+
it('attach dry-run throws REVISION_MISMATCH when expectedRevision is stale', () => {
236236
const editor = makeEditor();
237237

238-
const result = metadataAttachWrapper(
239-
editor,
240-
{ id: 'stale-attach', target: TARGET, namespace: 'urn:test:metadata', payload: { label: 'Preview' } },
241-
{ changeMode: 'direct', dryRun: true, expectedRevision: 'stale-1' },
242-
);
243-
244-
expect(result).toMatchObject({
245-
success: false,
246-
failure: { code: 'REVISION_MISMATCH' },
247-
});
238+
// Live attach throws via executeDomainCommand -> checkRevision; dry-run
239+
// must match that shape so consumers can use one try/catch path.
240+
expect(() =>
241+
metadataAttachWrapper(
242+
editor,
243+
{ id: 'stale-attach', target: TARGET, namespace: 'urn:test:metadata', payload: { label: 'Preview' } },
244+
{ changeMode: 'direct', dryRun: true, expectedRevision: 'stale-1' },
245+
),
246+
).toThrow(/REVISION_MISMATCH/);
248247
expect(metadataListWrapper(editor).total).toBe(0);
249248
});
250249

251-
it('remove dry-run reports REVISION_MISMATCH when expectedRevision is stale', () => {
250+
it('remove dry-run throws REVISION_MISMATCH when expectedRevision is stale', () => {
252251
const editor = makeEditor();
253252

254253
// Seed an entry so remove finds something to act on; without this it
@@ -259,17 +258,14 @@ describe('anchored metadata wrappers', () => {
259258
{ changeMode: 'direct' },
260259
);
261260

262-
const result = metadataRemoveWrapper(
263-
editor,
264-
{ id: 'seed' },
265-
{ changeMode: 'direct', dryRun: true, expectedRevision: 'stale-1' },
266-
);
267-
268-
expect(result).toMatchObject({
269-
success: false,
270-
failure: { code: 'REVISION_MISMATCH' },
271-
});
272-
// The seeded entry remains intact — dry-run with stale revision must not mutate.
261+
expect(() =>
262+
metadataRemoveWrapper(
263+
editor,
264+
{ id: 'seed' },
265+
{ changeMode: 'direct', dryRun: true, expectedRevision: 'stale-1' },
266+
),
267+
).toThrow(/REVISION_MISMATCH/);
268+
// The seeded entry remains intact: dry-run with stale revision must not mutate.
273269
expect(metadataGetWrapper(editor, { id: 'seed' })?.payload).toEqual({ v: 1 });
274270
});
275271

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/anchored-metadata-wrappers.ts

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { paginate } from '../helpers/adapter-utils.js';
3939
import { findAllSdtNodes, SDT_INLINE_NAME } from '../helpers/content-controls/index.js';
4040
import { executeOutOfBandMutation } from '../out-of-band-mutation.js';
4141
import { executeDomainCommand } from './plan-wrappers.js';
42-
import { getRevision } from './revision-tracker.js';
42+
import { checkRevision, getRevision } from './revision-tracker.js';
4343

4444
type XmlNode = {
4545
type?: string;
@@ -69,7 +69,7 @@ type MetadataPart = {
6969
entries: MetadataEntry[];
7070
};
7171

72-
type FailureCode = 'INVALID_INPUT' | 'INVALID_TARGET' | 'TARGET_NOT_FOUND' | 'REVISION_MISMATCH';
72+
type FailureCode = 'INVALID_INPUT' | 'INVALID_TARGET' | 'TARGET_NOT_FOUND';
7373

7474
type FailureResult = {
7575
success: false;
@@ -80,24 +80,6 @@ function failure(code: FailureCode, message: string): FailureResult {
8080
return { success: false, failure: { code, message } };
8181
}
8282

83-
/**
84-
* Dry-run paths take an early-return shortcut and don't go through
85-
* `executeOutOfBandMutation` / `executeDomainCommand`, which means they
86-
* also skip the revision guard those helpers run. Mirror the guard here
87-
* so optimistic-concurrency previews stay honest: `dryRun: true` with a
88-
* stale `expectedRevision` must report `REVISION_MISMATCH`, same as the
89-
* real path would.
90-
*/
91-
function revisionMismatchFailure(editor: Editor, expectedRevision: string | undefined): FailureResult | null {
92-
if (expectedRevision === undefined) return null;
93-
const current = getRevision(editor);
94-
if (expectedRevision === current) return null;
95-
return failure(
96-
'REVISION_MISMATCH',
97-
`Expected revision "${expectedRevision}" but document is at "${current}". Re-run query.match() to obtain a fresh ref.`,
98-
);
99-
}
100-
10183
function getConverter(editor: Editor): ConverterWithConvertedXml | null {
10284
return (editor as unknown as { converter?: ConverterWithConvertedXml }).converter ?? null;
10385
}
@@ -481,8 +463,10 @@ export function metadataAttachWrapper(
481463

482464
const preview = writeEntry(editor, input.namespace, id, input.payload, true);
483465
if (options?.dryRun) {
484-
const stale = revisionMismatchFailure(editor, options.expectedRevision);
485-
if (stale) return stale;
466+
// Mirror the revision guard that the live path runs inside
467+
// executeDomainCommand, so previews against stale revisions throw
468+
// REVISION_MISMATCH instead of falsely reporting success.
469+
checkRevision(editor, options.expectedRevision);
486470
return { success: true, id, namespace: input.namespace, partName: preview.partName };
487471
}
488472

@@ -540,8 +524,9 @@ export function metadataRemoveWrapper(
540524
}
541525

542526
if (options?.dryRun) {
543-
const stale = revisionMismatchFailure(editor, options.expectedRevision);
544-
if (stale) return stale;
527+
// Same revision guard as the live executeOutOfBandMutation /
528+
// executeDomainCommand paths. Throws PlanError(REVISION_MISMATCH).
529+
checkRevision(editor, options.expectedRevision);
545530
return { success: true, id: input.id };
546531
}
547532

0 commit comments

Comments
 (0)