From 1a6621d73ce45767340eb82aacdc474a5c040c80 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 17 Mar 2026 16:38:28 -0700 Subject: [PATCH 1/4] =?UTF-8?q?refactor(document-api):=20coherence=20pass?= =?UTF-8?q?=20=E2=80=94=20dead=20code,=20validation=20consistency,=20type?= =?UTF-8?q?=20safety?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../scripts/check-contract-parity.ts | 10 - .../src/authorities/authorities.ts | 9 +- .../src/authorities/authorities.types.ts | 14 +- .../document-api/src/bookmarks/bookmarks.ts | 5 +- .../src/bookmarks/bookmarks.types.ts | 9 +- .../document-api/src/captions/captions.ts | 5 +- .../src/captions/captions.types.ts | 16 +- .../document-api/src/citations/citations.ts | 13 +- .../src/citations/citations.types.ts | 23 +- .../src/clear-content/clear-content.ts | 4 + .../document-api/src/comments/comments.ts | 28 +- .../src/contract/operation-definitions.ts | 16 +- .../src/contract/operation-registry.ts | 1 + packages/document-api/src/create/create.ts | 101 ++--- .../document-api/src/cross-refs/cross-refs.ts | 5 +- .../src/cross-refs/cross-refs.types.ts | 9 +- packages/document-api/src/delete/delete.ts | 3 +- packages/document-api/src/errors.ts | 36 -- packages/document-api/src/fields/fields.ts | 5 +- .../document-api/src/fields/fields.types.ts | 9 +- packages/document-api/src/find/find.test.ts | 42 +- packages/document-api/src/find/find.ts | 29 +- .../document-api/src/footnotes/footnotes.ts | 5 +- .../src/footnotes/footnotes.types.ts | 16 +- .../document-api/src/format/format.test.ts | 2 +- packages/document-api/src/format/format.ts | 17 +- .../src/format/inline-run-patch.ts | 203 +++++----- packages/document-api/src/images/images.ts | 4 +- packages/document-api/src/index.test.ts | 30 +- packages/document-api/src/index.ts | 365 +++++++++++------- packages/document-api/src/index/index.ts | 9 +- .../document-api/src/index/index.types.ts | 16 +- packages/document-api/src/insert/insert.ts | 6 +- packages/document-api/src/invoke/invoke.ts | 2 +- packages/document-api/src/replace/replace.ts | 2 +- .../document-api/src/selection-mutation.ts | 4 +- packages/document-api/src/styles/apply.ts | 2 + packages/document-api/src/styles/index.ts | 5 +- .../document-api/src/styles/validation.ts | 94 +++-- packages/document-api/src/tables/tables.ts | 132 +++---- .../src/track-changes/track-changes.ts | 36 +- .../document-api/src/types/adapter-result.ts | 11 + packages/document-api/src/types/address.ts | 2 +- packages/document-api/src/types/index.ts | 1 + .../src/types/mutation-plan.types.ts | 14 +- .../src/validation-primitives.test.ts | 60 +-- .../document-api/src/validation-primitives.ts | 28 +- packages/document-api/src/write/write.ts | 12 +- .../assemble-adapters.ts | 3 +- 49 files changed, 657 insertions(+), 816 deletions(-) create mode 100644 packages/document-api/src/types/adapter-result.ts diff --git a/packages/document-api/scripts/check-contract-parity.ts b/packages/document-api/scripts/check-contract-parity.ts index 5ece571714..75296edb13 100644 --- a/packages/document-api/scripts/check-contract-parity.ts +++ b/packages/document-api/scripts/check-contract-parity.ts @@ -112,16 +112,6 @@ function createNoopAdapters(): DocumentApiAdapters { }, }), }, - format: { - apply: () => ({ - success: true, - resolution: { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 1 } }, - range: { from: 1, to: 2 }, - text: 'x', - }, - }), - }, trackChanges: { list: () => ({ evaluatedRevision: '', total: 0, items: [], page: { limit: 50, offset: 0, returned: 0 } }), get: ({ id }) => ({ diff --git a/packages/document-api/src/authorities/authorities.ts b/packages/document-api/src/authorities/authorities.ts index aeedcc2fac..ada58c1cc3 100644 --- a/packages/document-api/src/authorities/authorities.ts +++ b/packages/document-api/src/authorities/authorities.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { AuthoritiesAddress, AuthorityEntryAddress, @@ -51,9 +52,7 @@ export type AuthoritiesAdapter = AuthoritiesApi; // --------------------------------------------------------------------------- function validateAuthoritiesTarget(target: unknown, operationName: string): asserts target is AuthoritiesAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'block' || t.nodeType !== 'tableOfAuthorities' || typeof t.nodeId !== 'string') { throw new DocumentApiValidationError( @@ -65,9 +64,7 @@ function validateAuthoritiesTarget(target: unknown, operationName: string): asse } function validateAuthorityEntryTarget(target: unknown, operationName: string): asserts target is AuthorityEntryAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'inline' || t.nodeType !== 'authorityEntry') { throw new DocumentApiValidationError( diff --git a/packages/document-api/src/authorities/authorities.types.ts b/packages/document-api/src/authorities/authorities.types.ts index 7a58f3543d..ffe3238ebf 100644 --- a/packages/document-api/src/authorities/authorities.types.ts +++ b/packages/document-api/src/authorities/authorities.types.ts @@ -1,6 +1,6 @@ import type { InlineAnchor } from '../types/base.js'; import type { TextTarget } from '../types/address.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; import type { TocCreateLocation } from '../toc/toc.types.js'; @@ -163,21 +163,13 @@ export interface AuthoritiesMutationSuccess { success: true; authorities: AuthoritiesAddress; } -export interface AuthoritiesMutationFailure { - success: false; - failure: ReceiptFailure; -} -export type AuthoritiesMutationResult = AuthoritiesMutationSuccess | AuthoritiesMutationFailure; +export type AuthoritiesMutationResult = AuthoritiesMutationSuccess | AdapterMutationFailure; export interface AuthorityEntryMutationSuccess { success: true; entry: AuthorityEntryAddress; } -export interface AuthorityEntryMutationFailure { - success: false; - failure: ReceiptFailure; -} -export type AuthorityEntryMutationResult = AuthorityEntryMutationSuccess | AuthorityEntryMutationFailure; +export type AuthorityEntryMutationResult = AuthorityEntryMutationSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // List results diff --git a/packages/document-api/src/bookmarks/bookmarks.ts b/packages/document-api/src/bookmarks/bookmarks.ts index c7e7a492a2..c020bca39b 100644 --- a/packages/document-api/src/bookmarks/bookmarks.ts +++ b/packages/document-api/src/bookmarks/bookmarks.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { BookmarkAddress, BookmarkGetInput, @@ -32,9 +33,7 @@ export type BookmarksAdapter = BookmarksApi; // --------------------------------------------------------------------------- function validateBookmarkTarget(target: unknown, operationName: string): asserts target is BookmarkAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'entity' || t.entityType !== 'bookmark' || typeof t.name !== 'string') { diff --git a/packages/document-api/src/bookmarks/bookmarks.types.ts b/packages/document-api/src/bookmarks/bookmarks.types.ts index 8005051683..a1dfea95c2 100644 --- a/packages/document-api/src/bookmarks/bookmarks.types.ts +++ b/packages/document-api/src/bookmarks/bookmarks.types.ts @@ -1,6 +1,6 @@ import type { Position } from '../types/base.js'; import type { TextTarget } from '../types/address.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; // --------------------------------------------------------------------------- @@ -80,12 +80,7 @@ export interface BookmarkMutationSuccess { bookmark: BookmarkAddress; } -export interface BookmarkMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type BookmarkMutationResult = BookmarkMutationSuccess | BookmarkMutationFailure; +export type BookmarkMutationResult = BookmarkMutationSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // List result diff --git a/packages/document-api/src/captions/captions.ts b/packages/document-api/src/captions/captions.ts index 225e983ace..d0b2d71589 100644 --- a/packages/document-api/src/captions/captions.ts +++ b/packages/document-api/src/captions/captions.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { CaptionAddress, CaptionListInput, @@ -35,9 +36,7 @@ export type CaptionsAdapter = CaptionsApi; // --------------------------------------------------------------------------- function validateCaptionTarget(target: unknown, operationName: string): asserts target is CaptionAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'block' || t.nodeType !== 'paragraph' || typeof t.nodeId !== 'string') { diff --git a/packages/document-api/src/captions/captions.types.ts b/packages/document-api/src/captions/captions.types.ts index ee87c02e2a..d626a86682 100644 --- a/packages/document-api/src/captions/captions.types.ts +++ b/packages/document-api/src/captions/captions.types.ts @@ -1,5 +1,5 @@ import type { BlockNodeAddress } from '../types/base.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; // --------------------------------------------------------------------------- @@ -78,23 +78,13 @@ export interface CaptionMutationSuccess { caption: CaptionAddress; } -export interface CaptionMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type CaptionMutationResult = CaptionMutationSuccess | CaptionMutationFailure; +export type CaptionMutationResult = CaptionMutationSuccess | AdapterMutationFailure; export interface CaptionConfigSuccess { success: true; } -export interface CaptionConfigFailure { - success: false; - failure: ReceiptFailure; -} - -export type CaptionConfigResult = CaptionConfigSuccess | CaptionConfigFailure; +export type CaptionConfigResult = CaptionConfigSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // List result diff --git a/packages/document-api/src/citations/citations.ts b/packages/document-api/src/citations/citations.ts index 8c71047bb3..50c3ccd844 100644 --- a/packages/document-api/src/citations/citations.ts +++ b/packages/document-api/src/citations/citations.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { CitationAddress, CitationSourceAddress, @@ -65,9 +66,7 @@ export type CitationsAdapter = CitationsApi; // --------------------------------------------------------------------------- function validateCitationTarget(target: unknown, operationName: string): asserts target is CitationAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'inline' || t.nodeType !== 'citation') { @@ -80,9 +79,7 @@ function validateCitationTarget(target: unknown, operationName: string): asserts } function validateCitationSourceTarget(target: unknown, operationName: string): asserts target is CitationSourceAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'entity' || t.entityType !== 'citationSource' || typeof t.sourceId !== 'string') { @@ -95,9 +92,7 @@ function validateCitationSourceTarget(target: unknown, operationName: string): a } function validateBibliographyTarget(target: unknown, operationName: string): asserts target is BibliographyAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'block' || t.nodeType !== 'bibliography' || typeof t.nodeId !== 'string') { diff --git a/packages/document-api/src/citations/citations.types.ts b/packages/document-api/src/citations/citations.types.ts index f57a42c486..6dcee27f5c 100644 --- a/packages/document-api/src/citations/citations.types.ts +++ b/packages/document-api/src/citations/citations.types.ts @@ -1,6 +1,6 @@ import type { InlineAnchor } from '../types/base.js'; import type { TextTarget } from '../types/address.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; import type { TocCreateLocation } from '../toc/toc.types.js'; @@ -210,36 +210,21 @@ export interface CitationMutationSuccess { citation: CitationAddress; } -export interface CitationMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type CitationMutationResult = CitationMutationSuccess | CitationMutationFailure; +export type CitationMutationResult = CitationMutationSuccess | AdapterMutationFailure; export interface CitationSourceMutationSuccess { success: true; source: CitationSourceAddress; } -export interface CitationSourceMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type CitationSourceMutationResult = CitationSourceMutationSuccess | CitationSourceMutationFailure; +export type CitationSourceMutationResult = CitationSourceMutationSuccess | AdapterMutationFailure; export interface BibliographyMutationSuccess { success: true; bibliography: BibliographyAddress; } -export interface BibliographyMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type BibliographyMutationResult = BibliographyMutationSuccess | BibliographyMutationFailure; +export type BibliographyMutationResult = BibliographyMutationSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // List results diff --git a/packages/document-api/src/clear-content/clear-content.ts b/packages/document-api/src/clear-content/clear-content.ts index 20faa83d69..da2ad67f88 100644 --- a/packages/document-api/src/clear-content/clear-content.ts +++ b/packages/document-api/src/clear-content/clear-content.ts @@ -16,6 +16,10 @@ export interface ClearContentAdapter { /** * Execute a clearContent operation through the provided adapter. * + * clearContent is a destructive reset — tracked mode and dry run are not + * meaningful, so this accepts {@link RevisionGuardOptions} rather than + * `MutationOptions`. + * * @param adapter - Engine-specific clear-content adapter. * @param input - Canonical clear-content input (empty object). * @param options - Optional revision guard options. diff --git a/packages/document-api/src/comments/comments.ts b/packages/document-api/src/comments/comments.ts index 408e630115..a3a7375818 100644 --- a/packages/document-api/src/comments/comments.ts +++ b/packages/document-api/src/comments/comments.ts @@ -272,6 +272,10 @@ function validatePatchCommentInput(input: unknown): asserts input is CommentsPat /** * Execute `comments.create` — routes to `adapter.add` or `adapter.reply` * depending on whether `parentCommentId` is provided. + * + * Accepts {@link RevisionGuardOptions} instead of `MutationOptions` because + * comments route to specialized adapter methods (add/edit/reply/move/resolve/remove) + * outside the plan engine, so changeMode and dryRun are not applicable. */ export function executeCommentsCreate( adapter: CommentsAdapter, @@ -290,6 +294,10 @@ export function executeCommentsCreate( /** * Execute `comments.patch` — routes to exactly one adapter method based on * the single mutation field provided. Validation enforces one-field-per-call. + * + * Accepts {@link RevisionGuardOptions} instead of `MutationOptions` because + * comments route to specialized adapter methods (add/edit/reply/move/resolve/remove) + * outside the plan engine, so changeMode and dryRun are not applicable. */ export function executeCommentsPatch( adapter: CommentsAdapter, @@ -311,8 +319,11 @@ export function executeCommentsPatch( return adapter.setInternal({ commentId: input.commentId, isInternal: input.isInternal }, options); } - // Unreachable after validation, but satisfies the return type. - return { success: true }; + // Unreachable after validation — throw if we somehow get here. + throw new DocumentApiValidationError( + 'INTERNAL_ERROR', + 'comments.patch: no mutation field matched after validation. This is a bug.', + ); } /** @@ -326,19 +337,6 @@ export function executeCommentsDelete( return adapter.remove({ commentId: input.commentId }, options); } -// Internal-use execute wrappers (setActive, goTo remain for adapter consumers) -export function executeSetCommentActive( - adapter: CommentsAdapter, - input: SetCommentActiveInput, - options?: RevisionGuardOptions, -): Receipt { - return adapter.setActive(input, options); -} - -export function executeGoToComment(adapter: CommentsAdapter, input: GoToCommentInput): Receipt { - return adapter.goTo(input); -} - export function executeGetComment(adapter: CommentsAdapter, input: GetCommentInput): CommentInfo { return adapter.get(input); } diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index b873e9598d..08d2c52977 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -190,7 +190,6 @@ const T_PLAN_ENGINE = [ // All mutation operations include CAPABILITY_UNAVAILABLE (contract invariant). // _TRACKED suffix signals the operation also supports tracked change mode. const T_NOT_FOUND_COMMAND = ['TARGET_NOT_FOUND', 'INVALID_TARGET', 'CAPABILITY_UNAVAILABLE'] as const; -const T_NOT_FOUND_COMMAND_TRACKED = [...T_NOT_FOUND_COMMAND] as const; // Image operations can throw AMBIGUOUS_TARGET when multiple images share an sdImageId. const T_IMAGE_COMMAND = ['TARGET_NOT_FOUND', 'AMBIGUOUS_TARGET', 'INVALID_TARGET', 'CAPABILITY_UNAVAILABLE'] as const; @@ -826,7 +825,8 @@ export const OPERATION_DEFINITIONS = { 'sections.setOddEvenHeadersFooters': { memberPath: 'sections.setOddEvenHeadersFooters', description: 'Enable or disable odd/even header-footer mode in document settings.', - expectedResult: 'Returns a DocumentMutationResult receipt; reports NO_OP if the odd/even setting already matches.', + expectedResult: + 'Returns a DocumentMutationResult (not SectionMutationResult) because odd/even headers-footers is a document-level setting, not per-section. Reports NO_OP if the setting already matches.', requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'conditional', @@ -1941,7 +1941,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET'], - throws: [...T_NOT_FOUND_COMMAND_TRACKED, 'INVALID_TARGET', 'AMBIGUOUS_TARGET'], + throws: [...T_NOT_FOUND_COMMAND, 'INVALID_TARGET', 'AMBIGUOUS_TARGET'], }), referenceDocPath: 'create/table.mdx', referenceGroup: 'create', @@ -1976,7 +1976,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET', 'NO_OP'], - throws: [...T_NOT_FOUND_COMMAND_TRACKED, 'INVALID_TARGET'], + throws: [...T_NOT_FOUND_COMMAND, 'INVALID_TARGET'], }), referenceDocPath: 'tables/delete.mdx', referenceGroup: 'tables', @@ -2078,7 +2078,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET'], - throws: [...T_NOT_FOUND_COMMAND_TRACKED, 'INVALID_TARGET'], + throws: [...T_NOT_FOUND_COMMAND, 'INVALID_TARGET'], }), referenceDocPath: 'tables/insert-row.mdx', referenceGroup: 'tables', @@ -2093,7 +2093,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET', 'NO_OP'], - throws: [...T_NOT_FOUND_COMMAND_TRACKED, 'INVALID_TARGET'], + throws: [...T_NOT_FOUND_COMMAND, 'INVALID_TARGET'], }), referenceDocPath: 'tables/delete-row.mdx', referenceGroup: 'tables', @@ -2158,7 +2158,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET'], - throws: [...T_NOT_FOUND_COMMAND_TRACKED, 'INVALID_TARGET'], + throws: [...T_NOT_FOUND_COMMAND, 'INVALID_TARGET'], }), referenceDocPath: 'tables/insert-column.mdx', referenceGroup: 'tables', @@ -2173,7 +2173,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET', 'NO_OP'], - throws: [...T_NOT_FOUND_COMMAND_TRACKED, 'INVALID_TARGET'], + throws: [...T_NOT_FOUND_COMMAND, 'INVALID_TARGET'], }), referenceDocPath: 'tables/delete-column.mdx', referenceGroup: 'tables', diff --git a/packages/document-api/src/contract/operation-registry.ts b/packages/document-api/src/contract/operation-registry.ts index 96ac9de5c5..7158ff066e 100644 --- a/packages/document-api/src/contract/operation-registry.ts +++ b/packages/document-api/src/contract/operation-registry.ts @@ -729,6 +729,7 @@ export interface OperationRegistry extends FormatInlineAliasOperationRegistry { options: MutationOptions; output: SectionMutationResult; }; + // Returns DocumentMutationResult (not SectionMutationResult) — document-level setting, not per-section. 'sections.setOddEvenHeadersFooters': { input: SectionsSetOddEvenHeadersFootersInput; options: MutationOptions; diff --git a/packages/document-api/src/create/create.ts b/packages/document-api/src/create/create.ts index fcb686f3af..eef949ce61 100644 --- a/packages/document-api/src/create/create.ts +++ b/packages/document-api/src/create/create.ts @@ -92,12 +92,25 @@ function validateTargetOrNodeIdCreateLocation(at: TableCreateLocation, operation } } -const SECTION_BREAK_TYPES: readonly SectionBreakType[] = ['continuous', 'nextPage', 'evenPage', 'oddPage'] as const; +/** + * All create-location union types share `{ kind: 'documentEnd' }` as a member, + * so we use that as the common base constraint. The generic preserves the + * concrete location type at each call site. + */ +type CreateLocation = { kind: string }; -function normalizeSectionBreakCreateLocation(location?: SectionBreakCreateLocation): SectionBreakCreateLocation { - return location ?? { kind: 'documentEnd' }; +/** + * Normalises an optional create-location to a concrete value (defaulting to + * `{ kind: 'documentEnd' }`) and runs the caller-supplied validation against it. + */ +function normalizeCreateLocation(location: T | undefined, validate: (loc: T) => void): T { + const normalized = (location ?? { kind: 'documentEnd' }) as T; + validate(normalized); + return normalized; } +const SECTION_BREAK_TYPES: readonly SectionBreakType[] = ['continuous', 'nextPage', 'evenPage', 'oddPage'] as const; + function validateMarginValue(field: string, value: unknown): void { if (typeof value !== 'number' || !Number.isFinite(value) || value < 0) { throw new DocumentApiValidationError('INVALID_INPUT', `${field} must be a non-negative number.`, { @@ -132,13 +145,9 @@ function validateCreateSectionBreakInput(input: CreateSectionBreakInput): void { } } -function normalizeParagraphCreateLocation(location?: ParagraphCreateLocation): ParagraphCreateLocation { - return location ?? { kind: 'documentEnd' }; -} - export function normalizeCreateParagraphInput(input: CreateParagraphInput): CreateParagraphInput { return { - at: normalizeParagraphCreateLocation(input.at), + at: normalizeCreateLocation(input.at, () => {}), text: input.text ?? '', }; } @@ -148,19 +157,17 @@ export function executeCreateParagraph( input: CreateParagraphInput, options?: MutationOptions, ): CreateParagraphResult { - const normalized = normalizeCreateParagraphInput(input); - validateTargetOnlyCreateLocation(normalized.at!, 'create.paragraph'); + const at = normalizeCreateLocation(input.at, (loc) => + validateTargetOnlyCreateLocation(loc, 'create.paragraph'), + ); + const normalized: CreateParagraphInput = { at, text: input.text ?? '' }; return adapter.paragraph(normalized, normalizeMutationOptions(options)); } -function normalizeHeadingCreateLocation(location?: HeadingCreateLocation): HeadingCreateLocation { - return location ?? { kind: 'documentEnd' }; -} - export function normalizeCreateHeadingInput(input: CreateHeadingInput): CreateHeadingInput { return { level: input.level, - at: normalizeHeadingCreateLocation(input.at), + at: normalizeCreateLocation(input.at, () => {}), text: input.text ?? '', }; } @@ -170,20 +177,18 @@ export function executeCreateHeading( input: CreateHeadingInput, options?: MutationOptions, ): CreateHeadingResult { - const normalized = normalizeCreateHeadingInput(input); - validateTargetOnlyCreateLocation(normalized.at!, 'create.heading'); + const at = normalizeCreateLocation(input.at, (loc) => + validateTargetOnlyCreateLocation(loc, 'create.heading'), + ); + const normalized: CreateHeadingInput = { level: input.level, at, text: input.text ?? '' }; return adapter.heading(normalized, normalizeMutationOptions(options)); } -function normalizeTableCreateLocation(location?: TableCreateLocation): TableCreateLocation { - return location ?? { kind: 'documentEnd' }; -} - export function normalizeCreateTableInput(input: CreateTableInput): CreateTableInput { return { rows: input.rows, columns: input.columns, - at: normalizeTableCreateLocation(input.at), + at: normalizeCreateLocation(input.at, () => {}), }; } @@ -192,14 +197,16 @@ export function executeCreateTable( input: CreateTableInput, options?: MutationOptions, ): CreateTableResult { - const normalized = normalizeCreateTableInput(input); - validateTargetOrNodeIdCreateLocation(normalized.at!, 'create.table'); + const at = normalizeCreateLocation(input.at, (loc) => + validateTargetOrNodeIdCreateLocation(loc, 'create.table'), + ); + const normalized: CreateTableInput = { rows: input.rows, columns: input.columns, at }; return adapter.table(normalized, normalizeMutationOptions(options)); } export function normalizeCreateSectionBreakInput(input: CreateSectionBreakInput): CreateSectionBreakInput { return { - at: normalizeSectionBreakCreateLocation(input.at), + at: normalizeCreateLocation(input.at, () => {}), breakType: input.breakType, pageMargins: input.pageMargins, headerFooterMargins: input.headerFooterMargins, @@ -211,19 +218,22 @@ export function executeCreateSectionBreak( input: CreateSectionBreakInput, options?: MutationOptions, ): CreateSectionBreakResult { - const normalized = normalizeCreateSectionBreakInput(input); - validateTargetOnlyCreateLocation(normalized.at!, 'create.sectionBreak'); + const at = normalizeCreateLocation(input.at, (loc) => + validateTargetOnlyCreateLocation(loc, 'create.sectionBreak'), + ); + const normalized: CreateSectionBreakInput = { + at, + breakType: input.breakType, + pageMargins: input.pageMargins, + headerFooterMargins: input.headerFooterMargins, + }; validateCreateSectionBreakInput(normalized); return adapter.sectionBreak(normalized, normalizeMutationOptions(options)); } -function normalizeTocCreateLocation(location?: TocCreateLocation): TocCreateLocation { - return location ?? { kind: 'documentEnd' }; -} - export function normalizeCreateTableOfContentsInput(input: CreateTableOfContentsInput): CreateTableOfContentsInput { return { - at: normalizeTocCreateLocation(input.at), + at: normalizeCreateLocation(input.at, () => {}), config: input.config, }; } @@ -233,19 +243,18 @@ export function executeCreateTableOfContents( input: CreateTableOfContentsInput, options?: MutationOptions, ): CreateTableOfContentsResult { - const normalized = normalizeCreateTableOfContentsInput(input); - const at = normalized.at!; - - // TocCreateLocation only supports the `target` form, not the legacy `nodeId` form. - // Reject `nodeId` explicitly when callers send untyped payloads. - if ((at.kind === 'before' || at.kind === 'after') && 'nodeId' in at) { - throw new DocumentApiValidationError( - 'INVALID_TARGET', - 'create.tableOfContents requires at.target for before/after positioning. The nodeId form is not supported.', - { fields: ['at.nodeId'] }, - ); - } - - validateTargetOnlyCreateLocation(at, 'create.tableOfContents'); + const at = normalizeCreateLocation(input.at, (loc) => { + // TocCreateLocation only supports the `target` form, not the legacy `nodeId` form. + // Reject `nodeId` explicitly when callers send untyped payloads. + if ((loc.kind === 'before' || loc.kind === 'after') && 'nodeId' in loc) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + 'create.tableOfContents requires at.target for before/after positioning. The nodeId form is not supported.', + { fields: ['at.nodeId'] }, + ); + } + validateTargetOnlyCreateLocation(loc, 'create.tableOfContents'); + }); + const normalized: CreateTableOfContentsInput = { at, config: input.config }; return adapter.tableOfContents(normalized, normalizeMutationOptions(options)); } diff --git a/packages/document-api/src/cross-refs/cross-refs.ts b/packages/document-api/src/cross-refs/cross-refs.ts index 0ffa4375d6..756b9c9f7f 100644 --- a/packages/document-api/src/cross-refs/cross-refs.ts +++ b/packages/document-api/src/cross-refs/cross-refs.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { CrossRefAddress, CrossRefGetInput, @@ -32,9 +33,7 @@ export type CrossRefsAdapter = CrossRefsApi; // --------------------------------------------------------------------------- function validateCrossRefTarget(target: unknown, operationName: string): asserts target is CrossRefAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'inline' || t.nodeType !== 'crossRef') { diff --git a/packages/document-api/src/cross-refs/cross-refs.types.ts b/packages/document-api/src/cross-refs/cross-refs.types.ts index f00f850296..ba14f7a26e 100644 --- a/packages/document-api/src/cross-refs/cross-refs.types.ts +++ b/packages/document-api/src/cross-refs/cross-refs.types.ts @@ -1,6 +1,6 @@ import type { InlineAnchor } from '../types/base.js'; import type { TextTarget } from '../types/address.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; // --------------------------------------------------------------------------- @@ -97,11 +97,6 @@ export interface CrossRefMutationSuccess { crossRef: CrossRefAddress; } -export interface CrossRefMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type CrossRefMutationResult = CrossRefMutationSuccess | CrossRefMutationFailure; +export type CrossRefMutationResult = CrossRefMutationSuccess | AdapterMutationFailure; export type CrossRefsListResult = DiscoveryOutput; diff --git a/packages/document-api/src/delete/delete.ts b/packages/document-api/src/delete/delete.ts index 2ab45f2014..28790b2065 100644 --- a/packages/document-api/src/delete/delete.ts +++ b/packages/document-api/src/delete/delete.ts @@ -9,6 +9,7 @@ import type { SelectionTarget, DeleteBehavior } from '../types/address.js'; import type { TextMutationReceipt } from '../types/receipt.js'; import type { MutationOptions } from '../types/mutation-plan.types.js'; import type { SelectionMutationAdapter } from '../selection-mutation.js'; +import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; import { isRecord, assertNoUnknownFields } from '../validation-primitives.js'; import { isSelectionTarget } from '../validation/selection-target-validator.js'; @@ -105,6 +106,6 @@ export function executeDelete( ref: input.ref, behavior: input.behavior ?? 'selection', }, - options, + normalizeMutationOptions(options), ); } diff --git a/packages/document-api/src/errors.ts b/packages/document-api/src/errors.ts index 930c28712f..72cb5ee05b 100644 --- a/packages/document-api/src/errors.ts +++ b/packages/document-api/src/errors.ts @@ -5,8 +5,6 @@ * across package boundaries and bundling scenarios. */ -import type { SDError, SDErrorCode } from './types/sd-contract.js'; - export class DocumentApiValidationError extends Error { readonly code: string; readonly details?: Record; @@ -19,37 +17,3 @@ export class DocumentApiValidationError extends Error { Object.setPrototypeOf(this, DocumentApiValidationError.prototype); } } - -// --------------------------------------------------------------------------- -// SDErrorCode crosswalk — maps legacy codes to SDM/1 error vocabulary -// --------------------------------------------------------------------------- - -const LEGACY_TO_SD_CODE: Record = { - INVALID_FRAGMENT: 'INVALID_PAYLOAD', - EMPTY_FRAGMENT: 'INVALID_PAYLOAD', - INVALID_INPUT: 'INVALID_PAYLOAD', - INVALID_TARGET: 'INVALID_TARGET', - TARGET_NOT_FOUND: 'TARGET_NOT_FOUND', - CAPABILITY_UNAVAILABLE: 'CAPABILITY_UNSUPPORTED', - INVALID_NESTING: 'INVALID_NESTING', - INVALID_PLACEMENT: 'INVALID_PLACEMENT', - REVISION_MISMATCH: 'REVISION_MISMATCH', - INTERNAL_ERROR: 'INTERNAL_ERROR', - CAPABILITY_UNSUPPORTED: 'CAPABILITY_UNSUPPORTED', - PRECONDITION_FAILED: 'INVALID_PAYLOAD', -}; - -/** - * Converts a {@link DocumentApiValidationError} to an {@link SDError}. - * - * Maps legacy error codes to the normative SDErrorCode vocabulary. - * Unknown codes fall through as `INTERNAL_ERROR`. - */ -export function toSDError(error: DocumentApiValidationError): SDError { - const sdCode = LEGACY_TO_SD_CODE[error.code] ?? 'INTERNAL_ERROR'; - return { - code: sdCode, - message: error.message, - ...(error.details ? { details: error.details } : {}), - }; -} diff --git a/packages/document-api/src/fields/fields.ts b/packages/document-api/src/fields/fields.ts index 275d81ee5b..833c8f1955 100644 --- a/packages/document-api/src/fields/fields.ts +++ b/packages/document-api/src/fields/fields.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { FieldAddress, FieldGetInput, @@ -32,9 +33,7 @@ export type FieldsAdapter = FieldsApi; // --------------------------------------------------------------------------- function validateFieldTarget(target: unknown, operationName: string): asserts target is FieldAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'field' || typeof t.blockId !== 'string' || typeof t.occurrenceIndex !== 'number') { diff --git a/packages/document-api/src/fields/fields.types.ts b/packages/document-api/src/fields/fields.types.ts index c5f58b14d6..bf775ecd0f 100644 --- a/packages/document-api/src/fields/fields.types.ts +++ b/packages/document-api/src/fields/fields.types.ts @@ -1,5 +1,5 @@ import type { TextTarget } from '../types/address.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; // --------------------------------------------------------------------------- @@ -72,11 +72,6 @@ export interface FieldMutationSuccess { field: FieldAddress; } -export interface FieldMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type FieldMutationResult = FieldMutationSuccess | FieldMutationFailure; +export type FieldMutationResult = FieldMutationSuccess | AdapterMutationFailure; export type FieldsListResult = DiscoveryOutput; diff --git a/packages/document-api/src/find/find.test.ts b/packages/document-api/src/find/find.test.ts index 1c34d8303c..2b9b13334e 100644 --- a/packages/document-api/src/find/find.test.ts +++ b/packages/document-api/src/find/find.test.ts @@ -1,5 +1,5 @@ -import { executeFind, executeLegacyFind, normalizeFindQuery } from './find.js'; -import type { Query, FindOutput, Selector } from '../types/index.js'; +import { executeFind, normalizeFindQuery } from './find.js'; +import type { Query, Selector } from '../types/index.js'; import type { SDFindInput, SDFindResult } from '../types/sd-envelope.js'; import type { FindAdapter } from './find.js'; @@ -198,41 +198,3 @@ describe('executeFind', () => { expect(adapter.find).toHaveBeenCalledWith(input); }); }); - -describe('executeLegacyFind', () => { - it('normalizes input and delegates to findLegacy', () => { - const envelope: FindOutput = { - evaluatedRevision: 'r1', - total: 0, - items: [], - page: { limit: 5, offset: 0, returned: 0 }, - }; - const adapter: FindAdapter = { - find: vi.fn(() => ({ total: 0, limit: 0, offset: 0, items: [] })), - findLegacy: vi.fn(() => envelope), - }; - - const result = executeLegacyFind(adapter, { nodeType: 'paragraph' }, { limit: 5 }); - - expect(result).toBe(envelope); - expect(adapter.findLegacy).toHaveBeenCalledWith({ - select: { type: 'node', nodeType: 'paragraph' }, - limit: 5, - offset: undefined, - within: undefined, - require: undefined, - includeNodes: undefined, - includeUnknown: undefined, - }); - }); - - it('throws when findLegacy is not available', () => { - const adapter: FindAdapter = { - find: vi.fn(() => ({ total: 0, limit: 0, offset: 0, items: [] })), - }; - - expect(() => executeLegacyFind(adapter, { nodeType: 'paragraph' })).toThrow( - 'Legacy find is not supported by this adapter', - ); - }); -}); diff --git a/packages/document-api/src/find/find.ts b/packages/document-api/src/find/find.ts index 1fa5d92e55..af377ad15f 100644 --- a/packages/document-api/src/find/find.ts +++ b/packages/document-api/src/find/find.ts @@ -1,4 +1,4 @@ -import type { BlockNodeAddress, NodeSelector, Query, FindOutput, Selector, TextSelector } from '../types/index.js'; +import type { BlockNodeAddress, NodeSelector, Query, Selector, TextSelector } from '../types/index.js'; import type { SDFindInput, SDFindResult } from '../types/sd-envelope.js'; import { DocumentApiValidationError } from '../errors.js'; @@ -33,13 +33,6 @@ export interface FindAdapter { * @returns The find result as an SDFindResult envelope. */ find(input: SDFindInput): SDFindResult; - - /** - * Legacy query-based find, used internally by info-adapter. - * Returns the old FindOutput shape for backward compatibility. - * @internal - */ - findLegacy?(query: Query): FindOutput; } /** Normalizes a selector shorthand into its canonical discriminated-union form. @@ -119,23 +112,3 @@ export function normalizeFindQuery(selectorOrQuery: Selector | Query, options?: export function executeFind(adapter: FindAdapter, input: SDFindInput): SDFindResult { return adapter.find(input); } - -/** - * Executes a legacy find using the old Query/Selector interface. - * Used internally by info-adapter. Prefers `findLegacy` if available, - * otherwise translates to SDFindInput. - * - * @internal - */ -export function executeLegacyFind( - adapter: FindAdapter, - selectorOrQuery: Selector | Query, - options?: FindOptions, -): FindOutput { - const query = normalizeFindQuery(selectorOrQuery, options); - if (adapter.findLegacy) { - return adapter.findLegacy(query); - } - // Fallback: shouldn't happen in practice since super-editor adapter provides findLegacy - throw new Error('Legacy find is not supported by this adapter'); -} diff --git a/packages/document-api/src/footnotes/footnotes.ts b/packages/document-api/src/footnotes/footnotes.ts index 7b1ef77466..616470996a 100644 --- a/packages/document-api/src/footnotes/footnotes.ts +++ b/packages/document-api/src/footnotes/footnotes.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { FootnoteAddress, FootnoteGetInput, @@ -35,9 +36,7 @@ export type FootnotesAdapter = FootnotesApi; // --------------------------------------------------------------------------- function validateFootnoteTarget(target: unknown, operationName: string): asserts target is FootnoteAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'entity' || t.entityType !== 'footnote' || typeof t.noteId !== 'string') { diff --git a/packages/document-api/src/footnotes/footnotes.types.ts b/packages/document-api/src/footnotes/footnotes.types.ts index fbe337818b..d4d5dacfc3 100644 --- a/packages/document-api/src/footnotes/footnotes.types.ts +++ b/packages/document-api/src/footnotes/footnotes.types.ts @@ -1,4 +1,4 @@ -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; import type { TextTarget } from '../types/address.js'; @@ -98,12 +98,7 @@ export interface FootnoteMutationSuccess { footnote: FootnoteAddress; } -export interface FootnoteMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type FootnoteMutationResult = FootnoteMutationSuccess | FootnoteMutationFailure; +export type FootnoteMutationResult = FootnoteMutationSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // Config result @@ -113,12 +108,7 @@ export interface FootnoteConfigSuccess { success: true; } -export interface FootnoteConfigFailure { - success: false; - failure: ReceiptFailure; -} - -export type FootnoteConfigResult = FootnoteConfigSuccess | FootnoteConfigFailure; +export type FootnoteConfigResult = FootnoteConfigSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // List result diff --git a/packages/document-api/src/format/format.test.ts b/packages/document-api/src/format/format.test.ts index 8e16460e02..d0297f1b1c 100644 --- a/packages/document-api/src/format/format.test.ts +++ b/packages/document-api/src/format/format.test.ts @@ -85,7 +85,7 @@ describe('executeStyleApply validation', () => { it('rejects unknown inline keys', () => { const adapter = makeAdapter(); const input = { target: TARGET, inline: { superscript: true } }; - expect(() => executeStyleApply(adapter, input as any)).toThrow('Unknown inline style key "superscript"'); + expect(() => executeStyleApply(adapter, input as any)).toThrow('Unknown inline property: "superscript".'); }); it('rejects invalid boolean payload type', () => { diff --git a/packages/document-api/src/format/format.ts b/packages/document-api/src/format/format.ts index 33a559c5c8..2336ff3b09 100644 --- a/packages/document-api/src/format/format.ts +++ b/packages/document-api/src/format/format.ts @@ -66,19 +66,14 @@ export interface StyleApplyInput { inline: InlineRunPatch; } -/** Options for `format.apply` — same shape as all other mutations. */ +/** + * Named alias for MutationOptions on format.apply. + * + * Exists as a distinct type so the styles system can add style-specific + * options (e.g. scope, priority) without changing the public API shape. + */ export type StyleApplyOptions = MutationOptions; -// --------------------------------------------------------------------------- -// Legacy FormatAdapter — kept temporarily for inline aliases that still -// route through the old path. Will be fully retired once all aliases migrate. -// --------------------------------------------------------------------------- - -/** @deprecated Use SelectionMutationAdapter instead. Kept for inline-alias compatibility. */ -export interface FormatAdapter { - apply(input: StyleApplyInput, options?: MutationOptions): TextMutationReceipt; -} - // --------------------------------------------------------------------------- // Public API surface // --------------------------------------------------------------------------- diff --git a/packages/document-api/src/format/inline-run-patch.ts b/packages/document-api/src/format/inline-run-patch.ts index 7c4755718c..24dac5fd89 100644 --- a/packages/document-api/src/format/inline-run-patch.ts +++ b/packages/document-api/src/format/inline-run-patch.ts @@ -566,117 +566,96 @@ function validateStylisticSets(value: unknown): void { }); } +const PROPERTY_VALIDATOR_MAP: Record void> = { + bold: assertBooleanOrNull, + italic: assertBooleanOrNull, + strike: assertBooleanOrNull, + dstrike: assertBooleanOrNull, + smallCaps: assertBooleanOrNull, + caps: assertBooleanOrNull, + outline: assertBooleanOrNull, + shadow: assertBooleanOrNull, + emboss: assertBooleanOrNull, + imprint: assertBooleanOrNull, + rtl: assertBooleanOrNull, + cs: assertBooleanOrNull, + bCs: assertBooleanOrNull, + iCs: assertBooleanOrNull, + vanish: assertBooleanOrNull, + webHidden: assertBooleanOrNull, + specVanish: assertBooleanOrNull, + snapToGrid: assertBooleanOrNull, + oMath: assertBooleanOrNull, + contextualAlternates: assertBooleanOrNull, + underline: (value) => validateUnderlinePatch(value), + highlight: assertStringOrNull, + color: assertStringOrNull, + fontFamily: assertStringOrNull, + rStyle: assertStringOrNull, + em: assertStringOrNull, + ligatures: assertStringOrNull, + numForm: assertStringOrNull, + numSpacing: assertStringOrNull, + fontSize: assertNumberOrNull, + fontSizeCs: assertNumberOrNull, + letterSpacing: assertNumberOrNull, + charScale: assertNumberOrNull, + kerning: assertNumberOrNull, + position: assertNumberOrNull, + vertAlign: assertVertAlignOrNull, + shading: (value) => + validateObjectPatch(value, 'shading', SHADING_ALLOWED_KEYS, { + fill: assertStringOrNull, + color: assertStringOrNull, + val: assertStringOrNull, + }), + border: (value) => + validateObjectPatch(value, 'border', BORDER_ALLOWED_KEYS, { + val: assertStringOrNull, + color: assertStringOrNull, + sz: assertNumberOrNull, + space: assertNumberOrNull, + }), + fitText: (value) => + validateObjectPatch(value, 'fitText', FIT_TEXT_ALLOWED_KEYS, { + val: assertNumberOrNull, + id: assertStringOrNull, + }), + lang: (value) => + validateObjectPatch(value, 'lang', LANG_ALLOWED_KEYS, { + val: assertStringOrNull, + eastAsia: assertStringOrNull, + bidi: assertStringOrNull, + }), + rFonts: (value) => + validateObjectPatch(value, 'rFonts', RFONTS_ALLOWED_KEYS, { + ascii: assertStringOrNull, + hAnsi: assertStringOrNull, + eastAsia: assertStringOrNull, + cs: assertStringOrNull, + asciiTheme: assertStringOrNull, + hAnsiTheme: assertStringOrNull, + eastAsiaTheme: assertStringOrNull, + csTheme: assertStringOrNull, + hint: assertStringOrNull, + }), + eastAsianLayout: (value) => + validateObjectPatch(value, 'eastAsianLayout', EAST_ASIAN_LAYOUT_ALLOWED_KEYS, { + id: assertStringOrNull, + combine: assertBooleanOrNull, + combineBrackets: assertStringOrNull, + vert: assertBooleanOrNull, + vertCompress: assertBooleanOrNull, + }), + stylisticSets: (value) => validateStylisticSets(value), +}; + function validateInlineProperty(key: string, value: unknown): void { - switch (key as InlineRunPatchKey) { - case 'bold': - case 'italic': - case 'strike': - case 'dstrike': - case 'smallCaps': - case 'caps': - case 'outline': - case 'shadow': - case 'emboss': - case 'imprint': - case 'rtl': - case 'cs': - case 'bCs': - case 'iCs': - case 'vanish': - case 'webHidden': - case 'specVanish': - case 'snapToGrid': - case 'oMath': - case 'contextualAlternates': - assertBooleanOrNull(value, `inline.${key}`); - return; - case 'underline': - validateUnderlinePatch(value); - return; - case 'highlight': - case 'color': - case 'fontFamily': - case 'rStyle': - case 'em': - case 'ligatures': - case 'numForm': - case 'numSpacing': - assertStringOrNull(value, `inline.${key}`); - return; - case 'fontSize': - case 'fontSizeCs': - case 'letterSpacing': - case 'charScale': - case 'kerning': - case 'position': - assertNumberOrNull(value, `inline.${key}`); - return; - case 'vertAlign': - assertVertAlignOrNull(value, 'inline.vertAlign'); - return; - case 'shading': - validateObjectPatch(value, 'shading', SHADING_ALLOWED_KEYS, { - fill: assertStringOrNull, - color: assertStringOrNull, - val: assertStringOrNull, - }); - return; - case 'border': - validateObjectPatch(value, 'border', BORDER_ALLOWED_KEYS, { - val: assertStringOrNull, - color: assertStringOrNull, - sz: assertNumberOrNull, - space: assertNumberOrNull, - }); - return; - case 'fitText': - validateObjectPatch(value, 'fitText', FIT_TEXT_ALLOWED_KEYS, { - val: assertNumberOrNull, - id: assertStringOrNull, - }); - return; - case 'lang': - validateObjectPatch(value, 'lang', LANG_ALLOWED_KEYS, { - val: assertStringOrNull, - eastAsia: assertStringOrNull, - bidi: assertStringOrNull, - }); - return; - case 'rFonts': - validateObjectPatch(value, 'rFonts', RFONTS_ALLOWED_KEYS, { - ascii: assertStringOrNull, - hAnsi: assertStringOrNull, - eastAsia: assertStringOrNull, - cs: assertStringOrNull, - asciiTheme: assertStringOrNull, - hAnsiTheme: assertStringOrNull, - eastAsiaTheme: assertStringOrNull, - csTheme: assertStringOrNull, - hint: assertStringOrNull, - }); - return; - case 'eastAsianLayout': - validateObjectPatch(value, 'eastAsianLayout', EAST_ASIAN_LAYOUT_ALLOWED_KEYS, { - id: assertStringOrNull, - combine: assertBooleanOrNull, - combineBrackets: assertStringOrNull, - vert: assertBooleanOrNull, - vertCompress: assertBooleanOrNull, - }); - return; - case 'stylisticSets': - validateStylisticSets(value); - return; - default: - throw new DocumentApiValidationError( - 'INVALID_INPUT', - `Unknown inline style key "${key}". Known keys are defined by INLINE_PROPERTY_REGISTRY.`, - { - field: 'inline', - key, - }, - ); + const validator = PROPERTY_VALIDATOR_MAP[key as InlineRunPatchKey]; + if (!validator) { + throw new DocumentApiValidationError('INVALID_INPUT', `Unknown inline property: "${key}".`, { field: key }); } + validator(value, `inline.${key}`); } export function validateInlineRunPatch(patch: unknown): asserts patch is InlineRunPatch { @@ -693,12 +672,6 @@ export function validateInlineRunPatch(patch: unknown): asserts patch is InlineR } for (const key of keys) { - if (!INLINE_PROPERTY_KEY_SET.has(key)) { - throw new DocumentApiValidationError('INVALID_INPUT', `Unknown inline style key "${key}".`, { - field: 'inline', - key, - }); - } validateInlineProperty(key, patch[key]); } } diff --git a/packages/document-api/src/images/images.ts b/packages/document-api/src/images/images.ts index 4c72d4b09e..03b8e06491 100644 --- a/packages/document-api/src/images/images.ts +++ b/packages/document-api/src/images/images.ts @@ -49,7 +49,7 @@ const VALID_IMAGE_SIZE_UNITS = new Set(['px', 'pt', 'twip']); // --------------------------------------------------------------------------- export interface ImagesAdapter { - list(input: ImagesListInput): ImagesListResult; + list(input?: ImagesListInput): ImagesListResult; get(input: ImagesGetInput): ImageSummary; delete(input: ImagesDeleteInput, options?: MutationOptions): ImagesMutationResult; move(input: MoveImageInput, options?: MutationOptions): ImagesMutationResult; @@ -126,7 +126,7 @@ function requireUnsignedInt32(value: unknown, field: string): asserts value is n // Execute functions // --------------------------------------------------------------------------- -export function executeImagesList(adapter: ImagesAdapter, input: ImagesListInput): ImagesListResult { +export function executeImagesList(adapter: ImagesAdapter, input?: ImagesListInput): ImagesListResult { return adapter.list(input ?? {}); } diff --git a/packages/document-api/src/index.test.ts b/packages/document-api/src/index.test.ts index d36a261011..98974d803e 100644 --- a/packages/document-api/src/index.test.ts +++ b/packages/document-api/src/index.test.ts @@ -633,7 +633,7 @@ describe('createDocumentApi', () => { ); expect(selectionAdpt.execute).toHaveBeenCalledWith( { kind: 'delete', target: selectionTarget, ref: undefined, behavior: 'selection' }, - undefined, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, ); }); @@ -1136,14 +1136,16 @@ describe('createDocumentApi', () => { }); } - function expectValidationError(fn: () => void, messageMatch?: string | RegExp) { + function expectValidationError(fn: () => void, messageMatch?: string | RegExp, expectedCode?: string) { try { fn(); expect.fail('Expected DocumentApiValidationError to be thrown'); } catch (err: unknown) { const e = err as { name: string; code: string; message: string }; expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe('INVALID_TARGET'); + if (expectedCode) { + expect(e.code).toBe(expectedCode); + } if (messageMatch) { if (typeof messageMatch === 'string') { expect(e.message).toContain(messageMatch); @@ -1323,7 +1325,10 @@ describe('createDocumentApi', () => { api.insert({ value: '# Heading', type: 'markdown' }); expect(writeAdpt.insertStructured).toHaveBeenCalledTimes(1); - expect(writeAdpt.insertStructured).toHaveBeenCalledWith({ value: '# Heading', type: 'markdown' }, undefined); + expect(writeAdpt.insertStructured).toHaveBeenCalledWith( + { value: '# Heading', type: 'markdown' }, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, + ); expect(writeAdpt.write).not.toHaveBeenCalled(); }); @@ -1346,7 +1351,10 @@ describe('createDocumentApi', () => { api.insert({ value: '

Hello

', type: 'html' }); expect(writeAdpt.insertStructured).toHaveBeenCalledTimes(1); - expect(writeAdpt.insertStructured).toHaveBeenCalledWith({ value: '

Hello

', type: 'html' }, undefined); + expect(writeAdpt.insertStructured).toHaveBeenCalledWith( + { value: '

Hello

', type: 'html' }, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, + ); expect(writeAdpt.write).not.toHaveBeenCalled(); }); @@ -1393,7 +1401,7 @@ describe('createDocumentApi', () => { api.insert({ target, value: '**bold**', type: 'markdown' }); expect(writeAdpt.insertStructured).toHaveBeenCalledWith( { target, value: '**bold**', type: 'markdown' }, - undefined, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, ); }); @@ -1787,7 +1795,7 @@ describe('createDocumentApi', () => { api.delete({ target: SELECTION_TARGET }); expect(selectionAdpt.execute).toHaveBeenCalledWith( { kind: 'delete', target: SELECTION_TARGET, ref: undefined, behavior: 'selection' }, - undefined, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, ); }); }); @@ -1955,7 +1963,7 @@ describe('createDocumentApi', () => { } catch (err: unknown) { const e = err as { name: string; code: string; message: string }; expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe('INVALID_TARGET'); + if (messageMatch) { if (typeof messageMatch === 'string') { expect(e.message).toContain(messageMatch); @@ -2085,7 +2093,7 @@ describe('createDocumentApi', () => { } catch (err: unknown) { const e = err as { name: string; code: string; message: string }; expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe('INVALID_TARGET'); + if (messageMatch) { if (typeof messageMatch === 'string') { expect(e.message).toContain(messageMatch); @@ -2253,7 +2261,7 @@ describe('createDocumentApi', () => { } catch (err: unknown) { const e = err as { name: string; code: string; message: string }; expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe('INVALID_TARGET'); + if (messageMatch) { if (typeof messageMatch === 'string') { expect(e.message).toContain(messageMatch); @@ -2367,7 +2375,7 @@ describe('createDocumentApi', () => { } catch (err: unknown) { const e = err as { name: string; code: string; message: string }; expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe('INVALID_TARGET'); + if (messageMatch) { if (typeof messageMatch === 'string') { expect(e.message).toContain(messageMatch); diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index 7b6b44312a..4e2d156672 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -296,7 +296,13 @@ import type { DiffApplyInput, DiffApplyOptions, } from './diff/diff.types.js'; -import { executeTableOperation } from './tables/tables.js'; +import { + executeTableLocatorOp, + executeRowLocatorOp, + executeColumnLocatorOp, + executeMergeRangeLocatorOp, + executeDocumentLevelTableOp, +} from './tables/tables.js'; import type { ParagraphsAdapter, ParagraphFormatApi, @@ -819,7 +825,6 @@ export type { GetHtmlAdapter, GetHtmlInput } from './get-html/get-html.js'; export type { InfoAdapter, InfoInput } from './info/info.js'; export type { WriteAdapter, WriteRequest } from './write/write.js'; export type { - FormatAdapter, FormatInlineAliasApi, FormatInlineAliasInput, FormatBoldInput, @@ -1288,7 +1293,7 @@ export type { SetCommentActiveInput, } from './comments/comments.js'; export type { CommentInfo, CommentsListQuery, CommentsListResult } from './comments/comments.types.js'; -export { DocumentApiValidationError, toSDError } from './errors.js'; +export { DocumentApiValidationError } from './errors.js'; export { textReceiptToSDReceipt, buildStructuralReceipt } from './receipt-bridge.js'; export type { StructuralReceiptParams } from './receipt-bridge.js'; export { isBlockNodeAddress } from './validation-primitives.js'; @@ -1645,6 +1650,26 @@ export interface DocumentApiAdapters { * } * ``` */ +/** + * Validates and normalizes query.match input — accepts canonical QueryMatchInput + * or a flat TextSelector/NodeSelector shorthand. + */ +function executeQueryMatch( + adapter: { match(input: QueryMatchInput): QueryMatchOutput }, + input: QueryMatchInput | TextSelector | NodeSelector, +): QueryMatchOutput { + if (!input || typeof input !== 'object') { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + 'query.match requires a QueryMatchInput or selector object.', + { value: input }, + ); + } + // Normalize flat selector shorthand to canonical nested form. + const normalized: QueryMatchInput = 'select' in input ? input : { select: input }; + return adapter.match(normalized); +} + function requireAdapter(adapter: T | undefined, namespace: string): T { if (!adapter) { throw new DocumentApiValidationError( @@ -1881,7 +1906,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { capabilities, images: { list(input?: ImagesListInput): ImagesListResult { - return executeImagesList(adapters.images, input ?? {}); + return executeImagesList(adapters.images, input); }, get(input: ImagesGetInput): ImageSummary { return executeImagesGet(adapters.images, input); @@ -2125,7 +2150,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { }, tables: { convertFromText(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.convertFromText', adapters.tables.convertFromText.bind(adapters.tables), input, @@ -2133,10 +2158,10 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, delete(input, options?) { - return executeTableOperation('tables.delete', adapters.tables.delete.bind(adapters.tables), input, options); + return executeTableLocatorOp('tables.delete', adapters.tables.delete.bind(adapters.tables), input, options); }, clearContents(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.clearContents', adapters.tables.clearContents.bind(adapters.tables), input, @@ -2144,13 +2169,13 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, move(input, options?) { - return executeTableOperation('tables.move', adapters.tables.move.bind(adapters.tables), input, options); + return executeTableLocatorOp('tables.move', adapters.tables.move.bind(adapters.tables), input, options); }, split(input, options?) { - return executeTableOperation('tables.split', adapters.tables.split.bind(adapters.tables), input, options); + return executeTableLocatorOp('tables.split', adapters.tables.split.bind(adapters.tables), input, options); }, convertToText(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.convertToText', adapters.tables.convertToText.bind(adapters.tables), input, @@ -2158,7 +2183,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setLayout(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setLayout', adapters.tables.setLayout.bind(adapters.tables), input, @@ -2166,23 +2191,13 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, insertRow(input, options?) { - return executeTableOperation( - 'tables.insertRow', - adapters.tables.insertRow.bind(adapters.tables), - input, - options, - ); + return executeRowLocatorOp('tables.insertRow', adapters.tables.insertRow.bind(adapters.tables), input, options); }, deleteRow(input, options?) { - return executeTableOperation( - 'tables.deleteRow', - adapters.tables.deleteRow.bind(adapters.tables), - input, - options, - ); + return executeRowLocatorOp('tables.deleteRow', adapters.tables.deleteRow.bind(adapters.tables), input, options); }, setRowHeight(input, options?) { - return executeTableOperation( + return executeRowLocatorOp( 'tables.setRowHeight', adapters.tables.setRowHeight.bind(adapters.tables), input, @@ -2190,7 +2205,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, distributeRows(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.distributeRows', adapters.tables.distributeRows.bind(adapters.tables), input, @@ -2198,7 +2213,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setRowOptions(input, options?) { - return executeTableOperation( + return executeRowLocatorOp( 'tables.setRowOptions', adapters.tables.setRowOptions.bind(adapters.tables), input, @@ -2206,7 +2221,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, insertColumn(input, options?) { - return executeTableOperation( + return executeColumnLocatorOp( 'tables.insertColumn', adapters.tables.insertColumn.bind(adapters.tables), input, @@ -2214,7 +2229,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, deleteColumn(input, options?) { - return executeTableOperation( + return executeColumnLocatorOp( 'tables.deleteColumn', adapters.tables.deleteColumn.bind(adapters.tables), input, @@ -2222,7 +2237,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setColumnWidth(input, options?) { - return executeTableOperation( + return executeColumnLocatorOp( 'tables.setColumnWidth', adapters.tables.setColumnWidth.bind(adapters.tables), input, @@ -2230,7 +2245,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, distributeColumns(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.distributeColumns', adapters.tables.distributeColumns.bind(adapters.tables), input, @@ -2238,7 +2253,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, insertCell(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.insertCell', adapters.tables.insertCell.bind(adapters.tables), input, @@ -2246,7 +2261,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, deleteCell(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.deleteCell', adapters.tables.deleteCell.bind(adapters.tables), input, @@ -2254,7 +2269,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, mergeCells(input, options?) { - return executeTableOperation( + return executeMergeRangeLocatorOp( 'tables.mergeCells', adapters.tables.mergeCells.bind(adapters.tables), input, @@ -2262,7 +2277,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, unmergeCells(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.unmergeCells', adapters.tables.unmergeCells.bind(adapters.tables), input, @@ -2270,7 +2285,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, splitCell(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.splitCell', adapters.tables.splitCell.bind(adapters.tables), input, @@ -2278,7 +2293,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setCellProperties(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setCellProperties', adapters.tables.setCellProperties.bind(adapters.tables), input, @@ -2286,10 +2301,10 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, sort(input, options?) { - return executeTableOperation('tables.sort', adapters.tables.sort.bind(adapters.tables), input, options); + return executeTableLocatorOp('tables.sort', adapters.tables.sort.bind(adapters.tables), input, options); }, setAltText(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setAltText', adapters.tables.setAltText.bind(adapters.tables), input, @@ -2297,10 +2312,10 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setStyle(input, options?) { - return executeTableOperation('tables.setStyle', adapters.tables.setStyle.bind(adapters.tables), input, options); + return executeTableLocatorOp('tables.setStyle', adapters.tables.setStyle.bind(adapters.tables), input, options); }, clearStyle(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.clearStyle', adapters.tables.clearStyle.bind(adapters.tables), input, @@ -2308,7 +2323,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setStyleOption(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setStyleOption', adapters.tables.setStyleOption.bind(adapters.tables), input, @@ -2316,7 +2331,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setBorder(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setBorder', adapters.tables.setBorder.bind(adapters.tables), input, @@ -2324,7 +2339,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, clearBorder(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.clearBorder', adapters.tables.clearBorder.bind(adapters.tables), input, @@ -2332,7 +2347,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, applyBorderPreset(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.applyBorderPreset', adapters.tables.applyBorderPreset.bind(adapters.tables), input, @@ -2340,7 +2355,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setShading(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setShading', adapters.tables.setShading.bind(adapters.tables), input, @@ -2348,7 +2363,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, clearShading(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.clearShading', adapters.tables.clearShading.bind(adapters.tables), input, @@ -2356,7 +2371,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setTablePadding(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setTablePadding', adapters.tables.setTablePadding.bind(adapters.tables), input, @@ -2364,7 +2379,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setCellPadding(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setCellPadding', adapters.tables.setCellPadding.bind(adapters.tables), input, @@ -2372,7 +2387,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, setCellSpacing(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.setCellSpacing', adapters.tables.setCellSpacing.bind(adapters.tables), input, @@ -2380,7 +2395,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, clearCellSpacing(input, options?) { - return executeTableOperation( + return executeTableLocatorOp( 'tables.clearCellSpacing', adapters.tables.clearCellSpacing.bind(adapters.tables), input, @@ -2400,10 +2415,10 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { return adapters.tables.getStyles(input); }, setDefaultStyle(input: TablesSetDefaultStyleInput, options?: MutationOptions) { - return adapters.tables.setDefaultStyle(input, options); + return executeDocumentLevelTableOp(adapters.tables.setDefaultStyle.bind(adapters.tables), input, options); }, clearDefaultStyle(input?: TablesClearDefaultStyleInput, options?: MutationOptions) { - return adapters.tables.clearDefaultStyle(input, options); + return executeDocumentLevelTableOp(adapters.tables.clearDefaultStyle.bind(adapters.tables), input, options); }, }, toc: { @@ -2495,90 +2510,179 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { }, }, contentControls: { - list: (query) => executeContentControlsList(adapters.contentControls, query), - get: (input) => executeContentControlsGet(adapters.contentControls, input), - listInRange: (input) => executeContentControlsListInRange(adapters.contentControls, input), - selectByTag: (input) => executeContentControlsSelectByTag(adapters.contentControls, input), - selectByTitle: (input) => executeContentControlsSelectByTitle(adapters.contentControls, input), - listChildren: (input) => executeContentControlsListChildren(adapters.contentControls, input), - getParent: (input) => executeContentControlsGetParent(adapters.contentControls, input), - wrap: (input, options) => executeContentControlsWrap(adapters.contentControls, input, options), - unwrap: (input, options) => executeContentControlsUnwrap(adapters.contentControls, input, options), - delete: (input, options) => executeContentControlsDelete(adapters.contentControls, input, options), - copy: (input, options) => executeContentControlsCopy(adapters.contentControls, input, options), - move: (input, options) => executeContentControlsMove(adapters.contentControls, input, options), - patch: (input, options) => executeContentControlsPatch(adapters.contentControls, input, options), - setLockMode: (input, options) => executeContentControlsSetLockMode(adapters.contentControls, input, options), - setType: (input, options) => executeContentControlsSetType(adapters.contentControls, input, options), - getContent: (input) => executeContentControlsGetContent(adapters.contentControls, input), - replaceContent: (input, options) => - executeContentControlsReplaceContent(adapters.contentControls, input, options), - clearContent: (input, options) => executeContentControlsClearContent(adapters.contentControls, input, options), - appendContent: (input, options) => executeContentControlsAppendContent(adapters.contentControls, input, options), - prependContent: (input, options) => - executeContentControlsPrependContent(adapters.contentControls, input, options), - insertBefore: (input, options) => executeContentControlsInsertBefore(adapters.contentControls, input, options), - insertAfter: (input, options) => executeContentControlsInsertAfter(adapters.contentControls, input, options), - getBinding: (input) => executeContentControlsGetBinding(adapters.contentControls, input), - setBinding: (input, options) => executeContentControlsSetBinding(adapters.contentControls, input, options), - clearBinding: (input, options) => executeContentControlsClearBinding(adapters.contentControls, input, options), - getRawProperties: (input) => executeContentControlsGetRawProperties(adapters.contentControls, input), - patchRawProperties: (input, options) => - executeContentControlsPatchRawProperties(adapters.contentControls, input, options), - validateWordCompatibility: (input) => - executeContentControlsValidateWordCompatibility(adapters.contentControls, input), - normalizeWordCompatibility: (input, options) => - executeContentControlsNormalizeWordCompatibility(adapters.contentControls, input, options), - normalizeTagPayload: (input, options) => - executeContentControlsNormalizeTagPayload(adapters.contentControls, input, options), + list(query) { + return executeContentControlsList(adapters.contentControls, query); + }, + get(input) { + return executeContentControlsGet(adapters.contentControls, input); + }, + listInRange(input) { + return executeContentControlsListInRange(adapters.contentControls, input); + }, + selectByTag(input) { + return executeContentControlsSelectByTag(adapters.contentControls, input); + }, + selectByTitle(input) { + return executeContentControlsSelectByTitle(adapters.contentControls, input); + }, + listChildren(input) { + return executeContentControlsListChildren(adapters.contentControls, input); + }, + getParent(input) { + return executeContentControlsGetParent(adapters.contentControls, input); + }, + wrap(input, options) { + return executeContentControlsWrap(adapters.contentControls, input, options); + }, + unwrap(input, options) { + return executeContentControlsUnwrap(adapters.contentControls, input, options); + }, + delete(input, options) { + return executeContentControlsDelete(adapters.contentControls, input, options); + }, + copy(input, options) { + return executeContentControlsCopy(adapters.contentControls, input, options); + }, + move(input, options) { + return executeContentControlsMove(adapters.contentControls, input, options); + }, + patch(input, options) { + return executeContentControlsPatch(adapters.contentControls, input, options); + }, + setLockMode(input, options) { + return executeContentControlsSetLockMode(adapters.contentControls, input, options); + }, + setType(input, options) { + return executeContentControlsSetType(adapters.contentControls, input, options); + }, + getContent(input) { + return executeContentControlsGetContent(adapters.contentControls, input); + }, + replaceContent(input, options) { + return executeContentControlsReplaceContent(adapters.contentControls, input, options); + }, + clearContent(input, options) { + return executeContentControlsClearContent(adapters.contentControls, input, options); + }, + appendContent(input, options) { + return executeContentControlsAppendContent(adapters.contentControls, input, options); + }, + prependContent(input, options) { + return executeContentControlsPrependContent(adapters.contentControls, input, options); + }, + insertBefore(input, options) { + return executeContentControlsInsertBefore(adapters.contentControls, input, options); + }, + insertAfter(input, options) { + return executeContentControlsInsertAfter(adapters.contentControls, input, options); + }, + getBinding(input) { + return executeContentControlsGetBinding(adapters.contentControls, input); + }, + setBinding(input, options) { + return executeContentControlsSetBinding(adapters.contentControls, input, options); + }, + clearBinding(input, options) { + return executeContentControlsClearBinding(adapters.contentControls, input, options); + }, + getRawProperties(input) { + return executeContentControlsGetRawProperties(adapters.contentControls, input); + }, + patchRawProperties(input, options) { + return executeContentControlsPatchRawProperties(adapters.contentControls, input, options); + }, + validateWordCompatibility(input) { + return executeContentControlsValidateWordCompatibility(adapters.contentControls, input); + }, + normalizeWordCompatibility(input, options) { + return executeContentControlsNormalizeWordCompatibility(adapters.contentControls, input, options); + }, + normalizeTagPayload(input, options) { + return executeContentControlsNormalizeTagPayload(adapters.contentControls, input, options); + }, text: { - setMultiline: (input, options) => - executeContentControlsTextSetMultiline(adapters.contentControls, input, options), - setValue: (input, options) => executeContentControlsTextSetValue(adapters.contentControls, input, options), - clearValue: (input, options) => executeContentControlsTextClearValue(adapters.contentControls, input, options), + setMultiline(input, options) { + return executeContentControlsTextSetMultiline(adapters.contentControls, input, options); + }, + setValue(input, options) { + return executeContentControlsTextSetValue(adapters.contentControls, input, options); + }, + clearValue(input, options) { + return executeContentControlsTextClearValue(adapters.contentControls, input, options); + }, }, date: { - setValue: (input, options) => executeContentControlsDateSetValue(adapters.contentControls, input, options), - clearValue: (input, options) => executeContentControlsDateClearValue(adapters.contentControls, input, options), - setDisplayFormat: (input, options) => - executeContentControlsDateSetDisplayFormat(adapters.contentControls, input, options), - setDisplayLocale: (input, options) => - executeContentControlsDateSetDisplayLocale(adapters.contentControls, input, options), - setStorageFormat: (input, options) => - executeContentControlsDateSetStorageFormat(adapters.contentControls, input, options), - setCalendar: (input, options) => - executeContentControlsDateSetCalendar(adapters.contentControls, input, options), + setValue(input, options) { + return executeContentControlsDateSetValue(adapters.contentControls, input, options); + }, + clearValue(input, options) { + return executeContentControlsDateClearValue(adapters.contentControls, input, options); + }, + setDisplayFormat(input, options) { + return executeContentControlsDateSetDisplayFormat(adapters.contentControls, input, options); + }, + setDisplayLocale(input, options) { + return executeContentControlsDateSetDisplayLocale(adapters.contentControls, input, options); + }, + setStorageFormat(input, options) { + return executeContentControlsDateSetStorageFormat(adapters.contentControls, input, options); + }, + setCalendar(input, options) { + return executeContentControlsDateSetCalendar(adapters.contentControls, input, options); + }, }, checkbox: { - getState: (input) => executeContentControlsCheckboxGetState(adapters.contentControls, input), - setState: (input, options) => executeContentControlsCheckboxSetState(adapters.contentControls, input, options), - toggle: (input, options) => executeContentControlsCheckboxToggle(adapters.contentControls, input, options), - setSymbolPair: (input, options) => - executeContentControlsCheckboxSetSymbolPair(adapters.contentControls, input, options), + getState(input) { + return executeContentControlsCheckboxGetState(adapters.contentControls, input); + }, + setState(input, options) { + return executeContentControlsCheckboxSetState(adapters.contentControls, input, options); + }, + toggle(input, options) { + return executeContentControlsCheckboxToggle(adapters.contentControls, input, options); + }, + setSymbolPair(input, options) { + return executeContentControlsCheckboxSetSymbolPair(adapters.contentControls, input, options); + }, }, choiceList: { - getItems: (input) => executeContentControlsChoiceListGetItems(adapters.contentControls, input), - setItems: (input, options) => - executeContentControlsChoiceListSetItems(adapters.contentControls, input, options), - setSelected: (input, options) => - executeContentControlsChoiceListSetSelected(adapters.contentControls, input, options), + getItems(input) { + return executeContentControlsChoiceListGetItems(adapters.contentControls, input); + }, + setItems(input, options) { + return executeContentControlsChoiceListSetItems(adapters.contentControls, input, options); + }, + setSelected(input, options) { + return executeContentControlsChoiceListSetSelected(adapters.contentControls, input, options); + }, }, repeatingSection: { - listItems: (input) => executeContentControlsRepeatingSectionListItems(adapters.contentControls, input), - insertItemBefore: (input, options) => - executeContentControlsRepeatingSectionInsertItemBefore(adapters.contentControls, input, options), - insertItemAfter: (input, options) => - executeContentControlsRepeatingSectionInsertItemAfter(adapters.contentControls, input, options), - cloneItem: (input, options) => - executeContentControlsRepeatingSectionCloneItem(adapters.contentControls, input, options), - deleteItem: (input, options) => - executeContentControlsRepeatingSectionDeleteItem(adapters.contentControls, input, options), - setAllowInsertDelete: (input, options) => - executeContentControlsRepeatingSectionSetAllowInsertDelete(adapters.contentControls, input, options), + listItems(input) { + return executeContentControlsRepeatingSectionListItems(adapters.contentControls, input); + }, + insertItemBefore(input, options) { + return executeContentControlsRepeatingSectionInsertItemBefore(adapters.contentControls, input, options); + }, + insertItemAfter(input, options) { + return executeContentControlsRepeatingSectionInsertItemAfter(adapters.contentControls, input, options); + }, + cloneItem(input, options) { + return executeContentControlsRepeatingSectionCloneItem(adapters.contentControls, input, options); + }, + deleteItem(input, options) { + return executeContentControlsRepeatingSectionDeleteItem(adapters.contentControls, input, options); + }, + setAllowInsertDelete(input, options) { + return executeContentControlsRepeatingSectionSetAllowInsertDelete(adapters.contentControls, input, options); + }, }, group: { - wrap: (input, options) => executeContentControlsGroupWrap(adapters.contentControls, input, options), - ungroup: (input, options) => executeContentControlsGroupUngroup(adapters.contentControls, input, options), + wrap(input, options) { + return executeContentControlsGroupWrap(adapters.contentControls, input, options); + }, + ungroup(input, options) { + return executeContentControlsGroupUngroup(adapters.contentControls, input, options); + }, }, }, @@ -2800,16 +2904,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { }, query: { match(input: QueryMatchInput | TextSelector | NodeSelector): QueryMatchOutput { - if (!input || typeof input !== 'object') { - throw new DocumentApiValidationError( - 'INVALID_INPUT', - 'query.match requires a QueryMatchInput or selector object.', - { value: input }, - ); - } - // Normalize flat selector shorthand to canonical nested form. - const normalized: QueryMatchInput = 'select' in input ? input : { select: input }; - return adapters.query.match(normalized); + return executeQueryMatch(adapters.query, input); }, }, ranges: { diff --git a/packages/document-api/src/index/index.ts b/packages/document-api/src/index/index.ts index 75dcd07ebe..cf7aa87dd0 100644 --- a/packages/document-api/src/index/index.ts +++ b/packages/document-api/src/index/index.ts @@ -1,6 +1,7 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; import { DocumentApiValidationError } from '../errors.js'; +import { assertTargetPresent } from '../validation-primitives.js'; import type { IndexAddress, IndexEntryAddress, @@ -50,9 +51,7 @@ export type IndexAdapter = IndexApi; // --------------------------------------------------------------------------- function validateIndexTarget(target: unknown, operationName: string): asserts target is IndexAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'block' || t.nodeType !== 'index' || typeof t.nodeId !== 'string') { @@ -65,9 +64,7 @@ function validateIndexTarget(target: unknown, operationName: string): asserts ta } function validateIndexEntryTarget(target: unknown, operationName: string): asserts target is IndexEntryAddress { - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); - } + assertTargetPresent(target, operationName); const t = target as Record; if (t.kind !== 'inline' || t.nodeType !== 'indexEntry') { diff --git a/packages/document-api/src/index/index.types.ts b/packages/document-api/src/index/index.types.ts index 9ef9f13e41..c0487e2a3a 100644 --- a/packages/document-api/src/index/index.types.ts +++ b/packages/document-api/src/index/index.types.ts @@ -1,6 +1,6 @@ import type { InlineAnchor, BlockNodeAddress } from '../types/base.js'; import type { TextTarget } from '../types/address.js'; -import type { ReceiptFailure } from '../types/receipt.js'; +import type { AdapterMutationFailure } from '../types/adapter-result.js'; import type { DiscoveryOutput } from '../types/discovery.js'; import type { TocCreateLocation } from '../toc/toc.types.js'; @@ -163,24 +163,14 @@ export interface IndexMutationSuccess { index: IndexAddress; } -export interface IndexMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type IndexMutationResult = IndexMutationSuccess | IndexMutationFailure; +export type IndexMutationResult = IndexMutationSuccess | AdapterMutationFailure; export interface IndexEntryMutationSuccess { success: true; entry: IndexEntryAddress; } -export interface IndexEntryMutationFailure { - success: false; - failure: ReceiptFailure; -} - -export type IndexEntryMutationResult = IndexEntryMutationSuccess | IndexEntryMutationFailure; +export type IndexEntryMutationResult = IndexEntryMutationSuccess | AdapterMutationFailure; // --------------------------------------------------------------------------- // List results diff --git a/packages/document-api/src/insert/insert.ts b/packages/document-api/src/insert/insert.ts index cb401071c0..d6483c46f6 100644 --- a/packages/document-api/src/insert/insert.ts +++ b/packages/document-api/src/insert/insert.ts @@ -1,4 +1,4 @@ -import { executeWrite, type MutationOptions, type WriteAdapter } from '../write/write.js'; +import { executeWrite, normalizeMutationOptions, type MutationOptions, type WriteAdapter } from '../write/write.js'; import type { TextAddress, TextMutationReceipt, SDMutationReceipt } from '../types/index.js'; import type { SDInsertInput } from '../types/structural-input.js'; import type { SDFragment } from '../types/fragment.js'; @@ -197,7 +197,7 @@ export function executeInsert(adapter: WriteAdapter, input: InsertInput, options // Structural content path — returns SDMutationReceipt directly if (isStructuralInsertInput(input)) { - return adapter.insertStructured(input as unknown as LegacyInsertInput, options); + return adapter.insertStructured(input, normalizeMutationOptions(options)); } // Legacy string path @@ -206,7 +206,7 @@ export function executeInsert(adapter: WriteAdapter, input: InsertInput, options // For non-text content types, delegate to the adapter's structured insert path. if (contentType !== 'text') { - return adapter.insertStructured(input, options); + return adapter.insertStructured(input, normalizeMutationOptions(options)); } // Text path: use the existing write pipeline, wrap TextMutationReceipt → SDMutationReceipt diff --git a/packages/document-api/src/invoke/invoke.ts b/packages/document-api/src/invoke/invoke.ts index d83d4ad772..f85d1142ba 100644 --- a/packages/document-api/src/invoke/invoke.ts +++ b/packages/document-api/src/invoke/invoke.ts @@ -267,7 +267,7 @@ export function buildDispatchTable(api: DocumentApi): TypedDispatchTable { 'create.image': (input, options) => api.create.image(input, options), // --- images.* --- - 'images.list': (input) => api.images.list(input ?? {}), + 'images.list': (input) => api.images.list(input), 'images.get': (input) => api.images.get(input), 'images.delete': (input, options) => api.images.delete(input, options), 'images.move': (input, options) => api.images.move(input, options), diff --git a/packages/document-api/src/replace/replace.ts b/packages/document-api/src/replace/replace.ts index c11abe45dc..7e3e63f2b2 100644 --- a/packages/document-api/src/replace/replace.ts +++ b/packages/document-api/src/replace/replace.ts @@ -211,7 +211,7 @@ export function executeReplace( // Structural content path — returns SDMutationReceipt directly if (isStructuralReplaceInput(input)) { - return writeAdapter.replaceStructured(input as unknown as ReplaceInput, options); + return writeAdapter.replaceStructured(input as unknown as ReplaceInput, normalizeMutationOptions(options)); } // Text replacement path — route through SelectionMutationAdapter diff --git a/packages/document-api/src/selection-mutation.ts b/packages/document-api/src/selection-mutation.ts index 79011d7180..fe2fb7f5f2 100644 --- a/packages/document-api/src/selection-mutation.ts +++ b/packages/document-api/src/selection-mutation.ts @@ -3,8 +3,8 @@ * `delete`, `replace` (text path), and `format.apply` in the new * SelectionTarget / ref model. * - * This replaces the WriteAdapter for delete/replace and the FormatAdapter - * for format.apply. All three operations route through the plan engine. + * This replaces the WriteAdapter for delete/replace and the legacy format + * adapter for format.apply. All three operations route through the plan engine. */ import type { SelectionTarget, DeleteBehavior } from './types/address.js'; diff --git a/packages/document-api/src/styles/apply.ts b/packages/document-api/src/styles/apply.ts index e854f85517..9a95453061 100644 --- a/packages/document-api/src/styles/apply.ts +++ b/packages/document-api/src/styles/apply.ts @@ -27,6 +27,7 @@ export type StylesArrayState = unknown[] | 'inherit'; // Patch Types // --------------------------------------------------------------------------- +// SYNC: StylesRunPatch must match PROPERTY_REGISTRY run keys in registry.ts /** Patch for run-channel properties (docDefaults/w:rPrDefault/w:rPr). */ export interface StylesRunPatch { // Booleans @@ -70,6 +71,7 @@ export interface StylesRunPatch { fitText?: Record; } +// SYNC: StylesParagraphPatch must match PROPERTY_REGISTRY paragraph keys in registry.ts /** Patch for paragraph-channel properties (docDefaults/w:pPrDefault/w:pPr). */ export interface StylesParagraphPatch { // Booleans diff --git a/packages/document-api/src/styles/index.ts b/packages/document-api/src/styles/index.ts index a662cccbdf..40d38105c7 100644 --- a/packages/document-api/src/styles/index.ts +++ b/packages/document-api/src/styles/index.ts @@ -48,5 +48,6 @@ export type { } from './apply.js'; export { executeStylesApply } from './apply.js'; -// Validation: exported for adapter use (excluded-key checking) -export { validateValue } from './validation.js'; +// Validation: exported for adapter use (excluded-key checking, patch key classification) +export type { PatchKeyClassification } from './validation.js'; +export { validateValue, classifyPatchKey } from './validation.js'; diff --git a/packages/document-api/src/styles/validation.ts b/packages/document-api/src/styles/validation.ts index f9ab3c1862..224a0c00a1 100644 --- a/packages/document-api/src/styles/validation.ts +++ b/packages/document-api/src/styles/validation.ts @@ -8,7 +8,7 @@ import { DocumentApiValidationError } from '../errors.js'; import { isRecord } from '../validation-primitives.js'; import type { ValueSchema, StylesChannel } from './registry.js'; -import { PROPERTY_REGISTRY, ALLOWED_KEYS_BY_CHANNEL, EXCLUDED_KEYS, getPropertyDefinition } from './registry.js'; +import { ALLOWED_KEYS_BY_CHANNEL, EXCLUDED_KEYS, getPropertyDefinition } from './registry.js'; // --------------------------------------------------------------------------- // Recursive ValueSchema validation @@ -198,46 +198,34 @@ export function validateStylesApplyInput(input: unknown): asserts input is Style } const allowedKeys = ALLOWED_KEYS_BY_CHANNEL[channel]; - const otherChannel: StylesChannel = channel === 'run' ? 'paragraph' : 'run'; for (const key of patchKeys) { - // 1. Check excluded keys first - const excludedEntry = EXCLUDED_KEYS[channel].get(key); - if (excludedEntry !== undefined) { - throw new DocumentApiValidationError( - 'INVALID_INPUT', - `patch key '${key}' is not valid in Word docDefaults (${excludedEntry}). This is an intentional restriction per MS-OI29500.`, - { field: 'patch', key, reason: 'excluded_docdefaults_key' }, - ); - } + const classification = classifyPatchKey(key, channel); - // 2. Check cross-channel - if (!allowedKeys.has(key)) { - const belongsToOther = ALLOWED_KEYS_BY_CHANNEL[otherChannel].has(key); - if (belongsToOther) { + switch (classification.status) { + case 'valid': + break; + + case 'excluded': throw new DocumentApiValidationError( 'INVALID_INPUT', - `Unknown patch key "${key}" for channel "${channel}". "${key}" is a ${otherChannel}-channel property. Allowed keys: ${[...allowedKeys].join(', ')}.`, - { field: 'patch', key }, + `patch key '${key}' is not valid in Word docDefaults (${classification.reason}). This is an intentional restriction per MS-OI29500.`, + { field: 'patch', key, reason: 'excluded_docdefaults_key' }, ); - } - // 3. Check excluded on other channel - const otherExcluded = EXCLUDED_KEYS[otherChannel].get(key); - if (otherExcluded !== undefined) { + case 'cross_channel': throw new DocumentApiValidationError( 'INVALID_INPUT', - `patch key '${key}' is not valid in Word docDefaults (${otherExcluded}). This is an intentional restriction per MS-OI29500.`, - { field: 'patch', key, reason: 'excluded_docdefaults_key' }, + `Unknown patch key "${key}" for channel "${channel}". "${key}" is a ${classification.ownerChannel}-channel property. Allowed keys: ${[...allowedKeys].join(', ')}.`, + { field: 'patch', key }, ); - } - // 4. Completely unknown - throw new DocumentApiValidationError( - 'INVALID_INPUT', - `Unknown patch key "${key}" for channel "${channel}". Allowed keys: ${[...allowedKeys].join(', ')}.`, - { field: 'patch', key }, - ); + case 'unknown': + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `Unknown patch key "${key}" for channel "${channel}". Allowed keys: ${[...allowedKeys].join(', ')}.`, + { field: 'patch', key }, + ); } // Validate the value against the registry schema @@ -278,6 +266,52 @@ export function validateStylesApplyOptions(options: unknown): void { } } +// --------------------------------------------------------------------------- +// Patch key classification +// --------------------------------------------------------------------------- + +/** Discriminated union returned by `classifyPatchKey`. */ +export type PatchKeyClassification = + | { status: 'valid' } + | { status: 'excluded'; reason: string } + | { status: 'cross_channel'; ownerChannel: StylesChannel } + | { status: 'unknown' }; + +/** + * Classifies a patch key relative to a given channel. + * + * Returns a discriminated union so callers can switch on `status` instead of + * nesting conditionals across excluded-key maps, allowed-key sets, and + * cross-channel lookups. + */ +export function classifyPatchKey(key: string, channel: StylesChannel): PatchKeyClassification { + // 1. Excluded on the requested channel + const excludedReason = EXCLUDED_KEYS[channel].get(key); + if (excludedReason !== undefined) { + return { status: 'excluded', reason: excludedReason }; + } + + // 2. Valid for the requested channel + if (ALLOWED_KEYS_BY_CHANNEL[channel].has(key)) { + return { status: 'valid' }; + } + + // 3. Belongs to the other channel + const otherChannel: StylesChannel = channel === 'run' ? 'paragraph' : 'run'; + if (ALLOWED_KEYS_BY_CHANNEL[otherChannel].has(key)) { + return { status: 'cross_channel', ownerChannel: otherChannel }; + } + + // 4. Excluded on the other channel (still an exclusion, not "unknown") + const otherExcludedReason = EXCLUDED_KEYS[otherChannel].get(key); + if (otherExcludedReason !== undefined) { + return { status: 'excluded', reason: otherExcludedReason }; + } + + // 5. Not in any registry + return { status: 'unknown' }; +} + // --------------------------------------------------------------------------- // Internal helpers // --------------------------------------------------------------------------- diff --git a/packages/document-api/src/tables/tables.ts b/packages/document-api/src/tables/tables.ts index 00c109e1f5..3aaf8957f0 100644 --- a/packages/document-api/src/tables/tables.ts +++ b/packages/document-api/src/tables/tables.ts @@ -101,99 +101,83 @@ function validateRowLocator( } // --------------------------------------------------------------------------- -// Locator category helpers — determine which validation to apply per operation +// Typed execute helpers — one per locator category // --------------------------------------------------------------------------- -type TableLocatorInput = { target?: unknown; nodeId?: unknown }; -type TableScopedInput = { tableTarget?: unknown; tableNodeId?: unknown }; -type RowLocatorInput = TableLocatorInput & TableScopedInput & { rowIndex?: unknown }; - /** - * Operations using the simple table locator (target/nodeId). + * Execute a table operation that uses the simple table locator (target/nodeId). + * Validates the locator and normalizes MutationOptions. */ -const TABLE_LOCATOR_OPS = new Set([ - 'tables.delete', - 'tables.clearContents', - 'tables.move', - 'tables.split', - 'tables.convertFromText', - 'tables.convertToText', - 'tables.setLayout', - 'tables.distributeRows', - 'tables.distributeColumns', - 'tables.sort', - 'tables.setAltText', - 'tables.setStyle', - 'tables.clearStyle', - 'tables.setStyleOption', - 'tables.setBorder', - 'tables.clearBorder', - 'tables.applyBorderPreset', - 'tables.setShading', - 'tables.clearShading', - 'tables.setTablePadding', - 'tables.setCellPadding', - 'tables.setCellSpacing', - 'tables.clearCellSpacing', - 'tables.unmergeCells', - 'tables.insertCell', - 'tables.deleteCell', - 'tables.splitCell', - 'tables.setCellProperties', - 'tables.get', - 'tables.getCells', - 'tables.getProperties', -]); +export function executeTableLocatorOp( + operationName: string, + adapter: (input: TInput, options?: MutationOptions) => TResult, + input: TInput, + options?: MutationOptions, +): TResult { + validateTableLocator(input, operationName); + return adapter(input, normalizeMutationOptions(options)); +} /** - * Operations using the mixed row locator (direct OR table-scoped). + * Execute a table operation that uses the mixed row locator + * (direct target/nodeId OR table-scoped tableTarget/tableNodeId + rowIndex). + * Validates the locator and normalizes MutationOptions. */ -const ROW_LOCATOR_OPS = new Set([ - 'tables.insertRow', - 'tables.deleteRow', - 'tables.setRowHeight', - 'tables.setRowOptions', -]); +export function executeRowLocatorOp< + TInput extends { + target?: unknown; + nodeId?: unknown; + tableTarget?: unknown; + tableNodeId?: unknown; + rowIndex?: unknown; + }, + TResult, +>( + operationName: string, + adapter: (input: TInput, options?: MutationOptions) => TResult, + input: TInput, + options?: MutationOptions, +): TResult { + validateRowLocator(input, operationName); + return adapter(input, normalizeMutationOptions(options)); +} /** - * Operations using a table-scoped column locator (tableTarget/tableNodeId). + * Execute a table operation that uses a table-scoped column locator + * (tableTarget/tableNodeId). Validates the locator and normalizes MutationOptions. */ -const COLUMN_LOCATOR_OPS = new Set(['tables.insertColumn', 'tables.deleteColumn', 'tables.setColumnWidth']); +export function executeColumnLocatorOp( + operationName: string, + adapter: (input: TInput, options?: MutationOptions) => TResult, + input: TInput, + options?: MutationOptions, +): TResult { + validateTableScopedLocator(input, operationName); + return adapter(input, normalizeMutationOptions(options)); +} /** - * Operations using a merge range locator (tableTarget/tableNodeId). + * Execute a table operation that uses a merge-range locator + * (tableTarget/tableNodeId). Validates the locator and normalizes MutationOptions. */ -const MERGE_RANGE_LOCATOR_OPS = new Set(['tables.mergeCells']); - -// --------------------------------------------------------------------------- -// Generic execute wrapper -// --------------------------------------------------------------------------- +export function executeMergeRangeLocatorOp( + operationName: string, + adapter: (input: TInput, options?: MutationOptions) => TResult, + input: TInput, + options?: MutationOptions, +): TResult { + validateTableScopedLocator(input, operationName); + return adapter(input, normalizeMutationOptions(options)); +} /** - * Validates the input locator for a table operation and normalizes MutationOptions. - * - * @param operationName - The operation ID (e.g. 'tables.delete') - * @param adapter - The adapter method to call - * @param input - The raw input from the caller - * @param options - Optional mutation options to normalize - * @returns The adapter return value + * Execute a document-level table mutation (no locator validation needed). + * Only normalizes MutationOptions. */ -export function executeTableOperation( - operationName: string, +export function executeDocumentLevelTableOp( adapter: (input: TInput, options?: MutationOptions) => TResult, input: TInput, options?: MutationOptions, ): TResult { - // Validate locator based on operation category - if (TABLE_LOCATOR_OPS.has(operationName)) { - validateTableLocator(input as TableLocatorInput, operationName); - } else if (ROW_LOCATOR_OPS.has(operationName)) { - validateRowLocator(input as RowLocatorInput, operationName); - } else if (COLUMN_LOCATOR_OPS.has(operationName)) { - validateTableScopedLocator(input as TableScopedInput, operationName); - } else if (MERGE_RANGE_LOCATOR_OPS.has(operationName)) { - validateTableScopedLocator(input as TableScopedInput, operationName); - } - return adapter(input, normalizeMutationOptions(options)); } diff --git a/packages/document-api/src/track-changes/track-changes.ts b/packages/document-api/src/track-changes/track-changes.ts index d39987b5b1..d227f3c2cd 100644 --- a/packages/document-api/src/track-changes/track-changes.ts +++ b/packages/document-api/src/track-changes/track-changes.ts @@ -69,41 +69,13 @@ export function executeTrackChangesGet(adapter: TrackChangesAdapter, input: Trac return adapter.get(input); } -export function executeTrackChangesAccept( - adapter: TrackChangesAdapter, - input: TrackChangesAcceptInput, - options?: RevisionGuardOptions, -): Receipt { - return adapter.accept(input, options); -} - -export function executeTrackChangesReject( - adapter: TrackChangesAdapter, - input: TrackChangesRejectInput, - options?: RevisionGuardOptions, -): Receipt { - return adapter.reject(input, options); -} - -export function executeTrackChangesAcceptAll( - adapter: TrackChangesAdapter, - input: TrackChangesAcceptAllInput, - options?: RevisionGuardOptions, -): Receipt { - return adapter.acceptAll(input, options); -} - -export function executeTrackChangesRejectAll( - adapter: TrackChangesAdapter, - input: TrackChangesRejectAllInput, - options?: RevisionGuardOptions, -): Receipt { - return adapter.rejectAll(input, options); -} - /** * Executes the consolidated `trackChanges.decide` operation by routing to the * appropriate adapter method based on the discriminated input. + * + * Accepting/rejecting changes is a resolution action, not a content mutation — + * changeMode and dryRun are not applicable, so this accepts + * {@link RevisionGuardOptions} rather than `MutationOptions`. */ export function executeTrackChangesDecide( adapter: TrackChangesAdapter, diff --git a/packages/document-api/src/types/adapter-result.ts b/packages/document-api/src/types/adapter-result.ts new file mode 100644 index 0000000000..9b6b655dd9 --- /dev/null +++ b/packages/document-api/src/types/adapter-result.ts @@ -0,0 +1,11 @@ +import type { ReceiptFailure } from './receipt.js'; + +/** + * Shared failure shape for optional adapter namespace mutation results. + * Per-entity success types vary (different property names for entity addresses), + * but all failure types share this identical shape. + */ +export interface AdapterMutationFailure { + success: false; + failure: ReceiptFailure; +} diff --git a/packages/document-api/src/types/address.ts b/packages/document-api/src/types/address.ts index bdeac0fff7..9b8bec596c 100644 --- a/packages/document-api/src/types/address.ts +++ b/packages/document-api/src/types/address.ts @@ -43,7 +43,7 @@ export type TextTarget = { }; // --------------------------------------------------------------------------- -// Selection-based mutation targeting (v1) +// Selection-based mutation targeting // --------------------------------------------------------------------------- /** diff --git a/packages/document-api/src/types/index.ts b/packages/document-api/src/types/index.ts index 2e63d02e85..119983b210 100644 --- a/packages/document-api/src/types/index.ts +++ b/packages/document-api/src/types/index.ts @@ -23,4 +23,5 @@ export * from './blocks.types.js'; export * from './toc.types.js'; export * from './fragment.js'; export * from './placement.js'; +export * from './adapter-result.js'; export * from './structural-input.js'; diff --git a/packages/document-api/src/types/mutation-plan.types.ts b/packages/document-api/src/types/mutation-plan.types.ts index 611af81fb1..70a0975db2 100644 --- a/packages/document-api/src/types/mutation-plan.types.ts +++ b/packages/document-api/src/types/mutation-plan.types.ts @@ -177,7 +177,8 @@ export type MutationStep = // Plan input // --------------------------------------------------------------------------- -export type ChangeMode = 'direct' | 'tracked'; +import type { ChangeMode } from '../write/write.js'; +export type { ChangeMode } from '../write/write.js'; export type MutationsApplyInput = { expectedRevision?: string; @@ -328,14 +329,7 @@ export type PlanExecutionError = { }; // --------------------------------------------------------------------------- -// Revision guard options +// Revision guard options — canonical definitions in write/write.ts // --------------------------------------------------------------------------- -export type RevisionGuardOptions = { - expectedRevision?: string; -}; - -export type MutationOptions = RevisionGuardOptions & { - changeMode?: ChangeMode; - dryRun?: boolean; -}; +export type { RevisionGuardOptions, MutationOptions } from '../write/write.js'; diff --git a/packages/document-api/src/validation-primitives.test.ts b/packages/document-api/src/validation-primitives.test.ts index 04045bd173..a5879c90ee 100644 --- a/packages/document-api/src/validation-primitives.test.ts +++ b/packages/document-api/src/validation-primitives.test.ts @@ -1,11 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { - isRecord, - isInteger, - isTextAddress, - assertNoUnknownFields, - assertNonNegativeInteger, -} from './validation-primitives.js'; +import { isRecord, isInteger, isTextAddress, assertNoUnknownFields } from './validation-primitives.js'; import { DocumentApiValidationError } from './errors.js'; describe('isRecord', () => { @@ -95,7 +89,7 @@ describe('assertNoUnknownFields', () => { expect(() => assertNoUnknownFields({}, allowlist, 'test')).not.toThrow(); }); - it('throws INVALID_TARGET for unknown fields', () => { + it('throws INVALID_INPUT for unknown fields', () => { const allowlist = new Set(['a']); try { assertNoUnknownFields({ a: 1, unknown: 2 }, allowlist, 'test'); @@ -103,57 +97,9 @@ describe('assertNoUnknownFields', () => { } catch (err) { expect(err).toBeInstanceOf(DocumentApiValidationError); const e = err as DocumentApiValidationError; - expect(e.code).toBe('INVALID_TARGET'); + expect(e.code).toBe('INVALID_INPUT'); expect(e.message).toContain('Unknown field "unknown"'); expect(e.message).toContain('test'); } }); }); - -describe('assertNonNegativeInteger', () => { - it('does not throw for valid non-negative integers', () => { - expect(() => assertNonNegativeInteger(0, 'offset')).not.toThrow(); - expect(() => assertNonNegativeInteger(10, 'offset')).not.toThrow(); - }); - - it('throws for negative numbers', () => { - try { - assertNonNegativeInteger(-1, 'offset'); - expect.fail('Should have thrown'); - } catch (err) { - expect(err).toBeInstanceOf(DocumentApiValidationError); - const e = err as DocumentApiValidationError; - expect(e.code).toBe('INVALID_TARGET'); - expect(e.message).toContain('non-negative integer'); - } - }); - - it('throws for non-integer numbers', () => { - try { - assertNonNegativeInteger(1.5, 'start'); - expect.fail('Should have thrown'); - } catch (err) { - expect(err).toBeInstanceOf(DocumentApiValidationError); - expect((err as DocumentApiValidationError).code).toBe('INVALID_TARGET'); - } - }); - - it('throws for non-numbers', () => { - try { - assertNonNegativeInteger('5', 'end'); - expect.fail('Should have thrown'); - } catch (err) { - expect(err).toBeInstanceOf(DocumentApiValidationError); - expect((err as DocumentApiValidationError).code).toBe('INVALID_TARGET'); - } - }); - - it('includes field name in error message', () => { - try { - assertNonNegativeInteger(-1, 'myField'); - expect.fail('Should have thrown'); - } catch (err) { - expect((err as DocumentApiValidationError).message).toContain('myField'); - } - }); -}); diff --git a/packages/document-api/src/validation-primitives.ts b/packages/document-api/src/validation-primitives.ts index 63c8d1e1e7..e187d9af52 100644 --- a/packages/document-api/src/validation-primitives.ts +++ b/packages/document-api/src/validation-primitives.ts @@ -13,6 +13,16 @@ import { BLOCK_NODE_TYPES } from './types/base.js'; import { TABLE_NESTING_POLICY_VALUES } from './types/placement.js'; import { DocumentApiValidationError } from './errors.js'; +/** + * Throws INVALID_TARGET if target is null or undefined. + * Shared preamble for optional adapter namespace validators. + */ +export function assertTargetPresent(target: unknown, operationName: string): void { + if (target === undefined || target === null) { + throw new DocumentApiValidationError('INVALID_TARGET', `${operationName} requires a target.`); + } +} + export function isRecord(value: unknown): value is Record { return typeof value === 'object' && value != null && !Array.isArray(value); } @@ -44,7 +54,8 @@ export function isBlockNodeAddress(value: unknown): value is BlockNodeAddress { } /** - * Throws INVALID_TARGET if any key on the input object is not in the allowlist. + * Throws INVALID_INPUT if any key on the input object is not in the allowlist. + * Unknown fields are a payload shape issue, not a locator problem. */ export function assertNoUnknownFields( input: Record, @@ -54,7 +65,7 @@ export function assertNoUnknownFields( for (const key of Object.keys(input)) { if (!allowlist.has(key)) { throw new DocumentApiValidationError( - 'INVALID_TARGET', + 'INVALID_INPUT', `Unknown field "${key}" on ${operationName} input. Allowed fields: ${[...allowlist].join(', ')}.`, { field: key }, ); @@ -62,19 +73,6 @@ export function assertNoUnknownFields( } } -/** - * Throws INVALID_TARGET if the value is not a non-negative integer. - */ -export function assertNonNegativeInteger(value: unknown, fieldName: string): void { - if (typeof value !== 'number' || !Number.isInteger(value) || value < 0) { - throw new DocumentApiValidationError( - 'INVALID_TARGET', - `${fieldName} must be a non-negative integer, got ${JSON.stringify(value)}.`, - { field: fieldName, value }, - ); - } -} - const NESTING_POLICY_ALLOWED_KEYS: ReadonlySet = new Set(['tables']); /** diff --git a/packages/document-api/src/write/write.ts b/packages/document-api/src/write/write.ts index d64d783ea3..6ba2a0ce60 100644 --- a/packages/document-api/src/write/write.ts +++ b/packages/document-api/src/write/write.ts @@ -5,6 +5,13 @@ import type { ReplaceInput } from '../replace/replace.js'; export type ChangeMode = 'direct' | 'tracked'; +/** + * Subset of MutationOptions that provides only revision guarding. + * + * Used by operations that don't participate in the plan engine (comments, + * clearContent, trackChanges.decide) where changeMode and dryRun are not + * applicable. + */ export interface RevisionGuardOptions { /** When provided, the engine rejects with REVISION_MISMATCH if the document has advanced past this revision. */ expectedRevision?: string; @@ -37,7 +44,10 @@ export type InsertWriteRequest = { text: string; } & Partial; -/** @deprecated Use `InsertWriteRequest` directly. Delete and replace now use SelectionMutationAdapter. */ +/** + * Alias for `InsertWriteRequest`. Retained because super-editor adapter-utils + * and plan-wrappers still reference this name. + */ export type WriteRequest = InsertWriteRequest; /** diff --git a/packages/super-editor/src/document-api-adapters/assemble-adapters.ts b/packages/super-editor/src/document-api-adapters/assemble-adapters.ts index 11196fa17a..a13dc3c821 100644 --- a/packages/super-editor/src/document-api-adapters/assemble-adapters.ts +++ b/packages/super-editor/src/document-api-adapters/assemble-adapters.ts @@ -1,7 +1,7 @@ import type { DocumentApiAdapters } from '@superdoc/document-api'; import type { Editor } from '../core/Editor.js'; import { getAdapter } from './get-adapter.js'; -import { sdFindAdapter, findLegacyAdapter } from './find-adapter.js'; +import { sdFindAdapter } from './find-adapter.js'; import { getNodeAdapter, getNodeByIdAdapter } from './get-node-adapter.js'; import { getTextAdapter } from './get-text-adapter.js'; import { getMarkdownAdapter } from './get-markdown-adapter.js'; @@ -333,7 +333,6 @@ export function assembleDocumentApiAdapters(editor: Editor): DocumentApiAdapters }, find: { find: (input) => sdFindAdapter(editor, input), - findLegacy: (query) => findLegacyAdapter(editor, query), }, getNode: { getNode: (address) => getNodeAdapter(editor, address), From 859dd4dc03f7094adacacce8e8d1d3ae294fcff5 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 17 Mar 2026 17:13:05 -0700 Subject: [PATCH 2/4] fix(cli): correct SKILL.md and README payload flags, workflows, and command references --- apps/cli/README.md | 9 ++--- apps/cli/skill/SKILL.md | 79 ++++++++++++++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/apps/cli/README.md b/apps/cli/README.md index e535db767c..5545a8156f 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -311,10 +311,11 @@ superdoc info ./contract.docx --pretty ## Input payload flags -- `--query-json`, `--query-file` -- `--address-json`, `--address-file` -- `--target-json`, `--target-file` -- `--at-json`, `--at-file` (for `create paragraph`) +- `--query-json`, `--query-file` (`find`, `lists list`) +- `--address-json`, `--address-file` (`get-node`, `lists get`) +- `--target-json` (mutation commands — no `--target-file` counterpart; use flat flags `--block-id`/`--start`/`--end` as alternative) +- `--input-json`, `--input-file` (`call`, `create paragraph`) +- `--at-json`, `--at-file` (`create paragraph`) ## Stdin support diff --git a/apps/cli/skill/SKILL.md b/apps/cli/skill/SKILL.md index 7ad53a919d..9c90960042 100644 --- a/apps/cli/skill/SKILL.md +++ b/apps/cli/skill/SKILL.md @@ -5,7 +5,7 @@ description: Edit, query, and transform Word documents with the SuperDoc CLI v1 # SuperDoc CLI (v1) -Use SuperDoc CLI for DOCX work. Prefer canonical v1 commands. +Use SuperDoc CLI for DOCX work. Use v1 commands (canonical operations and their helper wrappers). Do not default to legacy commands unless explicitly needed for v0-style bulk workflows. Use `superdoc` if installed, or `npx @superdoc-dev/cli@latest` as a fallback. @@ -28,12 +28,13 @@ Use `describe command` for per-command args and constraints. ```bash superdoc open ./contract.docx -superdoc find --type text --pattern "termination" +superdoc query match --select-json '{"type":"text","pattern":"termination"}' --require exactlyOne superdoc replace --target-json '{"kind":"text","blockId":"p1","range":{"start":0,"end":11}}' --text "expiration" superdoc save --in-place superdoc close ``` +- Always use `query match` (not `find`) to discover mutation targets — it returns exact addresses with cardinality guarantees. - After `open`, commands run against the active/default session when `` is omitted. - Use `superdoc session list|set-default|save|close` for explicit session control. - `close` on dirty state requires `--discard` or a prior `save`. @@ -57,28 +58,80 @@ superdoc replace ./proposal.docx \ - In stateless mode (`` provided), mutating commands require `--out` unless using `--dry-run`. +### Safety: preview before apply + +- Use `--dry-run` to preview any mutation without applying it. +- Use `--expected-revision ` with stateful mutations for optimistic concurrency checks. + ## Common v1 Commands -- Search text/nodes: `find --type text --pattern "..."` or `find --query-json '{...}'` -- Replace text: `replace --target-json '{...}' --text "..."` -- Add/edit comments: `comments add|reply|edit|resolve|remove` -- Review tracked changes: `track-changes list|accept|reject|accept-all|reject-all` +### Query & inspect + +- Search/browse content: `find --type text --pattern "..."` or `find --query-json '{...}'` +- Find mutation target: `query match --select-json '{...}' --require exactlyOne` +- Inspect blocks: `blocks list`, `get-node`, `get-node-by-id` - Extract content: `get-text`, `get-markdown`, `get-html` -- Low-level direct invoke: `call --input-json '{...}'` + +### Mutate + +- Replace text: `replace --target-json '{...}' --text "..."` +- Insert inline text: `insert --block-id --offset --value "..."` +- Delete text/node: `delete --target-json '{...}'` +- Delete blocks: `blocks delete`, `blocks delete-range` +- Batch mutations: `mutations apply --steps-json '[...]' --atomic true --change-mode direct` +- Create paragraph: `create paragraph --text "..."` (with optional `--at-json`) +- Create heading: `create heading --input-json '{"level":,"text":"..."}'` + +### Format + +- Apply formatting: `format apply --block-id --start --end --inline-json '{"bold":true}'` +- Shortcuts: `format bold`, `format italic`, `format underline`, `format strikethrough` + +### Lists + +- List items: `lists list`, `lists get` +- Insert list item: `lists insert --node-id --position after --text "..."` +- Modify: `lists indent`, `lists outdent`, `lists set-level`, `lists set-type`, `lists convert-to-text` + +### Comments + +- Add/reply: `comments add`, `comments reply` +- Read: `comments get`, `comments list` +- Edit/resolve/move: `comments edit`, `comments resolve`, `comments move`, `comments set-internal` +- Delete: `comments delete` (canonical) or `comments remove` (alias) + +### Track changes + +- List: `track-changes list`, `track-changes get` +- Decide: `track-changes accept`, `track-changes reject`, `track-changes accept-all`, `track-changes reject-all` + +### History + +- `history get`, `history undo`, `history redo` + +### Low-level + +- Direct invoke: `call --input-json '{...}'` (JSON output only — `--pretty` is not supported) ## JSON/File Payload Flags -Use one of each pair (not both): +Not all `--*-file` variants are available on every command. Use `describe command ` to check. + +Always supported alongside their `-json` counterpart (use one, not both): + +| Flag pair | Available on | +|-----------|-------------| +| `--query-json` / `--query-file` | `find`, `lists list` | +| `--address-json` / `--address-file` | `get-node`, `lists get` | +| `--input-json` / `--input-file` | `call`, `create paragraph` | +| `--at-json` / `--at-file` | `create paragraph` | -- `--query-json` or `--query-file` -- `--target-json` or `--target-file` -- `--address-json` or `--address-file` -- `--input-json` or `--input-file` (for `call`) +`--target-json` is widely available on mutation commands but has **no** `--target-file` counterpart. Use flat flags (`--block-id`, `--start`, `--end`) as an alternative to `--target-json`. ## Output and Global Flags - Default output is JSON envelope. -- Use `--pretty` for human-readable output. +- Use `--pretty` for human-readable output (not supported by `call`). - Global flags: `--output `, `--session `, `--timeout-ms `. - `` can be `-` to read DOCX bytes from stdin. From 51b868551d4318ab0e73dd1ee1c1120e5cd8b6eb Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 17 Mar 2026 17:41:31 -0700 Subject: [PATCH 3/4] fix(cli): align runtime document api with current contract --- .../src/__tests__/lib/error-mapping.test.ts | 25 +++++++++++++++ apps/cli/src/lib/document.ts | 20 ++++++++++-- apps/cli/src/lib/error-mapping.ts | 11 ++++++- .../src/format/inline-run-patch.test.ts | 31 ++++++++++++++++++- .../src/format/inline-run-patch.ts | 4 +-- 5 files changed, 85 insertions(+), 6 deletions(-) diff --git a/apps/cli/src/__tests__/lib/error-mapping.test.ts b/apps/cli/src/__tests__/lib/error-mapping.test.ts index 685259ba0a..cbb0f165da 100644 --- a/apps/cli/src/__tests__/lib/error-mapping.test.ts +++ b/apps/cli/src/__tests__/lib/error-mapping.test.ts @@ -274,3 +274,28 @@ describe('mapFailedReceipt: plan-engine code passthrough', () => { expect(result!.code).toBe('INVALID_ARGUMENT'); }); }); + +// --------------------------------------------------------------------------- +// textMutation: INVALID_INPUT ordering — plan-engine must win over adapter remap +// --------------------------------------------------------------------------- + +describe('mapInvokeError: textMutation INVALID_INPUT ordering', () => { + test('plan-engine INVALID_INPUT passes through verbatim for text mutations', () => { + const error = Object.assign(new Error('step schema invalid'), { + code: 'INVALID_INPUT', + details: { + stepIndex: 0, + operation: 'text.rewrite', + remediation: 'Fix the step payload.', + }, + }); + + const result = mapInvokeError('format.inline.apply' as any, error); + expect(result).toBeInstanceOf(CliError); + // Must preserve INVALID_INPUT — not remap to INVALID_ARGUMENT + expect(result.code).toBe('INVALID_INPUT'); + expect(result.details).toMatchObject({ + details: { stepIndex: 0, operation: 'text.rewrite' }, + }); + }); +}); diff --git a/apps/cli/src/lib/document.ts b/apps/cli/src/lib/document.ts index b5293010a1..e74b1e7452 100644 --- a/apps/cli/src/lib/document.ts +++ b/apps/cli/src/lib/document.ts @@ -2,9 +2,10 @@ import { readFile, writeFile } from 'node:fs/promises'; import { createHash } from 'node:crypto'; import { Editor } from 'superdoc/super-editor'; import { BLANK_DOCX_BASE64 } from '@superdoc/super-editor/blank-docx'; +import { getDocumentApiAdapters } from '@superdoc/super-editor/document-api-adapters'; import { markdownToPmDoc } from '@superdoc/super-editor/markdown'; -import type { DocumentApi } from '@superdoc/document-api'; +import { createDocumentApi, type DocumentApi } from '@superdoc/document-api'; import { createCliDomEnvironment } from './dom-environment'; import type { CollaborationProfile } from './collaboration'; import { createCollaborationRuntime } from './collaboration'; @@ -65,6 +66,21 @@ export interface FileOutputMeta { byteLength: number; } +function bindCurrentDocumentApi(editor: Editor): EditorWithDoc { + const editorWithDoc = editor as EditorWithDoc; + + // `superdoc/super-editor` resolves to the published dist bundle, which can + // lag the source-backed document-api contract used by the CLI tests. Shadow + // the bundled getter with a source-backed DocumentApi so runtime behavior and + // response validation stay on the same contract version. + Object.defineProperty(editorWithDoc, 'doc', { + configurable: true, + value: createDocumentApi(getDocumentApiAdapters(editor)), + }); + + return editorWithDoc; +} + function toUint8Array(data: unknown): Uint8Array { if (data instanceof Uint8Array) return data; if (data instanceof ArrayBuffer) return new Uint8Array(data); @@ -217,7 +233,7 @@ export async function openDocument( } } - const editorWithDoc = editor as EditorWithDoc; + const editorWithDoc = bindCurrentDocumentApi(editor); return { editor: editorWithDoc, diff --git a/apps/cli/src/lib/error-mapping.ts b/apps/cli/src/lib/error-mapping.ts index 5226ed4f81..cd9ba6cddf 100644 --- a/apps/cli/src/lib/error-mapping.ts +++ b/apps/cli/src/lib/error-mapping.ts @@ -142,10 +142,19 @@ function mapTextMutationError(operationId: CliExposedOperationId, error: unknown const message = extractErrorMessage(error); const details = extractErrorDetails(error); - // Plan-engine errors pass through with original code and structured details + // Plan-engine errors pass through with original code and structured details. + // Must be checked before the INVALID_INPUT → INVALID_ARGUMENT remap below, + // because INVALID_INPUT is also a valid plan-engine passthrough code. const planEngineError = tryMapPlanEngineError(operationId, error, code); if (planEngineError) return planEngineError; + // For direct text-mutation commands, adapter INVALID_INPUT errors reflect CLI + // payload-shape issues (for example flat-flag shortcuts that did not + // normalize into a canonical target), so present them as INVALID_ARGUMENT. + if (code === 'INVALID_INPUT') { + return new CliError('INVALID_ARGUMENT', message, { operationId, details }); + } + if (code === 'TARGET_NOT_FOUND') { return new CliError('TARGET_NOT_FOUND', message, { operationId, details }); } diff --git a/packages/document-api/src/format/inline-run-patch.test.ts b/packages/document-api/src/format/inline-run-patch.test.ts index dec3e70b63..04e6a1dd21 100644 --- a/packages/document-api/src/format/inline-run-patch.test.ts +++ b/packages/document-api/src/format/inline-run-patch.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { INLINE_PROPERTY_BY_KEY } from './inline-run-patch.js'; +import { INLINE_PROPERTY_BY_KEY, validateInlineRunPatch } from './inline-run-patch.js'; +import { DocumentApiValidationError } from '../errors.js'; describe('INLINE_PROPERTY_REGISTRY: caps entry', () => { const entry = INLINE_PROPERTY_BY_KEY['caps']; @@ -29,6 +30,34 @@ describe('INLINE_PROPERTY_REGISTRY: caps entry', () => { }); }); +describe('validateInlineRunPatch: rejects Object.prototype keys', () => { + it('rejects toString as an unknown inline property', () => { + expect(() => validateInlineRunPatch({ toString: true })).toThrow(DocumentApiValidationError); + expect(() => validateInlineRunPatch({ toString: true })).toThrow('Unknown inline property: "toString"'); + }); + + it('rejects constructor as an unknown inline property', () => { + expect(() => validateInlineRunPatch({ constructor: true })).toThrow(DocumentApiValidationError); + expect(() => validateInlineRunPatch({ constructor: true })).toThrow('Unknown inline property: "constructor"'); + }); + + it('rejects hasOwnProperty as an unknown inline property', () => { + expect(() => validateInlineRunPatch({ hasOwnProperty: true })).toThrow(DocumentApiValidationError); + }); + + it('rejects __proto__ as an unknown inline property', () => { + // JSON.parse produces __proto__ as an own property (not the setter) + const patch = JSON.parse('{"__proto__": true}'); + expect(() => validateInlineRunPatch(patch)).toThrow(DocumentApiValidationError); + }); + + it('still accepts valid inline properties', () => { + expect(() => validateInlineRunPatch({ bold: true })).not.toThrow(); + expect(() => validateInlineRunPatch({ italic: null })).not.toThrow(); + expect(() => validateInlineRunPatch({ fontSize: 12 })).not.toThrow(); + }); +}); + describe('INLINE_PROPERTY_REGISTRY: smallCaps entry', () => { const entry = INLINE_PROPERTY_BY_KEY['smallCaps']; diff --git a/packages/document-api/src/format/inline-run-patch.ts b/packages/document-api/src/format/inline-run-patch.ts index 24dac5fd89..0f8b0b9d73 100644 --- a/packages/document-api/src/format/inline-run-patch.ts +++ b/packages/document-api/src/format/inline-run-patch.ts @@ -651,10 +651,10 @@ const PROPERTY_VALIDATOR_MAP: Record Date: Tue, 17 Mar 2026 17:56:23 -0700 Subject: [PATCH 4/4] chore: fix cli --- apps/cli/src/__tests__/host.test.ts | 2 +- apps/cli/src/lib/error-mapping.ts | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/apps/cli/src/__tests__/host.test.ts b/apps/cli/src/__tests__/host.test.ts index 0cdc58f9dd..d661b67a00 100644 --- a/apps/cli/src/__tests__/host.test.ts +++ b/apps/cli/src/__tests__/host.test.ts @@ -328,7 +328,7 @@ describe('CLI host mode', () => { await invokeAndValidate('doc.comments.list', ['comments', 'list', docPath, '--include-resolved', 'false']); await host.shutdown(); - }); + }, 15_000); test('returns parse errors for malformed frames', async () => { const stateDir = await mkdtemp(path.join(tmpdir(), 'superdoc-host-test-')); diff --git a/apps/cli/src/lib/error-mapping.ts b/apps/cli/src/lib/error-mapping.ts index cd9ba6cddf..4a24bc3a75 100644 --- a/apps/cli/src/lib/error-mapping.ts +++ b/apps/cli/src/lib/error-mapping.ts @@ -142,12 +142,6 @@ function mapTextMutationError(operationId: CliExposedOperationId, error: unknown const message = extractErrorMessage(error); const details = extractErrorDetails(error); - // Plan-engine errors pass through with original code and structured details. - // Must be checked before the INVALID_INPUT → INVALID_ARGUMENT remap below, - // because INVALID_INPUT is also a valid plan-engine passthrough code. - const planEngineError = tryMapPlanEngineError(operationId, error, code); - if (planEngineError) return planEngineError; - // For direct text-mutation commands, adapter INVALID_INPUT errors reflect CLI // payload-shape issues (for example flat-flag shortcuts that did not // normalize into a canonical target), so present them as INVALID_ARGUMENT. @@ -155,6 +149,10 @@ function mapTextMutationError(operationId: CliExposedOperationId, error: unknown return new CliError('INVALID_ARGUMENT', message, { operationId, details }); } + // Other plan-engine errors pass through with original code and structured details. + const planEngineError = tryMapPlanEngineError(operationId, error, code); + if (planEngineError) return planEngineError; + if (code === 'TARGET_NOT_FOUND') { return new CliError('TARGET_NOT_FOUND', message, { operationId, details }); }