From 684ee52ef31d66101f703d21a26bbc142519fd42 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 12 May 2026 07:35:25 -0300 Subject: [PATCH 01/14] wip(document-api): customXml.parts.* contract layer (SD-3105) Locks in the public API surface for Custom XML Data Storage Parts: - Types + validators (customXml/customXml.types.ts, customXml.ts) - 5 operation definitions in operation-definitions.ts - Registry entries in operation-registry.ts - Dispatch entries in invoke.ts - DocumentApi.customXml + adapter slot in index.ts - Re-exports in package barrel Adapter implementation, plan-engine wrapper, OOXML package writer, and tests are pending. Two known typecheck failures left: reference-doc-map.ts (needs customXml group entry) and schemas.ts (needs JSON schemas for 5 ops). --- .../src/contract/operation-definitions.ts | 76 +++++++- .../src/contract/operation-registry.ts | 39 ++++ .../document-api/src/customXml/customXml.ts | 184 ++++++++++++++++++ .../src/customXml/customXml.types.ts | 145 ++++++++++++++ packages/document-api/src/index.ts | 53 +++++ packages/document-api/src/invoke/invoke.ts | 7 + 6 files changed, 503 insertions(+), 1 deletion(-) create mode 100644 packages/document-api/src/customXml/customXml.ts create mode 100644 packages/document-api/src/customXml/customXml.types.ts diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index 9ef679ddd6..5aa6d4477f 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -66,7 +66,8 @@ export type ReferenceGroupKey = | 'selection' | 'diff' | 'protection' - | 'permissionRanges'; + | 'permissionRanges' + | 'customXml'; // --------------------------------------------------------------------------- // Entry shape @@ -6259,6 +6260,79 @@ export const OPERATION_DEFINITIONS = { referenceGroup: 'permissionRanges', skipAsATool: true, }, + + // ------------------------------------------------------------------------- + // Custom XML Parts (ECMA-376 Part 1 §15.2.5, §15.2.6, §22.5) + // ------------------------------------------------------------------------- + + 'customXml.parts.list': { + memberPath: 'customXml.parts.list', + description: 'List Custom XML Data Storage Parts in the document, optionally filtered by root namespace or schema reference.', + expectedResult: 'Returns a CustomXmlPartsListResult with summary entries (no content); fetch content via get.', + requiresDocumentContext: true, + metadata: readOperation({ + idempotency: 'idempotent', + throws: T_REF_READ_LIST, + }), + referenceDocPath: 'custom-xml/parts/list.mdx', + referenceGroup: 'customXml', + }, + 'customXml.parts.get': { + memberPath: 'customXml.parts.get', + description: 'Get a single Custom XML Data Storage Part by itemID or package part name, including its full content.', + expectedResult: 'Returns a CustomXmlPartInfo with id, partName, namespaces, schemaRefs, and content; or null if not found.', + requiresDocumentContext: true, + metadata: readOperation({ + throws: T_NOT_FOUND_CAPABLE, + }), + referenceDocPath: 'custom-xml/parts/get.mdx', + referenceGroup: 'customXml', + }, + 'customXml.parts.create': { + memberPath: 'customXml.parts.create', + description: 'Add a new Custom XML Data Storage Part to the document. Generates a fresh itemID GUID and emits the Properties Part.', + expectedResult: 'Returns a CustomXmlPartsCreateResult with the generated id and package part names on success.', + requiresDocumentContext: true, + metadata: mutationOperation({ + idempotency: 'non-idempotent', + supportsDryRun: true, + supportsTrackedMode: false, + possibleFailureCodes: NONE_FAILURES, + throws: T_REF_INSERT, + }), + referenceDocPath: 'custom-xml/parts/create.mdx', + referenceGroup: 'customXml', + }, + 'customXml.parts.patch': { + memberPath: 'customXml.parts.patch', + description: 'Replace the content and/or schemaRefs of an existing Custom XML Data Storage Part. At least one of content or schemaRefs is required.', + expectedResult: 'Returns a CustomXmlPartsMutationResult indicating success with the resolved target or a failure.', + requiresDocumentContext: true, + metadata: mutationOperation({ + idempotency: 'idempotent', + supportsDryRun: true, + supportsTrackedMode: false, + possibleFailureCodes: NONE_FAILURES, + throws: T_REF_MUTATION, + }), + referenceDocPath: 'custom-xml/parts/patch.mdx', + referenceGroup: 'customXml', + }, + 'customXml.parts.remove': { + memberPath: 'customXml.parts.remove', + description: 'Remove a Custom XML Data Storage Part and clean up all linked package files (item, props, rels, content-types entry).', + expectedResult: 'Returns a CustomXmlPartsMutationResult indicating success or a failure.', + requiresDocumentContext: true, + metadata: mutationOperation({ + idempotency: 'non-idempotent', + supportsDryRun: true, + supportsTrackedMode: false, + possibleFailureCodes: NONE_FAILURES, + throws: T_REF_MUTATION_REMOVE, + }), + referenceDocPath: 'custom-xml/parts/remove.mdx', + referenceGroup: 'customXml', + }, } as const satisfies Record; // --------------------------------------------------------------------------- diff --git a/packages/document-api/src/contract/operation-registry.ts b/packages/document-api/src/contract/operation-registry.ts index 3b7be32a7a..416d080ae7 100644 --- a/packages/document-api/src/contract/operation-registry.ts +++ b/packages/document-api/src/contract/operation-registry.ts @@ -262,6 +262,18 @@ import type { BookmarkMutationResult, } from '../bookmarks/bookmarks.types.js'; +import type { + CustomXmlPartsListInput, + CustomXmlPartsListResult, + CustomXmlPartsGetInput, + CustomXmlPartInfo, + CustomXmlPartsCreateInput, + CustomXmlPartsCreateResult, + CustomXmlPartsPatchInput, + CustomXmlPartsRemoveInput, + CustomXmlPartsMutationResult, +} from '../customXml/customXml.types.js'; + import type { FootnoteListInput, FootnotesListResult, @@ -1544,6 +1556,33 @@ export interface OperationRegistry extends FormatInlineAliasOperationRegistry { options: MutationOptions; output: PermissionRangeMutationResult; }; + + // --- customXml.parts.* --- + 'customXml.parts.list': { + input: CustomXmlPartsListInput | undefined; + options: never; + output: CustomXmlPartsListResult; + }; + 'customXml.parts.get': { + input: CustomXmlPartsGetInput; + options: never; + output: CustomXmlPartInfo | null; + }; + 'customXml.parts.create': { + input: CustomXmlPartsCreateInput; + options: MutationOptions; + output: CustomXmlPartsCreateResult; + }; + 'customXml.parts.patch': { + input: CustomXmlPartsPatchInput; + options: MutationOptions; + output: CustomXmlPartsMutationResult; + }; + 'customXml.parts.remove': { + input: CustomXmlPartsRemoveInput; + options: MutationOptions; + output: CustomXmlPartsMutationResult; + }; } // --- Bidirectional completeness checks --- diff --git a/packages/document-api/src/customXml/customXml.ts b/packages/document-api/src/customXml/customXml.ts new file mode 100644 index 0000000000..2617a8bb93 --- /dev/null +++ b/packages/document-api/src/customXml/customXml.ts @@ -0,0 +1,184 @@ +import type { MutationOptions } from '../write/write.js'; +import { normalizeMutationOptions } from '../write/write.js'; +import { DocumentApiValidationError } from '../errors.js'; +import type { + CustomXmlPartsCreateInput, + CustomXmlPartsCreateResult, + CustomXmlPartsGetInput, + CustomXmlPartsListInput, + CustomXmlPartsListResult, + CustomXmlPartsMutationResult, + CustomXmlPartsPatchInput, + CustomXmlPartsRemoveInput, + CustomXmlPartInfo, + CustomXmlPartTarget, +} from './customXml.types.js'; + +// --------------------------------------------------------------------------- +// Adapter / API interface +// --------------------------------------------------------------------------- + +export interface CustomXmlPartsApi { + list(query?: CustomXmlPartsListInput): CustomXmlPartsListResult; + get(input: CustomXmlPartsGetInput): CustomXmlPartInfo | null; + create(input: CustomXmlPartsCreateInput, options?: MutationOptions): CustomXmlPartsCreateResult; + patch(input: CustomXmlPartsPatchInput, options?: MutationOptions): CustomXmlPartsMutationResult; + remove(input: CustomXmlPartsRemoveInput, options?: MutationOptions): CustomXmlPartsMutationResult; +} + +export type CustomXmlPartsAdapter = CustomXmlPartsApi; + +export interface CustomXmlApi { + parts: CustomXmlPartsApi; +} + +export type CustomXmlAdapter = CustomXmlApi; + +// --------------------------------------------------------------------------- +// Target validation +// --------------------------------------------------------------------------- + +function validateTarget(target: unknown, operationName: string): asserts target is CustomXmlPartTarget { + if (!target || typeof target !== 'object') { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `${operationName} requires a target with either { id } or { partName }.`, + { target }, + ); + } + const t = target as Record; + const hasId = typeof t.id === 'string' && t.id.length > 0; + const hasPartName = typeof t.partName === 'string' && t.partName.length > 0; + if (!hasId && !hasPartName) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `${operationName} target must have a non-empty 'id' or 'partName'.`, + { target }, + ); + } + if (hasId && hasPartName) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `${operationName} target must not provide both 'id' and 'partName'; choose one.`, + { target }, + ); + } +} + +// --------------------------------------------------------------------------- +// Content validation +// --------------------------------------------------------------------------- + +/** + * Lightweight well-formedness check for the Storage Part content. Catches + * empty strings, non-strings, and obviously malformed XML (no root element). + * Full XML parsing happens in the adapter; this is a fast boundary check + * to keep adapter errors actionable. + */ +function validateContent(content: unknown, operationName: string): asserts content is string { + if (typeof content !== 'string' || content.length === 0) { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `${operationName} requires a non-empty 'content' string of well-formed XML.`, + { contentType: typeof content }, + ); + } + // Minimal smell-test: there must be at least one '<' starting an element. + // The adapter does full parsing. + if (!/<\s*[A-Za-z_]/.test(content)) { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `${operationName} 'content' does not contain a root XML element.`, + ); + } +} + +function validateSchemaRefs(schemaRefs: unknown, operationName: string): asserts schemaRefs is string[] { + if (!Array.isArray(schemaRefs)) { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `${operationName} 'schemaRefs' must be an array of strings.`, + ); + } + for (const [i, entry] of schemaRefs.entries()) { + if (typeof entry !== 'string' || entry.length === 0) { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `${operationName} 'schemaRefs[${i}]' must be a non-empty string.`, + ); + } + } +} + +// --------------------------------------------------------------------------- +// Execute wrappers +// --------------------------------------------------------------------------- + +export function executeCustomXmlPartsList( + adapter: CustomXmlPartsAdapter, + query?: CustomXmlPartsListInput, +): CustomXmlPartsListResult { + if (query?.rootNamespace !== undefined && typeof query.rootNamespace !== 'string') { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `customXml.parts.list 'rootNamespace' must be a string when provided.`, + ); + } + if (query?.schemaRef !== undefined && typeof query.schemaRef !== 'string') { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `customXml.parts.list 'schemaRef' must be a string when provided.`, + ); + } + return adapter.list(query); +} + +export function executeCustomXmlPartsGet( + adapter: CustomXmlPartsAdapter, + input: CustomXmlPartsGetInput, +): CustomXmlPartInfo | null { + validateTarget(input.target, 'customXml.parts.get'); + return adapter.get(input); +} + +export function executeCustomXmlPartsCreate( + adapter: CustomXmlPartsAdapter, + input: CustomXmlPartsCreateInput, + options?: MutationOptions, +): CustomXmlPartsCreateResult { + validateContent(input.content, 'customXml.parts.create'); + if (input.schemaRefs !== undefined) { + validateSchemaRefs(input.schemaRefs, 'customXml.parts.create'); + } + return adapter.create(input, normalizeMutationOptions(options)); +} + +export function executeCustomXmlPartsPatch( + adapter: CustomXmlPartsAdapter, + input: CustomXmlPartsPatchInput, + options?: MutationOptions, +): CustomXmlPartsMutationResult { + validateTarget(input.target, 'customXml.parts.patch'); + if (input.content === undefined && input.schemaRefs === undefined) { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + `customXml.parts.patch requires at least one of 'content' or 'schemaRefs'.`, + ); + } + if (input.content !== undefined) { + validateContent(input.content, 'customXml.parts.patch'); + } + if (input.schemaRefs !== undefined) { + validateSchemaRefs(input.schemaRefs, 'customXml.parts.patch'); + } + return adapter.patch(input, normalizeMutationOptions(options)); +} + +export function executeCustomXmlPartsRemove( + adapter: CustomXmlPartsAdapter, + input: CustomXmlPartsRemoveInput, + options?: MutationOptions, +): CustomXmlPartsMutationResult { + validateTarget(input.target, 'customXml.parts.remove'); + return adapter.remove(input, normalizeMutationOptions(options)); +} diff --git a/packages/document-api/src/customXml/customXml.types.ts b/packages/document-api/src/customXml/customXml.types.ts new file mode 100644 index 0000000000..feacd29f30 --- /dev/null +++ b/packages/document-api/src/customXml/customXml.types.ts @@ -0,0 +1,145 @@ +import type { AdapterMutationFailure } from '../types/adapter-result.js'; +import type { DiscoveryOutput } from '../types/discovery.js'; + +// --------------------------------------------------------------------------- +// Custom XML Part targeting +// --------------------------------------------------------------------------- + +/** + * Stable identifier for a Custom XML Data Storage Part. + * + * Maps to the `` GUID in the part's Properties + * Part (ECMA-376 Part 1 §22.5.2.1). Format is a literal `ST_Guid` with + * braces: `"{A67AC88A-A164-4ADE-8889-8826CE44DE6E}"`. + * + * Absent when a Storage Part has no Properties Part (foreign producers + * sometimes ship one without the other; the spec allows it). In that + * case, use `{ partName }` to target the part instead. + */ +export type CustomXmlPartId = string; + +/** + * Target shape for read/patch/remove operations. + * + * Most callers will use `{ id }` (the itemID GUID). The `{ partName }` + * variant exists for Storage Parts that have no Properties Part — those + * have no itemID and can only be addressed by their file path inside + * the OOXML package. + */ +export type CustomXmlPartTarget = { id: CustomXmlPartId } | { partName: string }; + +// --------------------------------------------------------------------------- +// Input types +// --------------------------------------------------------------------------- + +export interface CustomXmlPartsListInput { + /** + * Filter by the XML namespace of the Storage Part's root element + * (e.g. `` → `'urn:harvey:refs:1'`). + * + * Distinct from `schemaRef`: this is what the *data* declares; schemaRef + * is what the Properties Part declares as the associated XML schema's + * target namespace. Often they match, but the spec does not require it. + */ + rootNamespace?: string; + /** + * Filter by one of the part's `` values + * (ECMA-376 Part 1 §22.5.2.2). Matches if any declared schemaRef equals + * this URI. + */ + schemaRef?: string; + limit?: number; + offset?: number; +} + +export interface CustomXmlPartsGetInput { + target: CustomXmlPartTarget; +} + +export interface CustomXmlPartsCreateInput { + /** + * Well-formed XML for the Storage Part's body. Anything that is legal + * `application/xml` is acceptable; consumers control the schema entirely. + */ + content: string; + /** + * Optional list of XML schema target namespaces to declare in the + * Properties Part (``). When omitted or empty, the + * Properties Part is still emitted with a fresh itemID and an empty + * `` so `id` is always discoverable on readback. + */ + schemaRefs?: string[]; +} + +export interface CustomXmlPartsPatchInput { + target: CustomXmlPartTarget; + /** Replace the Storage Part's content. Must be well-formed XML. */ + content?: string; + /** + * Replace the Properties Part's `` set with this list. + * Pass `[]` to clear all schemaRefs. + */ + schemaRefs?: string[]; +} + +export interface CustomXmlPartsRemoveInput { + target: CustomXmlPartTarget; +} + +// --------------------------------------------------------------------------- +// Info / domain +// --------------------------------------------------------------------------- + +/** + * Lightweight view of a Custom XML Part returned by `list()`. Does NOT + * carry `content` — parts can be large; fetch the full record via `get()` + * when needed. + */ +export interface CustomXmlPartSummary { + /** itemID GUID; absent when no Properties Part exists. */ + id?: CustomXmlPartId; + /** Package-relative path of the Storage Part, e.g. `"customXml/item1.xml"`. */ + partName: string; + /** Package-relative path of the Properties Part, when present. */ + propsPartName?: string; + /** + * XML namespace URI of the Storage Part's root element, parsed from + * `content`. Absent when the root element has no namespace. + */ + rootNamespace?: string; + /** Values from `` in the Properties Part. */ + schemaRefs: string[]; +} + +export type CustomXmlPartInfo = CustomXmlPartSummary & { + /** Full serialized XML body of the Storage Part. */ + content: string; +}; + +// --------------------------------------------------------------------------- +// Mutation results +// --------------------------------------------------------------------------- + +export interface CustomXmlPartsCreateSuccess { + success: true; + /** Generated itemID GUID for the newly created part. */ + id: CustomXmlPartId; + partName: string; + propsPartName: string; +} + +export type CustomXmlPartsCreateResult = CustomXmlPartsCreateSuccess | AdapterMutationFailure; + +export interface CustomXmlPartsMutationSuccess { + success: true; + /** Identifier the operation acted on (mirrors the resolved input target). */ + target: CustomXmlPartTarget; +} + +export type CustomXmlPartsMutationResult = CustomXmlPartsMutationSuccess | AdapterMutationFailure; + +// --------------------------------------------------------------------------- +// List result +// --------------------------------------------------------------------------- + +export type CustomXmlPartsListResult = DiscoveryOutput; diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index 7e86445db8..d8137b064d 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -680,6 +680,26 @@ import type { BookmarkMutationResult, } from './bookmarks/bookmarks.types.js'; +import type { CustomXmlApi, CustomXmlAdapter } from './customXml/customXml.js'; +import { + executeCustomXmlPartsList, + executeCustomXmlPartsGet, + executeCustomXmlPartsCreate, + executeCustomXmlPartsPatch, + executeCustomXmlPartsRemove, +} from './customXml/customXml.js'; +import type { + CustomXmlPartsListInput, + CustomXmlPartsListResult, + CustomXmlPartsGetInput, + CustomXmlPartInfo, + CustomXmlPartsCreateInput, + CustomXmlPartsCreateResult, + CustomXmlPartsPatchInput, + CustomXmlPartsRemoveInput, + CustomXmlPartsMutationResult, +} from './customXml/customXml.types.js'; + import type { ProtectionApi, ProtectionAdapter } from './protection/protection.js'; import { executeProtectionGet, @@ -1034,6 +1054,12 @@ export type { } from './images/images.types.js'; export type { TocApi, TocAdapter } from './toc/toc.js'; export type { BookmarksApi, BookmarksAdapter } from './bookmarks/bookmarks.js'; +export type { + CustomXmlApi, + CustomXmlAdapter, + CustomXmlPartsApi, + CustomXmlPartsAdapter, +} from './customXml/customXml.js'; export type { ProtectionApi, ProtectionAdapter } from './protection/protection.js'; export * from './protection/protection.types.js'; @@ -1201,6 +1227,7 @@ export type { HyperlinksRemoveInput, } from './hyperlinks/hyperlinks.types.js'; export type * from './bookmarks/bookmarks.types.js'; +export type * from './customXml/customXml.types.js'; export type * from './footnotes/footnotes.types.js'; export type * from './cross-refs/cross-refs.types.js'; @@ -1704,6 +1731,11 @@ export interface DocumentApi { * Permission range exception operations for protected documents. */ permissionRanges: PermissionRangesApi; + /** + * Custom XML Data Storage Part operations (ECMA-376 §15.2.5, §15.2.6). + * Read and write raw custom XML parts in the OOXML package. + */ + customXml: CustomXmlApi; /** * Runtime capability introspection. * @@ -1776,6 +1808,8 @@ export interface DocumentApiAdapters { history: HistoryAdapter; protection: ProtectionAdapter; permissionRanges: PermissionRangesAdapter; + /** Custom XML Data Storage Part operations. Optional; not all engines support custom XML. */ + customXml?: CustomXmlAdapter; } /** @@ -3266,6 +3300,25 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { return executePermissionRangesUpdatePrincipal(adapters.permissionRanges, input, options); }, }, + customXml: { + parts: { + list(input?: CustomXmlPartsListInput): CustomXmlPartsListResult { + return executeCustomXmlPartsList(requireAdapter(adapters.customXml, 'customXml').parts, input); + }, + get(input: CustomXmlPartsGetInput): CustomXmlPartInfo | null { + return executeCustomXmlPartsGet(requireAdapter(adapters.customXml, 'customXml').parts, input); + }, + create(input: CustomXmlPartsCreateInput, options?: MutationOptions): CustomXmlPartsCreateResult { + return executeCustomXmlPartsCreate(requireAdapter(adapters.customXml, 'customXml').parts, input, options); + }, + patch(input: CustomXmlPartsPatchInput, options?: MutationOptions): CustomXmlPartsMutationResult { + return executeCustomXmlPartsPatch(requireAdapter(adapters.customXml, 'customXml').parts, input, options); + }, + remove(input: CustomXmlPartsRemoveInput, options?: MutationOptions): CustomXmlPartsMutationResult { + return executeCustomXmlPartsRemove(requireAdapter(adapters.customXml, 'customXml').parts, input, options); + }, + }, + }, invoke(request: DynamicInvokeRequest): unknown { if (!Object.prototype.hasOwnProperty.call(dispatch, request.operationId)) { throw new Error(`Unknown operationId: "${request.operationId}"`); diff --git a/packages/document-api/src/invoke/invoke.ts b/packages/document-api/src/invoke/invoke.ts index db820c841f..92d146c7fb 100644 --- a/packages/document-api/src/invoke/invoke.ts +++ b/packages/document-api/src/invoke/invoke.ts @@ -530,5 +530,12 @@ export function buildDispatchTable(api: DocumentApi): TypedDispatchTable { 'permissionRanges.create': (input, options) => api.permissionRanges.create(input, options), 'permissionRanges.remove': (input, options) => api.permissionRanges.remove(input, options), 'permissionRanges.updatePrincipal': (input, options) => api.permissionRanges.updatePrincipal(input, options), + + // --- customXml.parts.* --- + 'customXml.parts.list': (input) => api.customXml.parts.list(input), + 'customXml.parts.get': (input) => api.customXml.parts.get(input), + 'customXml.parts.create': (input, options) => api.customXml.parts.create(input, options), + 'customXml.parts.patch': (input, options) => api.customXml.parts.patch(input, options), + 'customXml.parts.remove': (input, options) => api.customXml.parts.remove(input, options), }; } From ac35667a34eabff7209bd5e070e8abfdffb61661 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 12 May 2026 07:45:39 -0300 Subject: [PATCH 02/14] wip(document-api): customXml.parts read adapter + contract gaps (SD-3105) Completes the read path through the Document API and closes the remaining contract-layer wiring gaps. Contract: - reference-doc-map.ts: customXml group title/description/page entry - schemas.ts: 5 op JSON schemas + customXmlPartTargetSchema helper - 30 validator tests passing (target xor id/partName, content well-formedness smell-test, schemaRefs string[] check, patch requires at-least-one) Read adapter: - super-converter/custom-xml-parts.js: discovery, parsing, serialization helpers (listCustomXmlStoragePartNames, parsePropsPart, readCustomXmlPart) - plan-engine/custom-xml-wrappers.ts: list/get adapter routing through buildDiscoveryItem/Result, filters by rootNamespace and schemaRef, partName-targeting fallback for foreign parts without Properties Parts - assemble-adapters.ts: customXml plugged in alongside bookmarks - 10 integration tests passing (list empty, list with filter, get by id, get by partName fallback, get unknown id returns null) Write adapter: - create/patch/remove return CAPABILITY_UNAVAILABLE pending Phase B (OOXML package file coordination) --- .../src/contract/reference-doc-map.ts | 6 + packages/document-api/src/contract/schemas.ts | 63 +++++ .../src/customXml/customXml.test.ts | 241 ++++++++++++++++++ .../core/super-converter/custom-xml-parts.js | 229 +++++++++++++++++ .../assemble-adapters.ts | 4 + .../custom-xml-wrappers.integration.test.ts | 208 +++++++++++++++ .../plan-engine/custom-xml-wrappers.ts | 164 ++++++++++++ 7 files changed, 915 insertions(+) create mode 100644 packages/document-api/src/customXml/customXml.test.ts create mode 100644 packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js create mode 100644 packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts create mode 100644 packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts diff --git a/packages/document-api/src/contract/reference-doc-map.ts b/packages/document-api/src/contract/reference-doc-map.ts index c8b1ba353c..3e432bc614 100644 --- a/packages/document-api/src/contract/reference-doc-map.ts +++ b/packages/document-api/src/contract/reference-doc-map.ts @@ -191,6 +191,12 @@ const GROUP_METADATA: Record = { success: { type: 'object' }, failure: { type: 'object' }, }, + + // --- customXml.parts.* --- + 'customXml.parts.list': { + input: objectSchema({ + ...refListQueryProperties, + rootNamespace: { type: 'string' }, + schemaRef: { type: 'string' }, + }), + output: discoveryOutputSchema, + }, + 'customXml.parts.get': { + input: objectSchema({ target: customXmlPartTargetSchema }, ['target']), + output: { type: 'object' }, + }, + 'customXml.parts.create': { + input: objectSchema( + { + content: { type: 'string' }, + schemaRefs: { type: 'array', items: { type: 'string' } }, + }, + ['content'], + ), + ...customXmlPartCreateMutation, + }, + 'customXml.parts.patch': { + input: objectSchema( + { + target: customXmlPartTargetSchema, + content: { type: 'string' }, + schemaRefs: { type: 'array', items: { type: 'string' } }, + }, + ['target'], + ), + ...customXmlPartMutation, + }, + 'customXml.parts.remove': { + input: objectSchema({ target: customXmlPartTargetSchema }, ['target']), + ...customXmlPartMutation, + }, }; /** diff --git a/packages/document-api/src/customXml/customXml.test.ts b/packages/document-api/src/customXml/customXml.test.ts new file mode 100644 index 0000000000..59fe06a2fa --- /dev/null +++ b/packages/document-api/src/customXml/customXml.test.ts @@ -0,0 +1,241 @@ +import { describe, it, expect, mock } from 'bun:test'; +import { DocumentApiValidationError } from '../errors.js'; +import { + executeCustomXmlPartsList, + executeCustomXmlPartsGet, + executeCustomXmlPartsCreate, + executeCustomXmlPartsPatch, + executeCustomXmlPartsRemove, + type CustomXmlPartsAdapter, +} from './customXml.js'; + +function makeAdapter(): CustomXmlPartsAdapter { + return { + list: mock().mockReturnValue({ items: [], total: 0 }), + get: mock().mockReturnValue(null), + create: mock().mockReturnValue({ success: true, id: '{X}', partName: 'customXml/item1.xml', propsPartName: 'customXml/itemProps1.xml' }), + patch: mock().mockReturnValue({ success: true, target: { id: '{X}' } }), + remove: mock().mockReturnValue({ success: true, target: { id: '{X}' } }), + }; +} + +const VALID_XML = ''; + +// --------------------------------------------------------------------------- +// list +// --------------------------------------------------------------------------- + +describe('customXml.parts.list validation', () => { + it('accepts no input', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsList(adapter)).not.toThrow(); + expect(adapter.list).toHaveBeenCalled(); + }); + + it('accepts rootNamespace filter', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsList(adapter, { rootNamespace: 'urn:foo' })).not.toThrow(); + }); + + it('accepts schemaRef filter', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsList(adapter, { schemaRef: 'urn:foo' })).not.toThrow(); + }); + + it('rejects non-string rootNamespace', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsList(adapter, { rootNamespace: 42 as unknown as string })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects non-string schemaRef', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsList(adapter, { schemaRef: {} as unknown as string })).toThrow( + DocumentApiValidationError, + ); + }); +}); + +// --------------------------------------------------------------------------- +// Target validation (shared across get/patch/remove) +// --------------------------------------------------------------------------- + +describe('customXml.parts target validation', () => { + it('accepts { id }', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsGet(adapter, { target: { id: '{X}' } })).not.toThrow(); + }); + + it('accepts { partName }', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsGet(adapter, { target: { partName: 'customXml/item1.xml' } })).not.toThrow(); + }); + + it('rejects null target', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsGet(adapter, { target: null as unknown as { id: string } })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects target with neither id nor partName', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsGet(adapter, { target: {} as { id: string } })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects target with empty id', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsGet(adapter, { target: { id: '' } })).toThrow(DocumentApiValidationError); + }); + + it('rejects target with empty partName', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsGet(adapter, { target: { partName: '' } })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects target with BOTH id and partName', () => { + const adapter = makeAdapter(); + expect(() => + executeCustomXmlPartsGet(adapter, { + target: { id: '{X}', partName: 'customXml/item1.xml' } as { id: string }, + }), + ).toThrow(DocumentApiValidationError); + }); +}); + +// --------------------------------------------------------------------------- +// create +// --------------------------------------------------------------------------- + +describe('customXml.parts.create validation', () => { + it('accepts well-formed content with no schemaRefs', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsCreate(adapter, { content: VALID_XML })).not.toThrow(); + }); + + it('accepts well-formed content with schemaRefs', () => { + const adapter = makeAdapter(); + expect(() => + executeCustomXmlPartsCreate(adapter, { content: VALID_XML, schemaRefs: ['urn:test:1'] }), + ).not.toThrow(); + }); + + it('accepts empty schemaRefs array', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsCreate(adapter, { content: VALID_XML, schemaRefs: [] })).not.toThrow(); + }); + + it('rejects empty content', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsCreate(adapter, { content: '' })).toThrow(DocumentApiValidationError); + }); + + it('rejects non-string content', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsCreate(adapter, { content: 42 as unknown as string })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects content with no XML root element', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsCreate(adapter, { content: 'not xml at all' })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects non-array schemaRefs', () => { + const adapter = makeAdapter(); + expect(() => + executeCustomXmlPartsCreate(adapter, { content: VALID_XML, schemaRefs: 'urn:foo' as unknown as string[] }), + ).toThrow(DocumentApiValidationError); + }); + + it('rejects schemaRefs with empty string entries', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsCreate(adapter, { content: VALID_XML, schemaRefs: [''] })).toThrow( + DocumentApiValidationError, + ); + }); + + it('rejects schemaRefs with non-string entries', () => { + const adapter = makeAdapter(); + expect(() => + executeCustomXmlPartsCreate(adapter, { + content: VALID_XML, + schemaRefs: [42 as unknown as string], + }), + ).toThrow(DocumentApiValidationError); + }); +}); + +// --------------------------------------------------------------------------- +// patch +// --------------------------------------------------------------------------- + +describe('customXml.parts.patch validation', () => { + const target = { id: '{X}' }; + + it('accepts content-only patch', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsPatch(adapter, { target, content: VALID_XML })).not.toThrow(); + }); + + it('accepts schemaRefs-only patch', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsPatch(adapter, { target, schemaRefs: ['urn:foo'] })).not.toThrow(); + }); + + it('accepts patch with both content and schemaRefs', () => { + const adapter = makeAdapter(); + expect(() => + executeCustomXmlPartsPatch(adapter, { target, content: VALID_XML, schemaRefs: ['urn:foo'] }), + ).not.toThrow(); + }); + + it('rejects patch with neither content nor schemaRefs', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsPatch(adapter, { target })).toThrow(DocumentApiValidationError); + }); + + it('rejects patch with empty schemaRefs cleared but valid content', () => { + // Empty schemaRefs is allowed (means "clear them"), content also valid. + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsPatch(adapter, { target, content: VALID_XML, schemaRefs: [] })).not.toThrow(); + }); + + it('rejects malformed content', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsPatch(adapter, { target, content: 'not xml' })).toThrow( + DocumentApiValidationError, + ); + }); +}); + +// --------------------------------------------------------------------------- +// remove +// --------------------------------------------------------------------------- + +describe('customXml.parts.remove validation', () => { + it('accepts { id } target', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsRemove(adapter, { target: { id: '{X}' } })).not.toThrow(); + }); + + it('accepts { partName } target', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsRemove(adapter, { target: { partName: 'customXml/item1.xml' } })).not.toThrow(); + }); + + it('rejects missing target', () => { + const adapter = makeAdapter(); + expect(() => executeCustomXmlPartsRemove(adapter, { target: {} as { id: string } })).toThrow( + DocumentApiValidationError, + ); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js new file mode 100644 index 0000000000..d0b869d5a7 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -0,0 +1,229 @@ +/** + * Custom XML Data Storage Part runtime — generic read/write helpers for + * the OOXML custom XML feature (ECMA-376 Part 1 §15.2.5, §15.2.6, §22.5). + * + * Decoupled from any specific schema (citations, Harvey refs, etc.). + * Used by the Document API `customXml.parts.*` adapter to surface raw + * custom XML parts through the public API. + */ + +import * as xmljs from 'xml-js'; + +export const CUSTOM_XML_DATA_RELATIONSHIP_TYPE = + 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXml'; +export const CUSTOM_XML_PROPS_RELATIONSHIP_TYPE = + 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXmlProps'; +export const CUSTOM_XML_PROPS_CONTENT_TYPE = + 'application/vnd.openxmlformats-officedocument.customXmlProperties+xml'; +export const CUSTOM_XML_DATASTORE_NAMESPACE = + 'http://schemas.openxmlformats.org/officeDocument/2006/customXml'; + +// --------------------------------------------------------------------------- +// Local helpers +// --------------------------------------------------------------------------- + +function getLocalName(name) { + if (!name || typeof name !== 'string') return ''; + const i = name.indexOf(':'); + return i >= 0 ? name.slice(i + 1) : name; +} + +function findFirstElement(parent, localName) { + if (!parent?.elements?.length) return null; + return parent.elements.find((el) => el?.type === 'element' && getLocalName(el.name) === localName) ?? null; +} + +function findAllElements(parent, localName) { + if (!parent?.elements?.length) return []; + return parent.elements.filter((el) => el?.type === 'element' && getLocalName(el.name) === localName); +} + +function partNameFromIndex(index) { + return `customXml/item${index}.xml`; +} + +function propsPartNameFromIndex(index) { + return `customXml/itemProps${index}.xml`; +} + +function indexFromPartName(partName) { + const m = /^customXml\/item(\d+)\.xml$/i.exec(partName ?? ''); + return m ? Number.parseInt(m[1], 10) : null; +} + +function indexFromPropsPartName(propsPartName) { + const m = /^customXml\/itemProps(\d+)\.xml$/i.exec(propsPartName ?? ''); + return m ? Number.parseInt(m[1], 10) : null; +} + +// --------------------------------------------------------------------------- +// Discovery +// --------------------------------------------------------------------------- + +/** + * Enumerates every custom XML Storage Part in the package by scanning + * convertedXml keys (not the relationships file, because foreign producers + * sometimes leave orphan parts that aren't referenced from word/document.xml). + * + * Returns part names sorted by their numeric index. Pair-matching with + * Properties Parts is left to the caller. + */ +export function listCustomXmlStoragePartNames(convertedXml) { + if (!convertedXml || typeof convertedXml !== 'object') return []; + const indexes = []; + for (const path of Object.keys(convertedXml)) { + const idx = indexFromPartName(path); + if (idx != null) indexes.push(idx); + } + indexes.sort((a, b) => a - b); + return indexes.map(partNameFromIndex); +} + +/** + * Returns the Properties Part name paired with `partName`, if present. + * Pairs by matching numeric index (item1 ↔ itemProps1). + */ +export function findPropsPartFor(convertedXml, partName) { + const idx = indexFromPartName(partName); + if (idx == null) return null; + const candidate = propsPartNameFromIndex(idx); + return convertedXml?.[candidate] ? candidate : null; +} + +// --------------------------------------------------------------------------- +// Parsing +// --------------------------------------------------------------------------- + +/** + * Parses a Properties Part for its itemID and schemaRefs. + * + * @returns `{ itemId, schemaRefs }` or `null` when the doc is malformed. + */ +export function parsePropsPart(propsDoc) { + const root = propsDoc?.elements?.find((el) => el?.type === 'element' && getLocalName(el.name) === 'datastoreItem'); + if (!root) return null; + const itemId = root.attributes?.['ds:itemID'] ?? root.attributes?.itemID ?? null; + const schemaRefsEl = findFirstElement(root, 'schemaRefs'); + const schemaRefs = findAllElements(schemaRefsEl, 'schemaRef') + .map((el) => el.attributes?.['ds:uri'] ?? el.attributes?.uri ?? null) + .filter((uri) => typeof uri === 'string' && uri.length > 0); + return { itemId: typeof itemId === 'string' && itemId.length > 0 ? itemId : null, schemaRefs }; +} + +/** + * Extracts the namespace URI declared on the Storage Part's root element. + * Returns `null` when no `xmlns` is present (e.g. plain `` with no + * default namespace). + */ +export function parseStoragePartRootNamespace(storageDoc) { + const root = storageDoc?.elements?.find((el) => el?.type === 'element'); + if (!root) return null; + const xmlns = root.attributes?.xmlns; + if (typeof xmlns === 'string' && xmlns.length > 0) return xmlns; + // Check for prefixed default namespace forms like `xmlns:b="..."` where + // the root element actually uses that prefix. + const elementName = root.name ?? ''; + const colonIdx = elementName.indexOf(':'); + if (colonIdx > 0) { + const prefix = elementName.slice(0, colonIdx); + const prefixedAttr = `xmlns:${prefix}`; + const prefixedValue = root.attributes?.[prefixedAttr]; + if (typeof prefixedValue === 'string' && prefixedValue.length > 0) return prefixedValue; + } + return null; +} + +/** + * Serializes a parsed XML document (xml-js shape) back to a string. + * Used to surface part content through the Document API as a string. + */ +export function serializeXmlDoc(xmlDoc) { + if (!xmlDoc) return ''; + return xmljs.js2xml(xmlDoc, { compact: false, spaces: 0 }); +} + +// --------------------------------------------------------------------------- +// High-level: read a single part as a Document API record +// --------------------------------------------------------------------------- + +/** + * Reads a custom XML part identified by either an itemID GUID or a + * package part name. Returns null when not found. + * + * Shape: + * { + * id: string | null, // itemID GUID; null if no Properties Part + * partName: string, // e.g. "customXml/item1.xml" + * propsPartName: string | null, // null when no Properties Part exists + * rootNamespace: string | null, + * schemaRefs: string[], + * content: string, // serialized Storage Part XML + * } + */ +export function readCustomXmlPart(convertedXml, target) { + if (!target || !convertedXml) return null; + let partName = null; + let itemId = null; + if (typeof target.partName === 'string' && target.partName.length > 0) { + partName = target.partName; + } else if (typeof target.id === 'string' && target.id.length > 0) { + itemId = target.id; + for (const candidatePartName of listCustomXmlStoragePartNames(convertedXml)) { + const propsName = findPropsPartFor(convertedXml, candidatePartName); + if (!propsName) continue; + const parsed = parsePropsPart(convertedXml[propsName]); + if (parsed?.itemId === itemId) { + partName = candidatePartName; + break; + } + } + if (!partName) return null; + } else { + return null; + } + + const storageDoc = convertedXml[partName]; + if (!storageDoc) return null; + const propsPartName = findPropsPartFor(convertedXml, partName); + const props = propsPartName ? parsePropsPart(convertedXml[propsPartName]) : null; + return { + id: props?.itemId ?? null, + partName, + propsPartName: propsPartName ?? null, + rootNamespace: parseStoragePartRootNamespace(storageDoc), + schemaRefs: props?.schemaRefs ?? [], + content: serializeXmlDoc(storageDoc), + }; +} + +/** + * Lists all custom XML parts in the package as summary records (no content). + */ +export function listCustomXmlParts(convertedXml) { + return listCustomXmlStoragePartNames(convertedXml).map((partName) => { + const propsPartName = findPropsPartFor(convertedXml, partName); + const props = propsPartName ? parsePropsPart(convertedXml[propsPartName]) : null; + return { + id: props?.itemId ?? null, + partName, + propsPartName: propsPartName ?? null, + rootNamespace: parseStoragePartRootNamespace(convertedXml[partName]), + schemaRefs: props?.schemaRefs ?? [], + }; + }); +} + +// --------------------------------------------------------------------------- +// Index allocation (write side helper, also useful for tests) +// --------------------------------------------------------------------------- + +export function nextCustomXmlItemIndex(convertedXml) { + const used = new Set(); + for (const path of Object.keys(convertedXml ?? {})) { + const idx = indexFromPartName(path) ?? indexFromPropsPartName(path); + if (idx != null) used.add(idx); + } + let candidate = 1; + while (used.has(candidate)) candidate += 1; + return candidate; +} diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts b/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts index f06399ccfc..1859f00da4 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts @@ -256,6 +256,7 @@ import { bookmarksRenameWrapper, bookmarksRemoveWrapper, } from './plan-engine/bookmark-wrappers.js'; +import { createCustomXmlPartsAdapter } from './plan-engine/custom-xml-wrappers.js'; import { protectionGetAdapter, protectionSetEditingRestrictionAdapter, @@ -645,6 +646,9 @@ export function assembleDocumentApiAdapters(editor: Editor): DocumentApiAdapters rename: (input, options) => bookmarksRenameWrapper(editor, input, options), remove: (input, options) => bookmarksRemoveWrapper(editor, input, options), }, + customXml: { + parts: createCustomXmlPartsAdapter(editor), + }, footnotes: { list: (query) => footnotesListWrapper(editor, query), get: (input) => footnotesGetWrapper(editor, input), diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts new file mode 100644 index 0000000000..44a6d82442 --- /dev/null +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -0,0 +1,208 @@ +/* @vitest-environment jsdom */ + +/** + * customXml.parts.* read-side smoke tests against a real editor. + * + * - Empty document: list returns no parts; get returns null. + * - Manually injecting a custom XML part into the converter package: + * list discovers it, get returns its content, filters work. + * + * Write side (`create` / `patch` / `remove`) is implemented behind a + * `CAPABILITY_UNAVAILABLE` stub for now; tests exist only for the + * lookup-shaped failures, not for actual write behavior. + */ + +import { describe, expect, it } from 'vitest'; +import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js'; + +const NAMESPACE = 'urn:test:1'; +const PART_NAME = 'customXml/item1.xml'; +const PROPS_PART_NAME = 'customXml/itemProps1.xml'; +const ITEM_ID = '{F94E36C5-3D55-44E3-9CE6-29F345BB8E78}'; + +function makeStorageDoc() { + return { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8' } }, + elements: [ + { + type: 'element', + name: 'refs', + attributes: { xmlns: NAMESPACE }, + elements: [ + { + type: 'element', + name: 'ref', + attributes: { id: 'a' }, + elements: [], + }, + ], + }, + ], + }; +} + +function makePropsDoc(itemId: string, schemaRefUris: string[]) { + return { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8' } }, + elements: [ + { + type: 'element', + name: 'ds:datastoreItem', + attributes: { + 'ds:itemID': itemId, + 'xmlns:ds': 'http://schemas.openxmlformats.org/officeDocument/2006/customXml', + }, + elements: [ + { + type: 'element', + name: 'ds:schemaRefs', + elements: schemaRefUris.map((uri) => ({ + type: 'element', + name: 'ds:schemaRef', + attributes: { 'ds:uri': uri }, + })), + }, + ], + }, + ], + }; +} + +async function createEditorWithEmptyPackage() { + const docData = await loadTestDataForEditorTests('blank-doc.docx'); + const { editor } = initTestEditor({ + content: docData.docx, + media: docData.media, + mediaFiles: docData.mediaFiles, + fonts: docData.fonts, + useImmediateSetTimeout: false, + isHeadless: true, + user: { name: 'Test', email: 'test@example.com' }, + }); + return editor; +} + +describe('customXml.parts read-side (integration)', () => { + it('returns no parts when the document has none', async () => { + const editor = await createEditorWithEmptyPackage(); + const list = editor.doc.customXml.parts.list(); + expect(list.items).toEqual([]); + expect(list.total).toBe(0); + editor.destroy(); + }); + + it('discovers a manually injected part and exposes its summary', async () => { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted[PROPS_PART_NAME] = makePropsDoc(ITEM_ID, [NAMESPACE]); + + const list = editor.doc.customXml.parts.list(); + expect(list.items.length).toBe(1); + const item = list.items[0]!; + expect(item.id).toBe(ITEM_ID); + expect(item.partName).toBe(PART_NAME); + expect(item.propsPartName).toBe(PROPS_PART_NAME); + expect(item.rootNamespace).toBe(NAMESPACE); + expect(item.schemaRefs).toEqual([NAMESPACE]); + editor.destroy(); + }); + + it('filters by rootNamespace', async () => { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted[PROPS_PART_NAME] = makePropsDoc(ITEM_ID, [NAMESPACE]); + + expect(editor.doc.customXml.parts.list({ rootNamespace: NAMESPACE }).items.length).toBe(1); + expect(editor.doc.customXml.parts.list({ rootNamespace: 'urn:other' }).items.length).toBe(0); + editor.destroy(); + }); + + it('filters by schemaRef', async () => { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted[PROPS_PART_NAME] = makePropsDoc(ITEM_ID, [NAMESPACE]); + + expect(editor.doc.customXml.parts.list({ schemaRef: NAMESPACE }).items.length).toBe(1); + expect(editor.doc.customXml.parts.list({ schemaRef: 'urn:other' }).items.length).toBe(0); + editor.destroy(); + }); + + it('get by id returns full content', async () => { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted[PROPS_PART_NAME] = makePropsDoc(ITEM_ID, [NAMESPACE]); + + const info = editor.doc.customXml.parts.get({ target: { id: ITEM_ID } }); + expect(info).not.toBeNull(); + expect(info!.id).toBe(ITEM_ID); + expect(info!.partName).toBe(PART_NAME); + expect(info!.content).toContain(' { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + // Storage Part only — simulates a foreign producer's orphan part. + converted[PART_NAME] = makeStorageDoc(); + + const info = editor.doc.customXml.parts.get({ target: { partName: PART_NAME } }); + expect(info).not.toBeNull(); + expect(info!.id).toBeUndefined(); + expect(info!.propsPartName).toBeUndefined(); + expect(info!.partName).toBe(PART_NAME); + expect(info!.rootNamespace).toBe(NAMESPACE); + expect(info!.schemaRefs).toEqual([]); + expect(info!.content).toContain(' { + const editor = await createEditorWithEmptyPackage(); + const info = editor.doc.customXml.parts.get({ target: { id: '{NOT-A-REAL-ID}' } }); + expect(info).toBeNull(); + editor.destroy(); + }); +}); + +describe('customXml.parts write-side (stubs)', () => { + it('create returns CAPABILITY_UNAVAILABLE until Phase B lands', async () => { + const editor = await createEditorWithEmptyPackage(); + const result = editor.doc.customXml.parts.create({ content: '' }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.failure.code).toBe('CAPABILITY_UNAVAILABLE'); + } + editor.destroy(); + }); + + it('patch returns CAPABILITY_UNAVAILABLE', async () => { + const editor = await createEditorWithEmptyPackage(); + const result = editor.doc.customXml.parts.patch({ target: { id: '{X}' }, content: '' }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.failure.code).toBe('CAPABILITY_UNAVAILABLE'); + } + editor.destroy(); + }); + + it('remove returns CAPABILITY_UNAVAILABLE', async () => { + const editor = await createEditorWithEmptyPackage(); + const result = editor.doc.customXml.parts.remove({ target: { id: '{X}' } }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.failure.code).toBe('CAPABILITY_UNAVAILABLE'); + } + editor.destroy(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts new file mode 100644 index 0000000000..b567dead67 --- /dev/null +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts @@ -0,0 +1,164 @@ +import type { Editor } from '../../core/Editor.js'; +import type { + CustomXmlPartsListInput, + CustomXmlPartsListResult, + CustomXmlPartsGetInput, + CustomXmlPartInfo, + CustomXmlPartSummary, + CustomXmlPartsCreateInput, + CustomXmlPartsCreateResult, + CustomXmlPartsPatchInput, + CustomXmlPartsRemoveInput, + CustomXmlPartsMutationResult, + CustomXmlPartsAdapter, + MutationOptions, +} from '@superdoc/document-api'; +import { buildDiscoveryItem, buildDiscoveryResult, buildResolvedHandle } from '@superdoc/document-api'; +import { paginate } from '../helpers/adapter-utils.js'; +import { getRevision } from './revision-tracker.js'; +import { + listCustomXmlParts, + readCustomXmlPart, +} from '../../core/super-converter/custom-xml-parts.js'; + +// --------------------------------------------------------------------------- +// Converter access +// --------------------------------------------------------------------------- + +type ConverterWithConvertedXml = { + convertedXml?: Record; +}; + +function getConvertedXml(editor: Editor): Record { + const converter = (editor as unknown as { converter?: ConverterWithConvertedXml }).converter; + return converter?.convertedXml ?? {}; +} + +// --------------------------------------------------------------------------- +// Read operations +// --------------------------------------------------------------------------- + +function toSummary(record: ReturnType[number]): CustomXmlPartSummary { + const summary: CustomXmlPartSummary = { + partName: record.partName, + schemaRefs: record.schemaRefs, + }; + if (record.id) summary.id = record.id; + if (record.propsPartName) summary.propsPartName = record.propsPartName; + if (record.rootNamespace) summary.rootNamespace = record.rootNamespace; + return summary; +} + +export function customXmlPartsListWrapper( + editor: Editor, + query?: CustomXmlPartsListInput, +): CustomXmlPartsListResult { + const revision = getRevision(editor); + const all = listCustomXmlParts(getConvertedXml(editor)); + + let filtered = all; + if (query?.rootNamespace !== undefined) { + filtered = filtered.filter((p) => p.rootNamespace === query.rootNamespace); + } + if (query?.schemaRef !== undefined) { + filtered = filtered.filter((p) => p.schemaRefs.includes(query.schemaRef as string)); + } + + const allItems = filtered.map((record) => { + const summary = toSummary(record); + // Stable identifier for the discovery item: itemID GUID when present, + // partName otherwise (foreign parts without a Properties Part). + const stableId = summary.id ?? summary.partName; + return buildDiscoveryItem( + stableId, + buildResolvedHandle(`customXml:${stableId}`, 'ephemeral', 'ext:customXmlPart'), + summary, + ); + }); + + const { total, items: paged } = paginate(allItems, query?.offset, query?.limit); + const effectiveLimit = query?.limit ?? total; + + return buildDiscoveryResult({ + evaluatedRevision: revision, + total, + items: paged, + page: { limit: effectiveLimit, offset: query?.offset ?? 0, returned: paged.length }, + }); +} + +export function customXmlPartsGetWrapper( + editor: Editor, + input: CustomXmlPartsGetInput, +): CustomXmlPartInfo | null { + const record = readCustomXmlPart(getConvertedXml(editor), input.target); + if (!record) return null; + // Normalize null fields to match CustomXmlPartInfo shape (optional, not null). + const info: CustomXmlPartInfo = { + partName: record.partName, + rootNamespace: record.rootNamespace ?? undefined, + schemaRefs: record.schemaRefs, + content: record.content, + }; + if (record.id) info.id = record.id; + if (record.propsPartName) info.propsPartName = record.propsPartName; + return info; +} + +// --------------------------------------------------------------------------- +// Write operations (placeholder until SD-3105 Phase B) +// --------------------------------------------------------------------------- + +function notImplemented(op: string): CustomXmlPartsMutationResult { + return { + success: false, + failure: { + code: 'CAPABILITY_UNAVAILABLE', + message: `${op} is not yet implemented on this adapter.`, + }, + }; +} + +export function customXmlPartsCreateWrapper( + _editor: Editor, + _input: CustomXmlPartsCreateInput, + _options?: MutationOptions, +): CustomXmlPartsCreateResult { + return { + success: false, + failure: { + code: 'CAPABILITY_UNAVAILABLE', + message: 'customXml.parts.create is not yet implemented on this adapter.', + }, + }; +} + +export function customXmlPartsPatchWrapper( + _editor: Editor, + _input: CustomXmlPartsPatchInput, + _options?: MutationOptions, +): CustomXmlPartsMutationResult { + return notImplemented('customXml.parts.patch'); +} + +export function customXmlPartsRemoveWrapper( + _editor: Editor, + _input: CustomXmlPartsRemoveInput, + _options?: MutationOptions, +): CustomXmlPartsMutationResult { + return notImplemented('customXml.parts.remove'); +} + +// --------------------------------------------------------------------------- +// Adapter assembly +// --------------------------------------------------------------------------- + +export function createCustomXmlPartsAdapter(editor: Editor): CustomXmlPartsAdapter { + return { + list: (query) => customXmlPartsListWrapper(editor, query), + get: (input) => customXmlPartsGetWrapper(editor, input), + create: (input, options) => customXmlPartsCreateWrapper(editor, input, options), + patch: (input, options) => customXmlPartsPatchWrapper(editor, input, options), + remove: (input, options) => customXmlPartsRemoveWrapper(editor, input, options), + }; +} From b4a3ba7015e4dad89891103eaa062b2379f9b50a Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 12 May 2026 08:21:14 -0300 Subject: [PATCH 03/14] wip(document-api): customXml.parts write adapter + conformance vectors (SD-3105) create / patch / remove now implement the full OOXML package coordination instead of returning CAPABILITY_UNAVAILABLE. Write adapter (super-converter/custom-xml-parts.js): - createCustomXmlPart: generates fresh GUID itemID, allocates next free index, writes Storage Part + Properties Part + item rels + document-level relationship (5-file coordination) - patchCustomXmlPart: resolves by id or partName, replaces content and/or schemaRefs, preserves itemID. Creates a Properties Part on the fly when patching schemaRefs into a foreign part that doesn't have one yet. - removeCustomXmlPart: deletes storage, props, item rels, and the document-level relationship pointing at the part. Adapter wrappers (plan-engine/custom-xml-wrappers.ts): - All three writers call rejectTrackedMode (matches the contract declaration of supportsTrackedMode: false). - Errors map cleanly to INVALID_INPUT / TARGET_NOT_FOUND. - supportsDryRun set to false for v1; dry-run support can come later. Conformance: - contract-conformance.test.ts: throw/apply vectors registered for all three customXml.parts mutating ops. - contract.test.ts: customXml added to the validGroups list. Coverage: - 16 integration tests passing (read + write + round-trip). - 1195/1195 conformance tests passing. - 3392/3392 across the full document-api-adapters suite. - 1428/1428 across the document-api package suite. Round-trip test exports a created part to DOCX, reimports through the canonical loader, and verifies the itemID GUID, rootNamespace, schemaRefs, and content all survive. The 5-file package coordination is empirically OOXML-faithful. --- .../src/contract/contract.test.ts | 1 + .../src/contract/operation-definitions.ts | 6 +- .../core/super-converter/custom-xml-parts.js | 257 ++++++++++++++++++ .../contract-conformance.test.ts | 65 +++++ .../custom-xml-wrappers.integration.test.ts | 194 ++++++++++++- .../plan-engine/custom-xml-wrappers.ts | 75 +++-- 6 files changed, 555 insertions(+), 43 deletions(-) diff --git a/packages/document-api/src/contract/contract.test.ts b/packages/document-api/src/contract/contract.test.ts index e3424fdfbb..34d094c9dc 100644 --- a/packages/document-api/src/contract/contract.test.ts +++ b/packages/document-api/src/contract/contract.test.ts @@ -391,6 +391,7 @@ describe('document-api contract catalog', () => { 'diff', 'protection', 'permissionRanges', + 'customXml', ]; for (const id of OPERATION_IDS) { expect(validGroups, `${id} has invalid referenceGroup`).toContain(OPERATION_DEFINITIONS[id].referenceGroup); diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index 5aa6d4477f..f5fef77808 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -6295,7 +6295,7 @@ export const OPERATION_DEFINITIONS = { requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', - supportsDryRun: true, + supportsDryRun: false, supportsTrackedMode: false, possibleFailureCodes: NONE_FAILURES, throws: T_REF_INSERT, @@ -6310,7 +6310,7 @@ export const OPERATION_DEFINITIONS = { requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'idempotent', - supportsDryRun: true, + supportsDryRun: false, supportsTrackedMode: false, possibleFailureCodes: NONE_FAILURES, throws: T_REF_MUTATION, @@ -6325,7 +6325,7 @@ export const OPERATION_DEFINITIONS = { requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', - supportsDryRun: true, + supportsDryRun: false, supportsTrackedMode: false, possibleFailureCodes: NONE_FAILURES, throws: T_REF_MUTATION_REMOVE, diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index d0b869d5a7..3cc9d4258a 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -8,6 +8,9 @@ */ import * as xmljs from 'xml-js'; +import { v4 as uuidv4 } from 'uuid'; +import { resolveOpcTargetPath } from './helpers.js'; +import { DEFAULT_XML_DECLARATION } from './constants.js'; export const CUSTOM_XML_DATA_RELATIONSHIP_TYPE = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXml'; @@ -227,3 +230,257 @@ export function nextCustomXmlItemIndex(convertedXml) { while (used.has(candidate)) candidate += 1; return candidate; } + +// --------------------------------------------------------------------------- +// Write helpers (package coordination) +// --------------------------------------------------------------------------- + +function createXmlDocument(rootElement, declaration) { + const nextDeclaration = declaration ?? DEFAULT_XML_DECLARATION; + return { + declaration: { + ...nextDeclaration, + attributes: { ...nextDeclaration.attributes }, + }, + elements: [rootElement], + }; +} + +function parseContentToRootElement(content) { + const parsed = xmljs.xml2js(content, { compact: false }); + const root = (parsed.elements ?? []).find((el) => el?.type === 'element'); + if (!root) { + throw new Error('Custom XML content is missing a root element.'); + } + return { root, declaration: parsed.declaration ?? null }; +} + +function ensureDocumentRelationshipsRoot(convertedXml) { + if (!convertedXml['word/_rels/document.xml.rels']) { + convertedXml['word/_rels/document.xml.rels'] = createXmlDocument({ + type: 'element', + name: 'Relationships', + attributes: { + xmlns: 'http://schemas.openxmlformats.org/package/2006/relationships', + }, + elements: [], + }); + } + const relsData = convertedXml['word/_rels/document.xml.rels']; + relsData.elements ??= []; + let relsRoot = relsData.elements.find((el) => getLocalName(el?.name) === 'Relationships'); + if (!relsRoot) { + relsRoot = { + type: 'element', + name: 'Relationships', + attributes: { + xmlns: 'http://schemas.openxmlformats.org/package/2006/relationships', + }, + elements: [], + }; + relsData.elements.push(relsRoot); + } + relsRoot.elements ??= []; + return relsRoot; +} + +function getNextRelationshipId(relsRoot) { + const used = (relsRoot?.elements ?? []) + .map((rel) => { + const id = rel?.attributes?.Id; + const m = typeof id === 'string' ? /^rId(\d+)$/.exec(id) : null; + return m ? Number.parseInt(m[1], 10) : NaN; + }) + .filter((n) => Number.isFinite(n)); + const max = used.length > 0 ? Math.max(...used) : 0; + return `rId${max + 1}`; +} + +function buildDocumentRelTarget(partName) { + return partName.startsWith('customXml/') ? `../${partName}` : partName; +} + +function buildItemPropsRoot(itemId, schemaRefs) { + const schemaRefElements = (schemaRefs ?? []).map((uri) => ({ + type: 'element', + name: 'ds:schemaRef', + attributes: { 'ds:uri': uri }, + })); + return { + type: 'element', + name: 'ds:datastoreItem', + attributes: { + 'ds:itemID': itemId, + 'xmlns:ds': CUSTOM_XML_DATASTORE_NAMESPACE, + }, + elements: [ + { + type: 'element', + name: 'ds:schemaRefs', + elements: schemaRefElements, + }, + ], + }; +} + +function buildItemRelsRoot(propsPartFileName) { + return { + type: 'element', + name: 'Relationships', + attributes: { + xmlns: 'http://schemas.openxmlformats.org/package/2006/relationships', + }, + elements: [ + { + type: 'element', + name: 'Relationship', + attributes: { + Id: 'rId1', + Type: CUSTOM_XML_PROPS_RELATIONSHIP_TYPE, + Target: propsPartFileName, + }, + }, + ], + }; +} + +/** + * Locates the package part name for a given target. Returns null when the + * target can't be resolved (unknown id, unknown partName). + */ +export function resolveTargetPartName(convertedXml, target) { + if (!target) return null; + if (typeof target.partName === 'string' && target.partName.length > 0) { + return convertedXml[target.partName] ? target.partName : null; + } + if (typeof target.id === 'string' && target.id.length > 0) { + for (const partName of listCustomXmlStoragePartNames(convertedXml)) { + const propsName = findPropsPartFor(convertedXml, partName); + if (!propsName) continue; + const parsed = parsePropsPart(convertedXml[propsName]); + if (parsed?.itemId === target.id) return partName; + } + } + return null; +} + +/** + * Creates a new Custom XML Data Storage Part + its Properties Part, with + * the document-level relationship and the item rels file. Returns the + * generated itemID GUID and the package part names. + * + * @throws {Error} when `content` is not well-formed XML. + */ +export function createCustomXmlPart(convertedXml, { content, schemaRefs }) { + const { root, declaration } = parseContentToRootElement(content); + const index = nextCustomXmlItemIndex(convertedXml); + const partName = partNameFromIndex(index); + const propsPartName = propsPartNameFromIndex(index); + const itemRelsPath = `customXml/_rels/item${index}.xml.rels`; + const itemId = `{${uuidv4().toUpperCase()}}`; + + // Storage Part — wrap the customer's content in a fresh document envelope. + convertedXml[partName] = createXmlDocument(root, declaration); + + // Properties Part — datastoreItem with itemID + optional schemaRefs. + convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(itemId, schemaRefs ?? [])); + + // Item rels — link Storage Part → Properties Part. + convertedXml[itemRelsPath] = createXmlDocument(buildItemRelsRoot(`itemProps${index}.xml`)); + + // Document rel — link main document → Storage Part. + const relsRoot = ensureDocumentRelationshipsRoot(convertedXml); + relsRoot.elements.push({ + type: 'element', + name: 'Relationship', + attributes: { + Id: getNextRelationshipId(relsRoot), + Type: CUSTOM_XML_DATA_RELATIONSHIP_TYPE, + Target: buildDocumentRelTarget(partName), + }, + }); + + return { id: itemId, partName, propsPartName }; +} + +/** + * Replaces the content and/or schemaRefs of an existing part. Preserves + * the existing itemID and package part names. + * + * Returns `{ partName }` of the part that was patched, or `null` when + * the target couldn't be resolved. + * + * @throws {Error} when content is provided but not well-formed. + */ +export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs }) { + const partName = resolveTargetPartName(convertedXml, target); + if (!partName) return null; + + if (content !== undefined) { + const { root, declaration } = parseContentToRootElement(content); + const existingDecl = convertedXml[partName]?.declaration ?? declaration; + convertedXml[partName] = createXmlDocument(root, existingDecl); + } + + if (schemaRefs !== undefined) { + let propsPartName = findPropsPartFor(convertedXml, partName); + let itemId = null; + if (propsPartName) { + itemId = parsePropsPart(convertedXml[propsPartName])?.itemId ?? null; + } + if (!propsPartName) { + // Foreign part had no Properties Part; create one now so the + // schemaRefs we're writing actually land somewhere. + const idx = indexFromPartName(partName); + if (idx == null) return null; + propsPartName = propsPartNameFromIndex(idx); + const itemRelsPath = `customXml/_rels/item${idx}.xml.rels`; + itemId = `{${uuidv4().toUpperCase()}}`; + convertedXml[itemRelsPath] = createXmlDocument(buildItemRelsRoot(`itemProps${idx}.xml`)); + } + if (!itemId) itemId = `{${uuidv4().toUpperCase()}}`; + const existingDecl = convertedXml[propsPartName]?.declaration; + convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(itemId, schemaRefs), existingDecl); + } + + return { partName }; +} + +/** + * Removes a Custom XML Part and cleans up every linked package file: + * - the Storage Part + * - the Properties Part (if present) + * - the item rels file (if present) + * - the document-level relationship pointing at this part + * + * Returns `true` when the part existed and was removed, `false` when the + * target couldn't be resolved. + */ +export function removeCustomXmlPart(convertedXml, target) { + const partName = resolveTargetPartName(convertedXml, target); + if (!partName) return false; + const index = indexFromPartName(partName); + + delete convertedXml[partName]; + + if (index != null) { + const propsPartName = propsPartNameFromIndex(index); + if (convertedXml[propsPartName]) delete convertedXml[propsPartName]; + const itemRelsPath = `customXml/_rels/item${index}.xml.rels`; + if (convertedXml[itemRelsPath]) delete convertedXml[itemRelsPath]; + } + + // Strip the document-level relationship pointing at this part. + const relsRoot = convertedXml['word/_rels/document.xml.rels']?.elements?.find( + (el) => getLocalName(el?.name) === 'Relationships', + ); + if (relsRoot?.elements?.length) { + relsRoot.elements = relsRoot.elements.filter((rel) => { + if (rel?.attributes?.Type !== CUSTOM_XML_DATA_RELATIONSHIP_TYPE) return true; + const resolved = resolveOpcTargetPath(rel?.attributes?.Target, 'word'); + return resolved !== partName; + }); + } + + return true; +} diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts index 225c199d96..c305c030c0 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts @@ -196,6 +196,11 @@ import { bookmarksRenameWrapper, bookmarksRemoveWrapper, } from '../plan-engine/bookmark-wrappers.js'; +import { + customXmlPartsCreateWrapper, + customXmlPartsPatchWrapper, + customXmlPartsRemoveWrapper, +} from '../plan-engine/custom-xml-wrappers.js'; import { footnotesInsertWrapper, @@ -4044,6 +4049,66 @@ const refNamespaceMutationVectors: Partial> ); }, }, + + // ---- Custom XML Parts ---- + 'customXml.parts.create': { + throwCase: () => + customXmlPartsCreateWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + { content: '' }, + { changeMode: 'tracked' }, + ), + applyCase: () => + customXmlPartsCreateWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + { content: '' }, + { changeMode: 'direct' }, + ), + }, + 'customXml.parts.patch': { + throwCase: () => + customXmlPartsPatchWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + { target: { partName: 'customXml/item1.xml' }, content: '' }, + { changeMode: 'tracked' }, + ), + applyCase: () => { + const editor = makeRefEditor({ converter: { convertedXml: {} } }); + // Seed a part so the patch resolves. + customXmlPartsCreateWrapper( + editor, + { content: '' }, + { changeMode: 'direct' }, + ); + return customXmlPartsPatchWrapper( + editor, + { target: { partName: 'customXml/item1.xml' }, content: 'v2' }, + { changeMode: 'direct' }, + ); + }, + }, + 'customXml.parts.remove': { + throwCase: () => + customXmlPartsRemoveWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + { target: { partName: 'customXml/item1.xml' } }, + { changeMode: 'tracked' }, + ), + applyCase: () => { + const editor = makeRefEditor({ converter: { convertedXml: {} } }); + // Seed a part so the remove resolves. + customXmlPartsCreateWrapper( + editor, + { content: '' }, + { changeMode: 'direct' }, + ); + return customXmlPartsRemoveWrapper( + editor, + { target: { partName: 'customXml/item1.xml' } }, + { changeMode: 'direct' }, + ); + }, + }, }; const mutationVectors: Partial> = { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts index 44a6d82442..22ec1b4e3a 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -14,6 +14,7 @@ import { describe, expect, it } from 'vitest'; import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js'; +import { Editor } from '../../core/Editor.js'; const NAMESPACE = 'urn:test:1'; const PART_NAME = 'customXml/item1.xml'; @@ -175,34 +176,201 @@ describe('customXml.parts read-side (integration)', () => { }); }); -describe('customXml.parts write-side (stubs)', () => { - it('create returns CAPABILITY_UNAVAILABLE until Phase B lands', async () => { +describe('customXml.parts write-side', () => { + it('create makes a part discoverable via list and get', async () => { const editor = await createEditorWithEmptyPackage(); - const result = editor.doc.customXml.parts.create({ content: '' }); - expect(result.success).toBe(false); - if (!result.success) { - expect(result.failure.code).toBe('CAPABILITY_UNAVAILABLE'); - } + + const created = editor.doc.customXml.parts.create({ + content: '', + schemaRefs: ['urn:test:1'], + }); + expect(created.success).toBe(true); + if (!created.success) return; + expect(created.id).toMatch(/^\{[0-9A-F-]+\}$/); + expect(created.partName).toBe('customXml/item1.xml'); + expect(created.propsPartName).toBe('customXml/itemProps1.xml'); + + const list = editor.doc.customXml.parts.list(); + expect(list.items.length).toBe(1); + const summary = list.items[0]!; + expect(summary.id).toBe(created.id); + expect(summary.rootNamespace).toBe('urn:test:1'); + expect(summary.schemaRefs).toEqual(['urn:test:1']); + + const info = editor.doc.customXml.parts.get({ target: { id: created.id } }); + expect(info).not.toBeNull(); + expect(info!.content).toContain(' { + it('create allocates non-colliding indexes when called multiple times', async () => { const editor = await createEditorWithEmptyPackage(); - const result = editor.doc.customXml.parts.patch({ target: { id: '{X}' }, content: '' }); + const a = editor.doc.customXml.parts.create({ content: '' }); + const b = editor.doc.customXml.parts.create({ content: '' }); + expect(a.success && b.success).toBe(true); + if (!a.success || !b.success) return; + expect(a.partName).toBe('customXml/item1.xml'); + expect(b.partName).toBe('customXml/item2.xml'); + expect(a.id).not.toBe(b.id); + expect(editor.doc.customXml.parts.list().items.length).toBe(2); + editor.destroy(); + }); + + it('create wires up the document-level relationship', async () => { + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ content: '' }); + expect(created.success).toBe(true); + + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + const relsDoc = converted['word/_rels/document.xml.rels'] as { elements?: Array<{ elements?: Array<{ attributes?: Record }> }> } | undefined; + const relsRoot = relsDoc?.elements?.[0]; + const customXmlRels = (relsRoot?.elements ?? []).filter( + (rel) => + rel?.attributes?.Type === + 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXml', + ); + expect(customXmlRels.length).toBe(1); + expect(customXmlRels[0]!.attributes?.Target).toBe('../customXml/item1.xml'); + + editor.destroy(); + }); + + it('patch updates content while preserving itemID', async () => { + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ + content: 'one', + schemaRefs: ['urn:a'], + }); + expect(created.success).toBe(true); + if (!created.success) return; + + const patched = editor.doc.customXml.parts.patch({ + target: { id: created.id }, + content: 'two', + }); + expect(patched.success).toBe(true); + + const info = editor.doc.customXml.parts.get({ target: { id: created.id } }); + expect(info!.id).toBe(created.id); + expect(info!.content).toContain('>two<'); + expect(info!.content).not.toContain('>one<'); + expect(info!.schemaRefs).toEqual(['urn:a']); // preserved + editor.destroy(); + }); + + it('patch can update schemaRefs alone', async () => { + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ + content: '', + schemaRefs: ['urn:a'], + }); + if (!created.success) return; + + const patched = editor.doc.customXml.parts.patch({ + target: { id: created.id }, + schemaRefs: ['urn:a', 'urn:b'], + }); + expect(patched.success).toBe(true); + + const info = editor.doc.customXml.parts.get({ target: { id: created.id } }); + expect(info!.schemaRefs).toEqual(['urn:a', 'urn:b']); + editor.destroy(); + }); + + it('patch returns TARGET_NOT_FOUND for unknown id', async () => { + const editor = await createEditorWithEmptyPackage(); + const result = editor.doc.customXml.parts.patch({ + target: { id: '{NOPE}' }, + content: '', + }); expect(result.success).toBe(false); if (!result.success) { - expect(result.failure.code).toBe('CAPABILITY_UNAVAILABLE'); + expect(result.failure.code).toBe('TARGET_NOT_FOUND'); } editor.destroy(); }); - it('remove returns CAPABILITY_UNAVAILABLE', async () => { + it('remove deletes the part and its linked package files', async () => { + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ content: '' }); + if (!created.success) return; + + const removed = editor.doc.customXml.parts.remove({ target: { id: created.id } }); + expect(removed.success).toBe(true); + + expect(editor.doc.customXml.parts.list().items).toEqual([]); + + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + expect(converted['customXml/item1.xml']).toBeUndefined(); + expect(converted['customXml/itemProps1.xml']).toBeUndefined(); + expect(converted['customXml/_rels/item1.xml.rels']).toBeUndefined(); + + const relsDoc = converted['word/_rels/document.xml.rels'] as { elements?: Array<{ elements?: Array<{ attributes?: Record }> }> } | undefined; + const relsRoot = relsDoc?.elements?.[0]; + const lingering = (relsRoot?.elements ?? []).filter( + (rel) => + rel?.attributes?.Type === + 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXml', + ); + expect(lingering).toEqual([]); + + editor.destroy(); + }); + + it('remove returns TARGET_NOT_FOUND for unknown id', async () => { const editor = await createEditorWithEmptyPackage(); - const result = editor.doc.customXml.parts.remove({ target: { id: '{X}' } }); + const result = editor.doc.customXml.parts.remove({ target: { id: '{NOPE}' } }); expect(result.success).toBe(false); if (!result.success) { - expect(result.failure.code).toBe('CAPABILITY_UNAVAILABLE'); + expect(result.failure.code).toBe('TARGET_NOT_FOUND'); } editor.destroy(); }); + + it('round-trip: create → export → reimport preserves id, content, schemaRefs', async () => { + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ + content: '', + schemaRefs: ['urn:round-trip:1', 'urn:round-trip:audit'], + }); + if (!created.success) return; + const originalId = created.id; + + const buf = (await editor.exportDocx()) as Buffer | Uint8Array; + const bytes = buf instanceof Uint8Array ? buf : new Uint8Array(buf); + editor.destroy(); + + // Reimport from the exported bytes through the canonical loader. + const [reloadedDocx, reloadedMedia, reloadedMediaFiles, reloadedFonts] = await Editor.loadXmlData( + bytes, + true, + ); + const { editor: reloaded } = initTestEditor({ + content: reloadedDocx, + media: reloadedMedia, + mediaFiles: reloadedMediaFiles, + fonts: reloadedFonts, + useImmediateSetTimeout: false, + isHeadless: true, + user: { name: 'Test', email: 'test@example.com' }, + }); + + const list = reloaded.doc.customXml.parts.list(); + expect(list.items.length).toBe(1); + const summary = list.items[0]!; + expect(summary.id).toBe(originalId); + expect(summary.rootNamespace).toBe('urn:round-trip:1'); + expect(summary.schemaRefs).toEqual(['urn:round-trip:1', 'urn:round-trip:audit']); + + const info = reloaded.doc.customXml.parts.get({ target: { id: originalId } }); + expect(info!.content).toContain(' Date: Tue, 12 May 2026 08:48:57 -0300 Subject: [PATCH 04/14] =?UTF-8?q?wip(document-api):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20safety,=20lifecycle,=20rels=20pairing,=20tombstones?= =?UTF-8?q?=20(SD-3105)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All six findings from the SD-3105 review: #1 (High) partName scoping - resolveTargetPartName and readCustomXmlPart now require the path to match customXml/itemN.xml. Targets like word/document.xml are rejected cleanly instead of letting through. #2 (High) bypassed mutation lifecycle - create/patch/remove now route through executeOutOfBandMutation, the same shared primitive citation sources use. Each call gets: * expectedRevision check * dryRun preview path * dirty marking + GUID promotion * revision increment on actual change - supportsDryRun: true on all three ops with real dry-run validation (well-formedness for create/patch, target resolution for patch/remove). #3 (High) deletion didn't persist for imported DOCX parts - removeCustomXmlPart now stamps the removed paths into a converter.removedCustomXmlPaths set. Editor.ts export loop emits updatedDocs[path] = null for each entry, so original-zip entries are tombstoned instead of being copied through. #4 (Medium) props parts paired by filename - findPropsPartFor now reads customXml/_rels/itemN.xml.rels and follows the Type=customXmlProps relationship. Falls back to the index-name heuristic only when no rels file exists. Foreign docs with non- matching names now resolve correctly. #5 (Medium) contract metadata lied about failures - possibleFailureCodes updated to actual codes: ['INVALID_INPUT'], ['TARGET_NOT_FOUND', 'INVALID_INPUT'], ['TARGET_NOT_FOUND']. #6 (Medium) JSON schemas didn't match runtime - get output now { oneOf: [{ type: 'object' }, { type: 'null' }] }. - patch input encodes 'at least one of content or schemaRefs' via anyOf, with additionalProperties: false. - content fields gain minLength: 1. Coverage update: - Two new integration tests assert #1 (partName rejection on word/document.xml etc) and #4 (foreign-name props resolved via rels). - failureCase and dryRun vectors added for all three customXml.parts mutating ops in the conformance suite. Verified: - @superdoc/super-editor: 3398/3398 across 123 files - @superdoc/document-api: 1428/1428 across 51 files --- .../src/contract/operation-definitions.ts | 12 +- packages/document-api/src/contract/schemas.ts | 23 ++-- .../src/editors/v1/core/Editor.ts | 12 ++ .../core/super-converter/custom-xml-parts.js | 84 +++++++++++--- .../contract-conformance.test.ts | 46 ++++++++ .../custom-xml-wrappers.integration.test.ts | 59 ++++++++++ .../plan-engine/custom-xml-wrappers.ts | 109 +++++++++++++++--- 7 files changed, 299 insertions(+), 46 deletions(-) diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index f5fef77808..c2f14f1034 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -6295,9 +6295,9 @@ export const OPERATION_DEFINITIONS = { requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', - supportsDryRun: false, + supportsDryRun: true, supportsTrackedMode: false, - possibleFailureCodes: NONE_FAILURES, + possibleFailureCodes: ['INVALID_INPUT'], throws: T_REF_INSERT, }), referenceDocPath: 'custom-xml/parts/create.mdx', @@ -6310,9 +6310,9 @@ export const OPERATION_DEFINITIONS = { requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'idempotent', - supportsDryRun: false, + supportsDryRun: true, supportsTrackedMode: false, - possibleFailureCodes: NONE_FAILURES, + possibleFailureCodes: ['TARGET_NOT_FOUND', 'INVALID_INPUT'], throws: T_REF_MUTATION, }), referenceDocPath: 'custom-xml/parts/patch.mdx', @@ -6325,9 +6325,9 @@ export const OPERATION_DEFINITIONS = { requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', - supportsDryRun: false, + supportsDryRun: true, supportsTrackedMode: false, - possibleFailureCodes: NONE_FAILURES, + possibleFailureCodes: ['TARGET_NOT_FOUND'], throws: T_REF_MUTATION_REMOVE, }), referenceDocPath: 'custom-xml/parts/remove.mdx', diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index a6fb7cf113..efbf4331f3 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -7664,27 +7664,32 @@ const operationSchemas: Record = { }, 'customXml.parts.get': { input: objectSchema({ target: customXmlPartTargetSchema }, ['target']), - output: { type: 'object' }, + output: { oneOf: [{ type: 'object' }, { type: 'null' }] }, }, 'customXml.parts.create': { input: objectSchema( { - content: { type: 'string' }, - schemaRefs: { type: 'array', items: { type: 'string' } }, + content: { type: 'string', minLength: 1 }, + schemaRefs: { type: 'array', items: { type: 'string', minLength: 1 } }, }, ['content'], ), ...customXmlPartCreateMutation, }, 'customXml.parts.patch': { - input: objectSchema( - { + // `target` is required; `content` and `schemaRefs` are both optional + // but at least one MUST be present. Encoded via JSON Schema's `anyOf`. + input: { + type: 'object', + properties: { target: customXmlPartTargetSchema, - content: { type: 'string' }, - schemaRefs: { type: 'array', items: { type: 'string' } }, + content: { type: 'string', minLength: 1 }, + schemaRefs: { type: 'array', items: { type: 'string', minLength: 1 } }, }, - ['target'], - ), + required: ['target'], + anyOf: [{ required: ['content'] }, { required: ['schemaRefs'] }], + additionalProperties: false, + }, ...customXmlPartMutation, }, 'customXml.parts.remove': { diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index 60777862ae..533ac17ec4 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -3331,6 +3331,18 @@ export class Editor extends EventEmitter { } } + // Emit ZIP tombstones for custom XML parts that were removed via the + // Document API but originated in the imported DOCX. Without this, + // the exporter would copy the original zip entry through, and the + // removed part would reappear on the next import. + const removedCustomXmlPaths = (this.converter as unknown as { removedCustomXmlPaths?: Set }) + .removedCustomXmlPaths; + if (removedCustomXmlPaths instanceof Set) { + for (const path of removedCustomXmlPaths) { + updatedDocs[path] = null; + } + } + const zipper = new DocxZipper(); if (getUpdatedDocs) { diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index 3cc9d4258a..93be2e87bb 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -59,6 +59,15 @@ function indexFromPropsPartName(propsPartName) { return m ? Number.parseInt(m[1], 10) : null; } +/** + * Returns true when `partName` is the path of a Custom XML Data Storage Part. + * Used to reject `target.partName` values that point at unrelated package + * files (e.g. `word/document.xml`, `word/styles.xml`, `[Content_Types].xml`). + */ +export function isCustomXmlStoragePartName(partName) { + return indexFromPartName(partName) != null; +} + // --------------------------------------------------------------------------- // Discovery // --------------------------------------------------------------------------- @@ -83,14 +92,39 @@ export function listCustomXmlStoragePartNames(convertedXml) { } /** - * Returns the Properties Part name paired with `partName`, if present. - * Pairs by matching numeric index (item1 ↔ itemProps1). + * Returns the Properties Part name paired with `partName` via the OOXML + * relationship file `customXml/_rels/itemN.xml.rels`. Falls back to the + * index-match heuristic (`itemN.xml → itemPropsN.xml`) when no rels file + * is present. + * + * Pairing via the rels file is required by ECMA-376 §15.2.6 — foreign + * docs are not obligated to name their props parts to match. */ export function findPropsPartFor(convertedXml, partName) { + if (!convertedXml) return null; const idx = indexFromPartName(partName); if (idx == null) return null; - const candidate = propsPartNameFromIndex(idx); - return convertedXml?.[candidate] ? candidate : null; + + const relsPath = `customXml/_rels/item${idx}.xml.rels`; + const relsDoc = convertedXml[relsPath]; + const relsRoot = relsDoc?.elements?.find((el) => getLocalName(el?.name) === 'Relationships'); + if (relsRoot?.elements?.length) { + for (const rel of relsRoot.elements) { + if (rel?.attributes?.Type !== CUSTOM_XML_PROPS_RELATIONSHIP_TYPE) continue; + const target = rel?.attributes?.Target; + if (typeof target !== 'string' || target.length === 0) continue; + // Targets in customXml/_rels/itemN.xml.rels are relative to the + // rels file's base, i.e. `customXml/`. So `itemPropsN.xml` → + // `customXml/itemPropsN.xml`. Absolute or otherwise-prefixed + // targets are accepted as-is when they already point at a key. + const candidate = target.includes('/') ? target.replace(/^\.?\//, '') : `customXml/${target}`; + if (convertedXml[candidate]) return candidate; + } + } + + // Fallback: index-name heuristic for parts without a rels file. + const indexCandidate = propsPartNameFromIndex(idx); + return convertedXml[indexCandidate] ? indexCandidate : null; } // --------------------------------------------------------------------------- @@ -168,6 +202,8 @@ export function readCustomXmlPart(convertedXml, target) { let partName = null; let itemId = null; if (typeof target.partName === 'string' && target.partName.length > 0) { + // Reject non-storage-part paths. See note on resolveTargetPartName. + if (!isCustomXmlStoragePartName(target.partName)) return null; partName = target.partName; } else if (typeof target.id === 'string' && target.id.length > 0) { itemId = target.id; @@ -351,6 +387,10 @@ function buildItemRelsRoot(propsPartFileName) { export function resolveTargetPartName(convertedXml, target) { if (!target) return null; if (typeof target.partName === 'string' && target.partName.length > 0) { + // Restrict targeting by partName to actual Storage Parts. Without this + // gate, `customXml.parts.get/patch/remove({ partName })` could read or + // mutate unrelated package files like `word/document.xml`. + if (!isCustomXmlStoragePartName(target.partName)) return null; return convertedXml[target.partName] ? target.partName : null; } if (typeof target.id === 'string' && target.id.length > 0) { @@ -449,25 +489,31 @@ export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs } /** * Removes a Custom XML Part and cleans up every linked package file: * - the Storage Part - * - the Properties Part (if present) - * - the item rels file (if present) + * - the Properties Part (resolved via the item rels file) + * - the item rels file * - the document-level relationship pointing at this part * + * Paths of removed parts are tracked on `converter.removedCustomXmlPaths` + * so the exporter can emit ZIP tombstones (`updatedDocs[path] = null`) for + * parts that originated in the imported DOCX — otherwise the original + * entries would survive in the exported zip and the part would reappear + * on the next import. + * * Returns `true` when the part existed and was removed, `false` when the * target couldn't be resolved. */ -export function removeCustomXmlPart(convertedXml, target) { +export function removeCustomXmlPart(convertedXml, target, converter) { const partName = resolveTargetPartName(convertedXml, target); if (!partName) return false; const index = indexFromPartName(partName); + const propsPartName = findPropsPartFor(convertedXml, partName); + const itemRelsPath = index == null ? null : `customXml/_rels/item${index}.xml.rels`; + const removedPaths = [partName, propsPartName, itemRelsPath].filter( + (path) => typeof path === 'string' && path.length > 0, + ); - delete convertedXml[partName]; - - if (index != null) { - const propsPartName = propsPartNameFromIndex(index); - if (convertedXml[propsPartName]) delete convertedXml[propsPartName]; - const itemRelsPath = `customXml/_rels/item${index}.xml.rels`; - if (convertedXml[itemRelsPath]) delete convertedXml[itemRelsPath]; + for (const path of removedPaths) { + delete convertedXml[path]; } // Strip the document-level relationship pointing at this part. @@ -482,5 +528,15 @@ export function removeCustomXmlPart(convertedXml, target) { }); } + // Mark the paths as removed so the exporter emits null tombstones for + // them. Without this, an existing DOCX with these parts in the original + // zip would still ship them on export. + if (converter) { + if (!(converter.removedCustomXmlPaths instanceof Set)) { + converter.removedCustomXmlPaths = new Set(); + } + for (const path of removedPaths) converter.removedCustomXmlPaths.add(path); + } + return true; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts index c305c030c0..8916f96f1f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts @@ -4064,6 +4064,13 @@ const refNamespaceMutationVectors: Partial> { content: '' }, { changeMode: 'direct' }, ), + failureCase: () => + customXmlPartsCreateWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + // Malformed XML — adapter rejects with INVALID_INPUT. + { content: ' @@ -4086,6 +4093,13 @@ const refNamespaceMutationVectors: Partial> { changeMode: 'direct' }, ); }, + failureCase: () => + customXmlPartsPatchWrapper( + // Empty convertedXml — target can't resolve. + makeRefEditor({ converter: { convertedXml: {} } }), + { target: { partName: 'customXml/item999.xml' }, content: '' }, + { changeMode: 'direct' }, + ), }, 'customXml.parts.remove': { throwCase: () => @@ -4108,6 +4122,12 @@ const refNamespaceMutationVectors: Partial> { changeMode: 'direct' }, ); }, + failureCase: () => + customXmlPartsRemoveWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + { target: { partName: 'customXml/item999.xml' } }, + { changeMode: 'direct' }, + ), }, }; @@ -11233,6 +11253,32 @@ const dryRunVectors: Partial unknown>> = { { changeMode: 'direct', dryRun: true }, ); }, + + // ---- Custom XML Parts ---- + 'customXml.parts.create': () => + customXmlPartsCreateWrapper( + makeRefEditor({ converter: { convertedXml: {} } }), + { content: '' }, + { changeMode: 'direct', dryRun: true }, + ), + 'customXml.parts.patch': () => { + const editor = makeRefEditor({ converter: { convertedXml: {} } }); + customXmlPartsCreateWrapper(editor, { content: '' }, { changeMode: 'direct' }); + return customXmlPartsPatchWrapper( + editor, + { target: { partName: 'customXml/item1.xml' }, content: 'v2' }, + { changeMode: 'direct', dryRun: true }, + ); + }, + 'customXml.parts.remove': () => { + const editor = makeRefEditor({ converter: { convertedXml: {} } }); + customXmlPartsCreateWrapper(editor, { content: '' }, { changeMode: 'direct' }); + return customXmlPartsRemoveWrapper( + editor, + { target: { partName: 'customXml/item1.xml' } }, + { changeMode: 'direct', dryRun: true }, + ); + }, }; beforeAll(() => { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts index 22ec1b4e3a..bb56ef70ac 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -174,6 +174,65 @@ describe('customXml.parts read-side (integration)', () => { expect(info).toBeNull(); editor.destroy(); }); + + it('rejects partName targets that point at non-storage-part files', async () => { + const editor = await createEditorWithEmptyPackage(); + // get returns null (not the document content). + expect(editor.doc.customXml.parts.get({ target: { partName: 'word/document.xml' } })).toBeNull(); + expect(editor.doc.customXml.parts.get({ target: { partName: '[Content_Types].xml' } })).toBeNull(); + // patch and remove return TARGET_NOT_FOUND, not a successful mutation. + const patch = editor.doc.customXml.parts.patch({ + target: { partName: 'word/document.xml' }, + content: '', + }); + expect(patch.success).toBe(false); + if (!patch.success) expect(patch.failure.code).toBe('TARGET_NOT_FOUND'); + const remove = editor.doc.customXml.parts.remove({ target: { partName: 'word/document.xml' } }); + expect(remove.success).toBe(false); + if (!remove.success) expect(remove.failure.code).toBe('TARGET_NOT_FOUND'); + editor.destroy(); + }); + + it('pairs storage and props parts via the item rels file, not by filename', async () => { + // Foreign doc shape: item1.xml is linked to itemPropsFOREIGN.xml via + // customXml/_rels/item1.xml.rels. The index-match heuristic would + // miss the props; the rels-based pairing must find it. + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted['customXml/itemPropsFOREIGN.xml'] = makePropsDoc(ITEM_ID, [NAMESPACE]); + converted['customXml/_rels/item1.xml.rels'] = { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8' } }, + elements: [ + { + type: 'element', + name: 'Relationships', + attributes: { xmlns: 'http://schemas.openxmlformats.org/package/2006/relationships' }, + elements: [ + { + type: 'element', + name: 'Relationship', + attributes: { + Id: 'rId1', + Type: 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXmlProps', + Target: 'itemPropsFOREIGN.xml', + }, + }, + ], + }, + ], + }; + + const list = editor.doc.customXml.parts.list(); + expect(list.items.length).toBe(1); + const item = list.items[0]!; + expect(item.id).toBe(ITEM_ID); + expect(item.propsPartName).toBe('customXml/itemPropsFOREIGN.xml'); + expect(item.schemaRefs).toEqual([NAMESPACE]); + + editor.destroy(); + }); }); describe('customXml.parts write-side', () => { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts index 2c3e6877b3..b67e08ae97 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts @@ -17,12 +17,14 @@ import { buildDiscoveryItem, buildDiscoveryResult, buildResolvedHandle } from '@ import { paginate } from '../helpers/adapter-utils.js'; import { getRevision } from './revision-tracker.js'; import { rejectTrackedMode } from '../helpers/mutation-helpers.js'; +import { executeOutOfBandMutation } from '../out-of-band-mutation.js'; import { listCustomXmlParts, readCustomXmlPart, createCustomXmlPart, patchCustomXmlPart, removeCustomXmlPart, + resolveTargetPartName, } from '../../core/super-converter/custom-xml-parts.js'; // --------------------------------------------------------------------------- @@ -31,11 +33,15 @@ import { type ConverterWithConvertedXml = { convertedXml?: Record; + removedCustomXmlPaths?: Set; }; +function getConverter(editor: Editor): ConverterWithConvertedXml | null { + return (editor as unknown as { converter?: ConverterWithConvertedXml }).converter ?? null; +} + function getConvertedXml(editor: Editor): Record { - const converter = (editor as unknown as { converter?: ConverterWithConvertedXml }).converter; - return converter?.convertedXml ?? {}; + return getConverter(editor)?.convertedXml ?? {}; } // --------------------------------------------------------------------------- @@ -113,10 +119,21 @@ export function customXmlPartsGetWrapper( // Write operations // --------------------------------------------------------------------------- -function failure(code: string, message: string): { success: false; failure: { code: string; message: string } } { +type FailureCode = 'INVALID_INPUT' | 'TARGET_NOT_FOUND'; + +function failure( + code: FailureCode, + message: string, +): { success: false; failure: { code: FailureCode; message: string } } { return { success: false, failure: { code, message } }; } +type WriteOutcome = { ok: true; payload: T } | { ok: false; code: FailureCode; message: string }; + +function targetNotFound(): WriteOutcome { + return { ok: false, code: 'TARGET_NOT_FOUND', message: 'No custom XML part matched the supplied target.' }; +} + export function customXmlPartsCreateWrapper( editor: Editor, input: CustomXmlPartsCreateInput, @@ -124,15 +141,38 @@ export function customXmlPartsCreateWrapper( ): CustomXmlPartsCreateResult { rejectTrackedMode('customXml.parts.create', options); try { - const result = createCustomXmlPart(getConvertedXml(editor), { - content: input.content, - schemaRefs: input.schemaRefs, - }); + const outcome = executeOutOfBandMutation< + WriteOutcome<{ id: string; partName: string; propsPartName: string }> + >( + editor, + (dryRun) => { + if (dryRun) { + // Read-only preview: validate well-formedness without writing. + try { + createCustomXmlPart({}, { content: input.content, schemaRefs: input.schemaRefs }); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + return { changed: false, payload: { ok: false, code: 'INVALID_INPUT', message: msg } }; + } + return { + changed: false, + payload: { ok: true, payload: { id: '{DRY-RUN}', partName: '', propsPartName: '' } }, + }; + } + const created = createCustomXmlPart(getConvertedXml(editor), { + content: input.content, + schemaRefs: input.schemaRefs, + }); + return { changed: true, payload: { ok: true, payload: created } }; + }, + { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, + ); + if (!outcome.ok) return failure(outcome.code, outcome.message); return { success: true, - id: result.id, - partName: result.partName, - propsPartName: result.propsPartName, + id: outcome.payload.id, + partName: outcome.payload.partName, + propsPartName: outcome.payload.propsPartName, }; } catch (e) { const msg = e instanceof Error ? e.message : String(e); @@ -147,11 +187,32 @@ export function customXmlPartsPatchWrapper( ): CustomXmlPartsMutationResult { rejectTrackedMode('customXml.parts.patch', options); try { - const result = patchCustomXmlPart(getConvertedXml(editor), input.target, { - content: input.content, - schemaRefs: input.schemaRefs, - }); - if (!result) return failure('TARGET_NOT_FOUND', 'No custom XML part matched the supplied target.'); + const outcome = executeOutOfBandMutation>( + editor, + (dryRun) => { + if (dryRun) { + const partName = resolveTargetPartName(getConvertedXml(editor), input.target); + if (!partName) return { changed: false, payload: targetNotFound() }; + if (input.content !== undefined) { + try { + createCustomXmlPart({}, { content: input.content, schemaRefs: undefined }); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + return { changed: false, payload: { ok: false, code: 'INVALID_INPUT', message: msg } }; + } + } + return { changed: false, payload: { ok: true, payload: true } }; + } + const patched = patchCustomXmlPart(getConvertedXml(editor), input.target, { + content: input.content, + schemaRefs: input.schemaRefs, + }); + if (!patched) return { changed: false, payload: targetNotFound() }; + return { changed: true, payload: { ok: true, payload: true } }; + }, + { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, + ); + if (!outcome.ok) return failure(outcome.code, outcome.message); return { success: true, target: input.target }; } catch (e) { const msg = e instanceof Error ? e.message : String(e); @@ -165,8 +226,22 @@ export function customXmlPartsRemoveWrapper( options?: MutationOptions, ): CustomXmlPartsMutationResult { rejectTrackedMode('customXml.parts.remove', options); - const ok = removeCustomXmlPart(getConvertedXml(editor), input.target); - if (!ok) return failure('TARGET_NOT_FOUND', 'No custom XML part matched the supplied target.'); + const outcome = executeOutOfBandMutation>( + editor, + (dryRun) => { + if (dryRun) { + const partName = resolveTargetPartName(getConvertedXml(editor), input.target); + return partName + ? { changed: false, payload: { ok: true, payload: true } } + : { changed: false, payload: targetNotFound() }; + } + const ok = removeCustomXmlPart(getConvertedXml(editor), input.target, getConverter(editor)); + if (!ok) return { changed: false, payload: targetNotFound() }; + return { changed: true, payload: { ok: true, payload: true } }; + }, + { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, + ); + if (!outcome.ok) return failure(outcome.code, outcome.message); return { success: true, target: input.target }; } From fb03693e92a645c0ab08e3ec00c54468d2287fea Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 12 May 2026 09:22:00 -0300 Subject: [PATCH 05/14] wip(document-api): preserve revision errors + fix tombstone collision (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness fixes from the second review pass: #1 Broad catch was swallowing REVISION_MISMATCH customXml.parts.create and patch wrapped the entire executeOutOfBandMutation call in a try/catch that converted everything to INVALID_INPUT. Lifecycle errors from checkRevision (REVISION_MISMATCH) and any future PlanError propagation were being eaten. Replaced the outer try/catch with a scoped safeValidate helper that only catches content-parsing errors from createCustomXmlPart / patchCustomXmlPart, returning them as structured INVALID_INPUT outcomes. The executeOutOfBandMutation call itself now lets revision and other lifecycle errors bubble. Also reordered patch: target resolution runs FIRST, so a missing target reports TARGET_NOT_FOUND instead of (potentially) INVALID_INPUT if patch happened to throw. #2 Tombstone could null a newly-created part on the same index remove → create on a recycled index (customXml/item1.xml) had this sequence: remove writes 'customXml/item1.xml' to the tombstone set; create reuses index 1 because convertedXml has no item1.xml; export serializes the new part, then the tombstone loop runs and overwrites updatedDocs['customXml/item1.xml'] with null, deleting the freshly-created part from the exported zip. Fix: createCustomXmlPart now removes its written paths (partName, propsPartName, itemRelsPath) from converter.removedCustomXmlPaths whenever a converter is passed. The new integration test exercises the exact remove → create → export → reimport sequence and asserts the new part survives with its fresh id and content. Coverage: - 19/19 integration tests passing (incl. the new tombstone test). - 3401/3401 super-editor document-api-adapters tests. - 1428/1428 document-api package tests. --- .../core/super-converter/custom-xml-parts.js | 24 ++- .../custom-xml-wrappers.integration.test.ts | 71 +++++++++ .../plan-engine/custom-xml-wrappers.ts | 145 ++++++++++-------- 3 files changed, 170 insertions(+), 70 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index 93be2e87bb..b66d1b15a0 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -256,12 +256,17 @@ export function listCustomXmlParts(convertedXml) { // Index allocation (write side helper, also useful for tests) // --------------------------------------------------------------------------- -export function nextCustomXmlItemIndex(convertedXml) { +export function nextCustomXmlItemIndex(convertedXml, converter) { const used = new Set(); for (const path of Object.keys(convertedXml ?? {})) { const idx = indexFromPartName(path) ?? indexFromPropsPartName(path); if (idx != null) used.add(idx); } + // Reusing an index that was tombstoned for export is safe because the + // tombstone is cleared when the new part is written (see + // createCustomXmlPart). Listing them here would be unnecessarily + // conservative and force ever-growing indexes. + void converter; let candidate = 1; while (used.has(candidate)) candidate += 1; return candidate; @@ -409,11 +414,15 @@ export function resolveTargetPartName(convertedXml, target) { * the document-level relationship and the item rels file. Returns the * generated itemID GUID and the package part names. * + * When `converter` is provided and the chosen part path was previously + * tombstoned (via removeCustomXmlPart), the tombstone is cleared — the + * exporter would otherwise null the new part on save. + * * @throws {Error} when `content` is not well-formed XML. */ -export function createCustomXmlPart(convertedXml, { content, schemaRefs }) { +export function createCustomXmlPart(convertedXml, { content, schemaRefs }, converter) { const { root, declaration } = parseContentToRootElement(content); - const index = nextCustomXmlItemIndex(convertedXml); + const index = nextCustomXmlItemIndex(convertedXml, converter); const partName = partNameFromIndex(index); const propsPartName = propsPartNameFromIndex(index); const itemRelsPath = `customXml/_rels/item${index}.xml.rels`; @@ -440,6 +449,15 @@ export function createCustomXmlPart(convertedXml, { content, schemaRefs }) { }, }); + // Clear any tombstones that match the paths we just wrote. Without this, + // a sequence `remove → create` that recycles an index (`item1.xml`) + // would let the exporter null the brand-new part on save. + if (converter?.removedCustomXmlPaths instanceof Set) { + converter.removedCustomXmlPaths.delete(partName); + converter.removedCustomXmlPaths.delete(propsPartName); + converter.removedCustomXmlPaths.delete(itemRelsPath); + } + return { id: itemId, partName, propsPartName }; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts index bb56ef70ac..5811fc4748 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -390,6 +390,77 @@ describe('customXml.parts write-side', () => { editor.destroy(); }); + it('remove → create on the same index does not tombstone the new part on export', async () => { + // Reproduces the collision: a foreign DOCX has item1.xml, the + // customer removes it (which records a tombstone), then creates a + // fresh part. nextCustomXmlItemIndex returns 1 (recycling), and + // without the tombstone-clear the exporter would null the new + // part. With the clear, the new part survives export+reimport. + + // Seed: simulate a foreign DOCX with item1.xml already present + // by creating and round-tripping through export+reimport. + let editor = await createEditorWithEmptyPackage(); + const seeded = editor.doc.customXml.parts.create({ + content: '', + }); + expect(seeded.success).toBe(true); + + const seededBytes = (await editor.exportDocx()) as Buffer | Uint8Array; + const seededBuf = seededBytes instanceof Uint8Array ? seededBytes : new Uint8Array(seededBytes); + editor.destroy(); + + const [docx0, media0, mf0, fonts0] = await Editor.loadXmlData(seededBuf, true); + ({ editor } = initTestEditor({ + content: docx0, + media: media0, + mediaFiles: mf0, + fonts: fonts0, + useImmediateSetTimeout: false, + isHeadless: true, + user: { name: 'Test', email: 'test@example.com' }, + })); + + // Confirm the seed survived. + const beforeRemoveList = editor.doc.customXml.parts.list(); + expect(beforeRemoveList.items.length).toBe(1); + const originalId = beforeRemoveList.items[0]!.id!; + + // Remove the original, then create a new part. Without the + // tombstone-clear, the new part lands on customXml/item1.xml and the + // exporter nulls it out. + const removed = editor.doc.customXml.parts.remove({ target: { id: originalId } }); + expect(removed.success).toBe(true); + const created = editor.doc.customXml.parts.create({ content: '' }); + expect(created.success).toBe(true); + if (!created.success) return; + expect(created.partName).toBe('customXml/item1.xml'); + + // Export + reimport. The new part must survive. + const finalBytes = (await editor.exportDocx()) as Buffer | Uint8Array; + const finalBuf = finalBytes instanceof Uint8Array ? finalBytes : new Uint8Array(finalBytes); + editor.destroy(); + + const [docx1, media1, mf1, fonts1] = await Editor.loadXmlData(finalBuf, true); + const { editor: reloaded } = initTestEditor({ + content: docx1, + media: media1, + mediaFiles: mf1, + fonts: fonts1, + useImmediateSetTimeout: false, + isHeadless: true, + user: { name: 'Test', email: 'test@example.com' }, + }); + + const finalList = reloaded.doc.customXml.parts.list(); + expect(finalList.items.length).toBe(1); + expect(finalList.items[0]!.id).toBe(created.id); + const finalGet = reloaded.doc.customXml.parts.get({ target: { id: created.id } }); + expect(finalGet!.content).toContain(' { const editor = await createEditorWithEmptyPackage(); const created = editor.doc.customXml.parts.create({ diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts index b67e08ae97..540a70f7e2 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts @@ -134,50 +134,62 @@ function targetNotFound(): WriteOutcome { return { ok: false, code: 'TARGET_NOT_FOUND', message: 'No custom XML part matched the supplied target.' }; } +/** + * Wraps a synchronous block that can throw on well-formedness / parsing. + * Lifecycle errors (REVISION_MISMATCH from checkRevision, PlanError) MUST + * NOT pass through this — that's why the catch is scoped to just the + * content-validation block, not the whole executeOutOfBandMutation call. + */ +function safeValidate(fn: () => T): WriteOutcome { + try { + return { ok: true, payload: fn() }; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + return { ok: false, code: 'INVALID_INPUT', message: msg }; + } +} + export function customXmlPartsCreateWrapper( editor: Editor, input: CustomXmlPartsCreateInput, options?: MutationOptions, ): CustomXmlPartsCreateResult { rejectTrackedMode('customXml.parts.create', options); - try { - const outcome = executeOutOfBandMutation< - WriteOutcome<{ id: string; partName: string; propsPartName: string }> - >( - editor, - (dryRun) => { - if (dryRun) { - // Read-only preview: validate well-formedness without writing. - try { - createCustomXmlPart({}, { content: input.content, schemaRefs: input.schemaRefs }); - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - return { changed: false, payload: { ok: false, code: 'INVALID_INPUT', message: msg } }; - } - return { - changed: false, - payload: { ok: true, payload: { id: '{DRY-RUN}', partName: '', propsPartName: '' } }, - }; - } - const created = createCustomXmlPart(getConvertedXml(editor), { - content: input.content, - schemaRefs: input.schemaRefs, - }); - return { changed: true, payload: { ok: true, payload: created } }; - }, - { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, - ); - if (!outcome.ok) return failure(outcome.code, outcome.message); - return { - success: true, - id: outcome.payload.id, - partName: outcome.payload.partName, - propsPartName: outcome.payload.propsPartName, - }; - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - return failure('INVALID_INPUT', `customXml.parts.create failed: ${msg}`); - } + const outcome = executeOutOfBandMutation< + WriteOutcome<{ id: string; partName: string; propsPartName: string }> + >( + editor, + (dryRun) => { + if (dryRun) { + // Read-only preview: validate well-formedness without writing. + const probe = safeValidate(() => + createCustomXmlPart({}, { content: input.content, schemaRefs: input.schemaRefs }), + ); + if (!probe.ok) return { changed: false, payload: probe }; + return { + changed: false, + payload: { ok: true, payload: { id: '{DRY-RUN}', partName: '', propsPartName: '' } }, + }; + } + const probe = safeValidate(() => + createCustomXmlPart( + getConvertedXml(editor), + { content: input.content, schemaRefs: input.schemaRefs }, + getConverter(editor), + ), + ); + if (!probe.ok) return { changed: false, payload: probe }; + return { changed: true, payload: { ok: true, payload: probe.payload } }; + }, + { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, + ); + if (!outcome.ok) return failure(outcome.code, outcome.message); + return { + success: true, + id: outcome.payload.id, + partName: outcome.payload.partName, + propsPartName: outcome.payload.propsPartName, + }; } export function customXmlPartsPatchWrapper( @@ -186,38 +198,37 @@ export function customXmlPartsPatchWrapper( options?: MutationOptions, ): CustomXmlPartsMutationResult { rejectTrackedMode('customXml.parts.patch', options); - try { - const outcome = executeOutOfBandMutation>( - editor, - (dryRun) => { - if (dryRun) { - const partName = resolveTargetPartName(getConvertedXml(editor), input.target); - if (!partName) return { changed: false, payload: targetNotFound() }; - if (input.content !== undefined) { - try { - createCustomXmlPart({}, { content: input.content, schemaRefs: undefined }); - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - return { changed: false, payload: { ok: false, code: 'INVALID_INPUT', message: msg } }; - } - } - return { changed: false, payload: { ok: true, payload: true } }; + const outcome = executeOutOfBandMutation>( + editor, + (dryRun) => { + if (dryRun) { + const partName = resolveTargetPartName(getConvertedXml(editor), input.target); + if (!partName) return { changed: false, payload: targetNotFound() }; + if (input.content !== undefined) { + const probe = safeValidate(() => + createCustomXmlPart({}, { content: input.content, schemaRefs: undefined }), + ); + if (!probe.ok) return { changed: false, payload: probe }; } - const patched = patchCustomXmlPart(getConvertedXml(editor), input.target, { + return { changed: false, payload: { ok: true, payload: true } }; + } + // Resolve first so a missing target doesn't get reported as INVALID_INPUT. + const partName = resolveTargetPartName(getConvertedXml(editor), input.target); + if (!partName) return { changed: false, payload: targetNotFound() }; + const probe = safeValidate(() => + patchCustomXmlPart(getConvertedXml(editor), input.target, { content: input.content, schemaRefs: input.schemaRefs, - }); - if (!patched) return { changed: false, payload: targetNotFound() }; - return { changed: true, payload: { ok: true, payload: true } }; - }, - { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, - ); - if (!outcome.ok) return failure(outcome.code, outcome.message); - return { success: true, target: input.target }; - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - return failure('INVALID_INPUT', `customXml.parts.patch failed: ${msg}`); - } + }), + ); + if (!probe.ok) return { changed: false, payload: probe }; + if (!probe.payload) return { changed: false, payload: targetNotFound() }; + return { changed: true, payload: { ok: true, payload: true } }; + }, + { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, + ); + if (!outcome.ok) return failure(outcome.code, outcome.message); + return { success: true, target: input.target }; } export function customXmlPartsRemoveWrapper( From 7cb928e57ff84df1268337109051c63f85d4fc1a Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 12 May 2026 11:03:38 -0300 Subject: [PATCH 06/14] wip(document-api): OPC rels resolution, bibliography invalidation, content-types pruning (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings — each verified by a failing test first, then fixed: #1 findPropsPartFor used an ad-hoc regex that only handled bare names and `/` prefixes. Valid OPC Target forms like `./itemPropsN.xml` and `../customXml/itemPropsN.xml` (per RFC 3986 §5.2.4 and ECMA-376 §9.1.4) silently fell through to the index-name fallback or missed the props entirely. Route through resolveOpcTargetPath with baseDir='customXml' (the source part's directory). Two new tests assert resolution for `./` and `../customXml/` Target forms. #2 removeCustomXmlPart on the bibliography part left converter.bibliographyPart populated. On the next export, syncBibliographyPartToPackage(convertedXml, bibliographyPart) wrote the cached sources back into convertedXml — resurrecting the supposedly-removed part in the in-memory state (the tombstone still nulled the exported zip entry, but the editor's own view of the document silently re-grew the part). patchCustomXmlPart on the bibliography part had the worse variant: cached sources overwrote the customer's new content. Both now call invalidateConverterCachesForPath, which clears converter.bibliographyPart when its partPath matches the part we touched. New test exercises remove + exportDocx and asserts the convertedXml entry stays gone. #3 DocxZipper.updateContentTypes only pruned stale Override entries for comment parts. After customXml.parts.remove, the original DOCX's `` survived in the exported [Content_Types].xml, pointing at a non-existent part. The operation's cleanup contract claimed otherwise. Extended the stale-override pruning to also cover customXml/itemPropsN.xml paths absent from the final zip (i.e. tombstoned via updatedDocs[path] = null). Also clears the now-stale top-of-file docblock on the integration test that claimed write-side was `CAPABILITY_UNAVAILABLE`-stubbed; the file actually contains a full write-side suite including round-trip and bibliography-cache tests. Verified: - 22/22 customXml integration tests - 3404/3404 super-editor document-api-adapters - 1428/1428 document-api package --- .../src/editors/v1/core/DocxZipper.js | 15 ++ .../core/super-converter/custom-xml-parts.js | 45 +++++- .../custom-xml-wrappers.integration.test.ts | 145 +++++++++++++++++- .../plan-engine/custom-xml-wrappers.ts | 10 +- 4 files changed, 197 insertions(+), 18 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/DocxZipper.js b/packages/super-editor/src/editors/v1/core/DocxZipper.js index 7a5f752707..712c5beb2b 100644 --- a/packages/super-editor/src/editors/v1/core/DocxZipper.js +++ b/packages/super-editor/src/editors/v1/core/DocxZipper.js @@ -349,6 +349,21 @@ class DocxZipper { return !hasFile(filename); }); + // Also prune Custom XML props Override entries whose part was deleted + // (tombstoned via updatedDocs[path] = null, e.g. by `customXml.parts.remove`). + // Without this, [Content_Types].xml retains an Override pointing at a + // missing part, which is spec-malformed. + if (types?.elements?.length) { + for (const el of types.elements) { + if (el?.name !== 'Override') continue; + const partName = el?.attributes?.PartName; + if (typeof partName !== 'string') continue; + if (!/^\/customXml\/itemProps\d+\.xml$/i.test(partName)) continue; + const filename = partName.slice(1); // strip leading / + if (!hasFile(filename)) staleOverridePartNames.push(partName); + } + } + const beginningString = ''; let updatedContentTypesXml = contentTypesXml.replace(beginningString, `${beginningString}${typesString}`); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index b66d1b15a0..9ffb18e2ac 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -113,12 +113,12 @@ export function findPropsPartFor(convertedXml, partName) { if (rel?.attributes?.Type !== CUSTOM_XML_PROPS_RELATIONSHIP_TYPE) continue; const target = rel?.attributes?.Target; if (typeof target !== 'string' || target.length === 0) continue; - // Targets in customXml/_rels/itemN.xml.rels are relative to the - // rels file's base, i.e. `customXml/`. So `itemPropsN.xml` → - // `customXml/itemPropsN.xml`. Absolute or otherwise-prefixed - // targets are accepted as-is when they already point at a key. - const candidate = target.includes('/') ? target.replace(/^\.?\//, '') : `customXml/${target}`; - if (convertedXml[candidate]) return candidate; + // OPC resolution: Target is relative to the source part's directory + // (`customXml/` for a rels file at `customXml/_rels/itemN.xml.rels`). + // resolveOpcTargetPath handles bare names, `./`, `../`, and absolute + // forms per RFC 3986 §5.2.4. + const candidate = resolveOpcTargetPath(target, 'customXml'); + if (candidate && convertedXml[candidate]) return candidate; } } @@ -465,12 +465,17 @@ export function createCustomXmlPart(convertedXml, { content, schemaRefs }, conve * Replaces the content and/or schemaRefs of an existing part. Preserves * the existing itemID and package part names. * + * When `converter` is provided and the patched part is the cached + * bibliography part, the bibliographyPart cache is invalidated so the + * exporter's `syncBibliographyPartToPackage` doesn't overwrite the + * patched content with stale sources. + * * Returns `{ partName }` of the part that was patched, or `null` when * the target couldn't be resolved. * * @throws {Error} when content is provided but not well-formed. */ -export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs }) { +export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs }, converter) { const partName = resolveTargetPartName(convertedXml, target); if (!partName) return null; @@ -501,6 +506,10 @@ export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs } convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(itemId, schemaRefs), existingDecl); } + // If we just patched the bibliography part, invalidate the cache so + // the exporter doesn't overwrite our content from converter.bibliographyPart. + if (converter) invalidateConverterCachesForPath(converter, partName); + return { partName }; } @@ -554,7 +563,29 @@ export function removeCustomXmlPart(convertedXml, target, converter) { converter.removedCustomXmlPaths = new Set(); } for (const path of removedPaths) converter.removedCustomXmlPaths.add(path); + + // Invalidate the bibliographyPart cache if its part was removed. + // Without this, syncBibliographyPartToPackage on the next export + // would resurrect the deleted part from the stale cache. The + // `customXml.parts.remove` contract promises full cleanup. + invalidateConverterCachesForPath(converter, partName); } return true; } + +function invalidateConverterCachesForPath(converter, partName) { + if (!converter || typeof partName !== 'string') return; + const biblio = converter.bibliographyPart; + if (biblio && biblio.partPath === partName) { + converter.bibliographyPart = { + sources: [], + partPath: null, + itemPropsPath: null, + itemRelsPath: null, + selectedStyle: null, + styleName: null, + version: null, + }; + } +} diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts index 5811fc4748..ec9aca28a7 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -1,15 +1,21 @@ /* @vitest-environment jsdom */ /** - * customXml.parts.* read-side smoke tests against a real editor. + * customXml.parts.* integration tests against a real editor. * - * - Empty document: list returns no parts; get returns null. - * - Manually injecting a custom XML part into the converter package: - * list discovers it, get returns its content, filters work. + * Read side: + * - Empty document: list returns no parts; get returns null. + * - Manually injected custom XML parts: list discovers them, get + * returns content, filters work. + * - Foreign producer cases: partName-only targeting, OPC rels-based + * props pairing (including `./` and `../customXml/` Target forms), + * and non-customXml partName targets are rejected. * - * Write side (`create` / `patch` / `remove`) is implemented behind a - * `CAPABILITY_UNAVAILABLE` stub for now; tests exist only for the - * lookup-shaped failures, not for actual write behavior. + * Write side: + * - create / patch / remove round-trip through export and reimport. + * - Tombstone semantics for parts that originated in the imported zip. + * - Remove → create index recycling. + * - Bibliography part cache invalidation. */ import { describe, expect, it } from 'vitest'; @@ -193,6 +199,74 @@ describe('customXml.parts read-side (integration)', () => { editor.destroy(); }); + it('resolves rels Target with ./ prefix (valid OPC relative path)', async () => { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted['customXml/itemPropsFOREIGN.xml'] = makePropsDoc(ITEM_ID, [NAMESPACE]); + converted['customXml/_rels/item1.xml.rels'] = { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8' } }, + elements: [ + { + type: 'element', + name: 'Relationships', + attributes: { xmlns: 'http://schemas.openxmlformats.org/package/2006/relationships' }, + elements: [ + { + type: 'element', + name: 'Relationship', + attributes: { + Id: 'rId1', + Type: 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXmlProps', + // VALID OPC: "./itemPropsFOREIGN.xml" is sibling relative. + Target: './itemPropsFOREIGN.xml', + }, + }, + ], + }, + ], + }; + const list = editor.doc.customXml.parts.list(); + expect(list.items.length).toBe(1); + expect(list.items[0]!.propsPartName).toBe('customXml/itemPropsFOREIGN.xml'); + editor.destroy(); + }); + + it('resolves rels Target with ../customXml/ prefix (valid OPC relative path)', async () => { + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + converted['customXml/itemPropsFOREIGN.xml'] = makePropsDoc(ITEM_ID, [NAMESPACE]); + converted['customXml/_rels/item1.xml.rels'] = { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8' } }, + elements: [ + { + type: 'element', + name: 'Relationships', + attributes: { xmlns: 'http://schemas.openxmlformats.org/package/2006/relationships' }, + elements: [ + { + type: 'element', + name: 'Relationship', + attributes: { + Id: 'rId1', + Type: 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/customXmlProps', + // VALID OPC: "../customXml/itemPropsFOREIGN.xml" is also acceptable. + Target: '../customXml/itemPropsFOREIGN.xml', + }, + }, + ], + }, + ], + }; + const list = editor.doc.customXml.parts.list(); + expect(list.items.length).toBe(1); + expect(list.items[0]!.propsPartName).toBe('customXml/itemPropsFOREIGN.xml'); + editor.destroy(); + }); + it('pairs storage and props parts via the item rels file, not by filename', async () => { // Foreign doc shape: item1.xml is linked to itemPropsFOREIGN.xml via // customXml/_rels/item1.xml.rels. The index-match heuristic would @@ -461,6 +535,63 @@ describe('customXml.parts write-side', () => { reloaded.destroy(); }); + it('removing the bibliography part does not resurrect it via syncBibliographyPartToPackage on export', async () => { + // Seed: simulate a doc with a bibliography custom XML part already + // loaded. The converter's bibliographyPart cache will hold sources. + const editor = await createEditorWithEmptyPackage(); + const converter = (editor as unknown as { converter: { convertedXml: Record; bibliographyPart?: unknown } }) + .converter; + // Fake a populated bibliographyPart cache pointing at customXml/item1.xml. + converter.bibliographyPart = { + sources: [ + { + tag: 'src1', + type: 'book', + fields: { title: 'A Book' }, + }, + ], + partPath: 'customXml/item1.xml', + itemPropsPath: 'customXml/itemProps1.xml', + itemRelsPath: 'customXml/_rels/item1.xml.rels', + selectedStyle: '/APA.XSL', + styleName: 'APA', + version: '6', + }; + // Pretend the part also exists in convertedXml (it was loaded from a real doc). + converter.convertedXml['customXml/item1.xml'] = { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8' } }, + elements: [ + { + type: 'element', + name: 'b:Sources', + attributes: { + xmlns: 'http://schemas.openxmlformats.org/officeDocument/2006/bibliography', + 'xmlns:b': 'http://schemas.openxmlformats.org/officeDocument/2006/bibliography', + }, + elements: [], + }, + ], + }; + + // Locate the part by partName and remove it. + const removed = editor.doc.customXml.parts.remove({ + target: { partName: 'customXml/item1.xml' }, + }); + expect(removed.success).toBe(true); + + // Without a fix, syncBibliographyPartToPackage will re-create the part + // when exportDocx runs, because bibliographyPart.sources still has the + // cached entries. + await editor.exportDocx(); + + // After export, convertedXml should NOT have the part again (or, if + // it does, that's the staleness bug). + const partResurrectedInConvertedXml = converter.convertedXml['customXml/item1.xml'] !== undefined; + expect(partResurrectedInConvertedXml, 'syncBibliographyPartToPackage re-added the removed part to convertedXml').toBe(false); + + editor.destroy(); + }); + it('round-trip: create → export → reimport preserves id, content, schemaRefs', async () => { const editor = await createEditorWithEmptyPackage(); const created = editor.doc.customXml.parts.create({ diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts index 540a70f7e2..537df1fdd6 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts @@ -216,10 +216,12 @@ export function customXmlPartsPatchWrapper( const partName = resolveTargetPartName(getConvertedXml(editor), input.target); if (!partName) return { changed: false, payload: targetNotFound() }; const probe = safeValidate(() => - patchCustomXmlPart(getConvertedXml(editor), input.target, { - content: input.content, - schemaRefs: input.schemaRefs, - }), + patchCustomXmlPart( + getConvertedXml(editor), + input.target, + { content: input.content, schemaRefs: input.schemaRefs }, + getConverter(editor), + ), ); if (!probe.ok) return { changed: false, payload: probe }; if (!probe.payload) return { changed: false, payload: targetNotFound() }; From b47d3c9411adcf1b4dca3017c1b0f39366edf1c5 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:34:39 -0300 Subject: [PATCH 07/14] wip(document-api): foreign-named props Override pruning + schema minLength (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from another review round; verified two with failing tests, then fixed: #1 (real bug, verified) — Content_Types Override pruning regex was too tight: `/^\/customXml\/itemProps\d+\.xml$/i` only matched numeric-named props parts. But `findPropsPartFor` correctly resolves foreign-named props parts (e.g. `customXml/itemPropsFOREIGN.xml`) via the OPC rels file, so removing one would tombstone the file but leave a stale Override pointing at it. Fix: identify props Overrides by ContentType (`application/vnd.openxmlformats-officedocument.customXmlProperties+xml`) instead of filename. New DocxZipper test confirms the foreign-named Override is pruned on tombstone. #2 (contract gap, verified) — `customXmlPartTargetSchema` allowed empty `id` and `partName` strings even though the runtime validator requires non-empty. Added `minLength: 1` to both target-shape branches. Also added `minLength: 1` to `content` and `schemaRefs.items` on create/patch. Pulling minLength into the contract surfaced a secondary issue: the conformance test's custom JSON Schema validator didn't support `minLength` / `maxLength`. Added support (lines 84-91 had been rejecting unsupported keywords entirely, which made my oneOf branches both fail). The validator now matches the keywords its schemas actually use. #3 (scope question, not changed) — Discovery of foreign-named Storage Parts (filenames other than `customXml/itemN.xml`). Considered: walking word/_rels/document.xml.rels for `customXml`-type rels would cover this. But `isCustomXmlStoragePartName` and `findPropsPartFor` both key off the numeric-index convention, so broadening discovery without also broadening those would leave list and get/patch/remove disagreeing about what's a valid part. Documented as an explicit v1 scope limitation via AIDEV-NOTE on `listCustomXmlStoragePartNames`. No real-world producer (Word, Google Docs, LibreOffice, pandoc) deviates from the convention, so v1 ships Word-style only. Verified: - 3429/3429 super-editor document-api-adapters + DocxZipper - 1428/1428 document-api package - New DocxZipper test exercises the foreign-named props Override pruning end-to-end --- packages/document-api/src/contract/schemas.ts | 7 ++-- .../src/editors/v1/core/DocxZipper.js | 9 +++-- .../src/editors/v1/core/DocxZipper.test.js | 36 +++++++++++++++++++ .../core/super-converter/custom-xml-parts.js | 16 ++++++--- .../__conformance__/schema-validator.ts | 13 +++++++ 5 files changed, 73 insertions(+), 8 deletions(-) diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index efbf4331f3..b0100625ae 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -2586,8 +2586,11 @@ const bookmarkMutation = refMutationSchemas({ bookmark: bookmarkAddressSchema }, // --- Custom XML part schemas --- const customXmlPartTargetSchema: JsonSchema = { oneOf: [ - objectSchema({ id: { type: 'string' } }, ['id']), - objectSchema({ partName: { type: 'string' } }, ['partName']), + // Empty strings are runtime-rejected (target validator requires + // non-zero length); reflect that in the contract so generated SDKs + // and tool callers see the same constraint. + objectSchema({ id: { type: 'string', minLength: 1 } }, ['id']), + objectSchema({ partName: { type: 'string', minLength: 1 } }, ['partName']), ], }; diff --git a/packages/super-editor/src/editors/v1/core/DocxZipper.js b/packages/super-editor/src/editors/v1/core/DocxZipper.js index 712c5beb2b..f8099839d8 100644 --- a/packages/super-editor/src/editors/v1/core/DocxZipper.js +++ b/packages/super-editor/src/editors/v1/core/DocxZipper.js @@ -353,12 +353,17 @@ class DocxZipper { // (tombstoned via updatedDocs[path] = null, e.g. by `customXml.parts.remove`). // Without this, [Content_Types].xml retains an Override pointing at a // missing part, which is spec-malformed. + // + // Identify by ContentType, not by filename — OOXML does not require + // the props part to be named itemPropsN.xml. Foreign producers can + // use arbitrary names, linked via customXml/_rels/itemN.xml.rels. + const CUSTOM_XML_PROPS_CT = 'application/vnd.openxmlformats-officedocument.customXmlProperties+xml'; if (types?.elements?.length) { for (const el of types.elements) { if (el?.name !== 'Override') continue; + if (el?.attributes?.ContentType !== CUSTOM_XML_PROPS_CT) continue; const partName = el?.attributes?.PartName; - if (typeof partName !== 'string') continue; - if (!/^\/customXml\/itemProps\d+\.xml$/i.test(partName)) continue; + if (typeof partName !== 'string' || !partName.startsWith('/')) continue; const filename = partName.slice(1); // strip leading / if (!hasFile(filename)) staleOverridePartNames.push(partName); } diff --git a/packages/super-editor/src/editors/v1/core/DocxZipper.test.js b/packages/super-editor/src/editors/v1/core/DocxZipper.test.js index 902994963c..6782755c95 100644 --- a/packages/super-editor/src/editors/v1/core/DocxZipper.test.js +++ b/packages/super-editor/src/editors/v1/core/DocxZipper.test.js @@ -286,6 +286,42 @@ describe('DocxZipper - updateZip compression', () => { }); describe('DocxZipper - updateContentTypes', () => { + it('prunes a tombstoned customXml props Override with a foreign filename', async () => { + // Foreign producer convention: the Properties Part is named + // something other than itemPropsN.xml, but linked via the OPC rels + // file. When the part is removed (tombstoned via updatedDocs[path] + // = null), the [Content_Types].xml Override must be pruned too — + // otherwise the exported package has an Override pointing at a + // missing file, which is spec-malformed. + const zipper = new DocxZipper(); + const zip = new JSZip(); + + const contentTypes = ` + + + + + + `; + zip.file('[Content_Types].xml', contentTypes); + zip.file( + 'word/document.xml', + '', + ); + + const updatedDocs = { + // Tombstone the foreign-named props part. + 'customXml/itemPropsFOREIGN.xml': null, + }; + + await zipper.updateContentTypes(zip, {}, false, updatedDocs); + + const updatedContentTypes = await zip.file('[Content_Types].xml').async('string'); + expect(updatedContentTypes, 'foreign-named props Override should be pruned').not.toContain( + '/customXml/itemPropsFOREIGN.xml', + ); + }); + it('adds header/footer overrides for newly added parts', async () => { const zipper = new DocxZipper(); const zip = new JSZip(); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index 9ffb18e2ac..83231910a6 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -73,11 +73,19 @@ export function isCustomXmlStoragePartName(partName) { // --------------------------------------------------------------------------- /** - * Enumerates every custom XML Storage Part in the package by scanning - * convertedXml keys (not the relationships file, because foreign producers - * sometimes leave orphan parts that aren't referenced from word/document.xml). + * Enumerates every Custom XML Data Storage Part in the package. * - * Returns part names sorted by their numeric index. Pair-matching with + * AIDEV-NOTE: v1 scope is Word-style filenames (`customXml/itemN.xml`). + * ECMA-376 §15.2.5 allows arbitrary filenames identified by relationship + * + content type, but every real producer (Word, Google Docs, LibreOffice, + * pandoc) uses the Word convention. Supporting truly foreign-named + * storage parts is intentionally deferred — it requires broadening the + * partName safety filter (currently `isCustomXmlStoragePartName`) and + * the rels-path computation in `findPropsPartFor` to be path-agnostic. + * Foreign-named *Properties Parts* paired via rels ARE supported (see + * `findPropsPartFor`); only the Storage Part filename is constrained. + * + * Returns part names sorted by numeric index. Pair-matching with * Properties Parts is left to the caller. */ export function listCustomXmlStoragePartNames(convertedXml) { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/schema-validator.ts b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/schema-validator.ts index 5a65558888..ed3f9e48aa 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/schema-validator.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/schema-validator.ts @@ -8,6 +8,8 @@ type JsonSchema = { enum?: unknown[]; oneOf?: JsonSchema[]; anyOf?: JsonSchema[]; + minLength?: number; + maxLength?: number; $ref?: string; $defs?: Record; }; @@ -22,6 +24,8 @@ const SUPPORTED_SCHEMA_KEYWORDS = new Set([ 'enum', 'oneOf', 'anyOf', + 'minLength', + 'maxLength', '$ref', '$defs', 'description', @@ -138,6 +142,15 @@ function validateInternal( } } + if (typeof value === 'string') { + if (schema.minLength !== undefined && value.length < schema.minLength) { + errors.push(`${path}: expected minLength ${schema.minLength}, got ${value.length}`); + } + if (schema.maxLength !== undefined && value.length > schema.maxLength) { + errors.push(`${path}: expected maxLength ${schema.maxLength}, got ${value.length}`); + } + } + const types = Array.isArray(schema.type) ? schema.type : schema.type ? [schema.type] : []; if (types.includes('array') && schema.items && Array.isArray(value)) { value.forEach((item, index) => { From ced0fe30e69550af51c7e4d479cd077d9a8b62bd Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:38:24 -0300 Subject: [PATCH 08/14] wip(document-api): preserve schemaRefs omitted vs empty distinction (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ECMA-376 §22.5.2.3 distinguishes three schemaRefs states: - omitted → app may infer schemas - present empty → explicit "no schemas should be used" - with children → these schemas validate the part The previous write side conflated the first two: it always emitted regardless of whether the caller passed undefined or []. This made downstream consumers see "no schemas" when the customer intent was actually "app picks". Fix: - buildItemPropsRoot now omits the element when schemaRefs is undefined, and emits it (empty or with children) when an array is passed explicitly. - createCustomXmlPart no longer coerces undefined to [] before calling buildItemPropsRoot. - patchCustomXmlPart already passed through verbatim — no change needed. Read side keeps returning schemaRefs: [] for both the omitted and the present-empty cases. The distinction is lost in the public summary type (schemaRefs: string[]), and recovering it would require a type contract change — deferred for v1. Two new integration tests assert: 1. create({ content }) without schemaRefs produces a Properties Part with NO element. 2. create({ content, schemaRefs: [] }) produces a Properties Part WITH an empty element. Verified: - 3431/3431 super-editor document-api-adapters - 1428/1428 document-api package --- .../core/super-converter/custom-xml-parts.js | 57 ++++++++++++------- .../custom-xml-wrappers.integration.test.ts | 38 +++++++++++++ 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index 83231910a6..3cc9181765 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -350,26 +350,38 @@ function buildDocumentRelTarget(partName) { } function buildItemPropsRoot(itemId, schemaRefs) { - const schemaRefElements = (schemaRefs ?? []).map((uri) => ({ - type: 'element', - name: 'ds:schemaRef', - attributes: { 'ds:uri': uri }, - })); - return { - type: 'element', - name: 'ds:datastoreItem', - attributes: { - 'ds:itemID': itemId, - 'xmlns:ds': CUSTOM_XML_DATASTORE_NAMESPACE, - }, - elements: [ - { - type: 'element', - name: 'ds:schemaRefs', - elements: schemaRefElements, + // ECMA-376 §22.5.2.3 distinguishes three cases: + // - `` omitted → app may infer schemas + // - `` present empty → explicit "no schemas" + // - `` with children → these schemas validate the part + // + // We map `schemaRefs === undefined` → omit (caller didn't specify), + // `schemaRefs === []` → present-empty (caller explicitly cleared), + // anything else → present with children. + const elements = [ + { + type: 'element', + name: 'ds:datastoreItem', + attributes: { + 'ds:itemID': itemId, + 'xmlns:ds': CUSTOM_XML_DATASTORE_NAMESPACE, }, - ], - }; + elements: schemaRefs === undefined + ? [] + : [ + { + type: 'element', + name: 'ds:schemaRefs', + elements: schemaRefs.map((uri) => ({ + type: 'element', + name: 'ds:schemaRef', + attributes: { 'ds:uri': uri }, + })), + }, + ], + }, + ]; + return elements[0]; } function buildItemRelsRoot(propsPartFileName) { @@ -439,8 +451,11 @@ export function createCustomXmlPart(convertedXml, { content, schemaRefs }, conve // Storage Part — wrap the customer's content in a fresh document envelope. convertedXml[partName] = createXmlDocument(root, declaration); - // Properties Part — datastoreItem with itemID + optional schemaRefs. - convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(itemId, schemaRefs ?? [])); + // Properties Part — datastoreItem with itemID. `schemaRefs` is passed + // through verbatim so `undefined` (omit element, app may infer) and + // `[]` (present-empty, explicit "no schemas") stay distinct per + // ECMA-376 §22.5.2.3. + convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(itemId, schemaRefs)); // Item rels — link Storage Part → Properties Part. convertedXml[itemRelsPath] = createXmlDocument(buildItemRelsRoot(`itemProps${index}.xml`)); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts index ec9aca28a7..e1cb30a1d9 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -535,6 +535,44 @@ describe('customXml.parts write-side', () => { reloaded.destroy(); }); + it('omits when create is called without schemaRefs (ECMA-376 §22.5.2.3)', async () => { + // Per spec: schemaRefs omitted = app may infer schemas; schemaRefs + // present + empty = explicit "no schemas". These are different. + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ content: '' }); + if (!created.success) return; + + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + const propsDoc = converted[created.propsPartName] as + | { elements?: Array<{ name: string; elements?: Array<{ name: string }> }> } + | undefined; + const root = propsDoc?.elements?.[0]; + const schemaRefsEl = (root?.elements ?? []).find((el) => el?.name === 'ds:schemaRefs'); + expect(schemaRefsEl, 'omitted schemaRefs should not produce a element').toBeUndefined(); + editor.destroy(); + }); + + it('emits an empty when create is called with schemaRefs: []', async () => { + const editor = await createEditorWithEmptyPackage(); + const created = editor.doc.customXml.parts.create({ + content: '', + schemaRefs: [], + }); + if (!created.success) return; + + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + const propsDoc = converted[created.propsPartName] as + | { elements?: Array<{ name: string; elements?: Array<{ name: string }> }> } + | undefined; + const root = propsDoc?.elements?.[0]; + const schemaRefsEl = (root?.elements ?? []).find((el) => el?.name === 'ds:schemaRefs'); + expect(schemaRefsEl, 'empty array should produce an explicit element').toBeDefined(); + expect(schemaRefsEl?.elements ?? []).toEqual([]); + editor.destroy(); + }); + it('removing the bibliography part does not resurrect it via syncBibliographyPartToPackage on export', async () => { // Seed: simulate a doc with a bibliography custom XML part already // loaded. The converter's bibliographyPart cache will hold sources. From 0ebba648d89f47afa9fdfd16ae8bfaab982fcbc6 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:41:36 -0300 Subject: [PATCH 09/14] wip(document-api): tighten v1 scope statements (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewer follow-ups on commit b47d3c941: 1. AIDEV note framing softened. "Every real producer" was a stronger claim than I could verify. Reframed to "the Word-compatible producers we target use this convention" — accurate and doesn't pretend to have audited every OOXML producer. 2. v1 scope now surfaced in the public type contract, not just internal notes. Generated docs and consumer JSDoc tooltips now show the Word-style filename constraint: - CustomXmlPartTarget.partName: scope note added - CustomXmlPartSummary.partName: scope note added Foreign-named Properties Parts still work (paired via rels); only Storage Part filenames are constrained. Not in this commit: - The reviewer flagged the conformance schema validator's early-return after oneOf/anyOf — keywords like required and additionalProperties sitting at the same level as anyOf are not checked. Confirmed via source inspection. Affects the patch input schema specifically. Not a production API bug (runtime validators in customXml.ts cover these constraints); just a slight weakening of conformance signal. Worth a separate ticket on the test harness, not this PR. --- .../document-api/src/customXml/customXml.types.ts | 11 ++++++++++- .../v1/core/super-converter/custom-xml-parts.js | 11 ++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/document-api/src/customXml/customXml.types.ts b/packages/document-api/src/customXml/customXml.types.ts index feacd29f30..5120310bee 100644 --- a/packages/document-api/src/customXml/customXml.types.ts +++ b/packages/document-api/src/customXml/customXml.types.ts @@ -25,6 +25,12 @@ export type CustomXmlPartId = string; * variant exists for Storage Parts that have no Properties Part — those * have no itemID and can only be addressed by their file path inside * the OOXML package. + * + * Scope: `partName` accepts Word-style Storage Part paths only — + * `customXml/itemN.xml` for integer `N`. Foreign-named Storage Parts + * (which ECMA-376 §15.2.5 permits) are not in v1; see the implementation + * note on the converter's `listCustomXmlStoragePartNames`. Foreign-named + * *Properties Parts* paired via rels are fully supported on the read side. */ export type CustomXmlPartTarget = { id: CustomXmlPartId } | { partName: string }; @@ -98,7 +104,10 @@ export interface CustomXmlPartsRemoveInput { export interface CustomXmlPartSummary { /** itemID GUID; absent when no Properties Part exists. */ id?: CustomXmlPartId; - /** Package-relative path of the Storage Part, e.g. `"customXml/item1.xml"`. */ + /** + * Package-relative path of the Storage Part, e.g. `"customXml/item1.xml"`. + * v1 scope: Word-style `customXml/itemN.xml` only. + */ partName: string; /** Package-relative path of the Properties Part, when present. */ propsPartName?: string; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index 3cc9181765..0f62fb5ce9 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -77,11 +77,12 @@ export function isCustomXmlStoragePartName(partName) { * * AIDEV-NOTE: v1 scope is Word-style filenames (`customXml/itemN.xml`). * ECMA-376 §15.2.5 allows arbitrary filenames identified by relationship - * + content type, but every real producer (Word, Google Docs, LibreOffice, - * pandoc) uses the Word convention. Supporting truly foreign-named - * storage parts is intentionally deferred — it requires broadening the - * partName safety filter (currently `isCustomXmlStoragePartName`) and - * the rels-path computation in `findPropsPartFor` to be path-agnostic. + * + content type. The Word-compatible producers we target use this + * convention; truly foreign-named Storage Parts are out of scope for v1. + * Lifting this requires broadening the partName safety filter + * (currently `isCustomXmlStoragePartName`) and the rels-path computation + * in `findPropsPartFor` to be path-agnostic. + * * Foreign-named *Properties Parts* paired via rels ARE supported (see * `findPropsPartFor`); only the Storage Part filename is constrained. * From cf3b0e8b25ea521ec87e8d6bb236d96b8d476f9c Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:42:22 -0300 Subject: [PATCH 10/14] wip(document-api): update CustomXmlPartsCreateInput JSDoc to match new schemaRefs semantics (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSDoc on CustomXmlPartsCreateInput.schemaRefs and CustomXmlPartsPatchInput.schemaRefs was stale after commit ced0fe30e swapped the writer to preserve the ECMA-376 §22.5.2.3 omitted-vs-empty distinction. Old text claimed 'when omitted or empty, [an] empty [is] still emitted'. That was true before ced0fe30e; now omitted produces no element. Updated both JSDoc blocks to explain the three distinct spec states (omitted, empty, populated) so generated docs and IDE tooltips match runtime behavior. --- .../src/customXml/customXml.types.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/document-api/src/customXml/customXml.types.ts b/packages/document-api/src/customXml/customXml.types.ts index 5120310bee..cbe29763f9 100644 --- a/packages/document-api/src/customXml/customXml.types.ts +++ b/packages/document-api/src/customXml/customXml.types.ts @@ -70,9 +70,20 @@ export interface CustomXmlPartsCreateInput { content: string; /** * Optional list of XML schema target namespaces to declare in the - * Properties Part (``). When omitted or empty, the - * Properties Part is still emitted with a fresh itemID and an empty - * `` so `id` is always discoverable on readback. + * Properties Part (``). + * + * Per ECMA-376 §22.5.2.3, three states are distinct: + * - omitted (`undefined`) → no `` element emitted; + * consumers may infer the schema from the + * content's namespace. + * - empty array (`[]`) → `` is emitted explicitly, + * meaning "no schemas should be used to + * validate this part." + * - populated array → `` with one `` + * per URI. + * + * The Properties Part itself is always emitted (with a fresh itemID), + * so `id` is discoverable on readback regardless of `schemaRefs`. */ schemaRefs?: string[]; } @@ -83,7 +94,9 @@ export interface CustomXmlPartsPatchInput { content?: string; /** * Replace the Properties Part's `` set with this list. - * Pass `[]` to clear all schemaRefs. + * Pass `[]` to write an explicit empty `` (ECMA-376 + * §22.5.2.3 "no schemas should be used"). Omit the field entirely to + * leave the existing schemaRefs untouched. */ schemaRefs?: string[]; } From 9c3936c63f0fe373e3adef798e0d19f4bc02d2dc Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:47:15 -0300 Subject: [PATCH 11/14] wip(document-api): surface v1 partName scope in operation descriptions (SD-3105) operation-definitions.ts descriptions feed reference docs (Mintlify), LLM tool catalog descriptions, and CLI help text. The public type JSDoc on CustomXmlPartTarget already states the v1 partName scope, but consumers reading generated docs or tool descriptions wouldn't see it. Added the constraint to the three operation descriptions that accept a partName target: get, patch, remove. Not on list (returns whatever's discovered) or create (always emits Word-style filenames). --- .../src/contract/operation-definitions.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index c2f14f1034..eee02a720d 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -6279,7 +6279,9 @@ export const OPERATION_DEFINITIONS = { }, 'customXml.parts.get': { memberPath: 'customXml.parts.get', - description: 'Get a single Custom XML Data Storage Part by itemID or package part name, including its full content.', + description: + 'Get a single Custom XML Data Storage Part by itemID or package part name, including its full content. ' + + 'v1 partName targeting is limited to Word-style customXml/itemN.xml paths.', expectedResult: 'Returns a CustomXmlPartInfo with id, partName, namespaces, schemaRefs, and content; or null if not found.', requiresDocumentContext: true, metadata: readOperation({ @@ -6305,7 +6307,10 @@ export const OPERATION_DEFINITIONS = { }, 'customXml.parts.patch': { memberPath: 'customXml.parts.patch', - description: 'Replace the content and/or schemaRefs of an existing Custom XML Data Storage Part. At least one of content or schemaRefs is required.', + description: + 'Replace the content and/or schemaRefs of an existing Custom XML Data Storage Part. ' + + 'At least one of content or schemaRefs is required. ' + + 'v1 partName targeting is limited to Word-style customXml/itemN.xml paths.', expectedResult: 'Returns a CustomXmlPartsMutationResult indicating success with the resolved target or a failure.', requiresDocumentContext: true, metadata: mutationOperation({ @@ -6320,7 +6325,9 @@ export const OPERATION_DEFINITIONS = { }, 'customXml.parts.remove': { memberPath: 'customXml.parts.remove', - description: 'Remove a Custom XML Data Storage Part and clean up all linked package files (item, props, rels, content-types entry).', + description: + 'Remove a Custom XML Data Storage Part and clean up all linked package files (item, props, rels, content-types entry). ' + + 'v1 partName targeting is limited to Word-style customXml/itemN.xml paths.', expectedResult: 'Returns a CustomXmlPartsMutationResult indicating success or a failure.', requiresDocumentContext: true, metadata: mutationOperation({ From 049996d9f9984fb96b76be33e27b86ccf0a74046 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:52:20 -0300 Subject: [PATCH 12/14] fix(document-api): narrow custom XML write outcomes --- .../plan-engine/custom-xml-wrappers.ts | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts index 537df1fdd6..757347574b 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts @@ -59,10 +59,7 @@ function toSummary(record: ReturnType[number]): Custo return summary; } -export function customXmlPartsListWrapper( - editor: Editor, - query?: CustomXmlPartsListInput, -): CustomXmlPartsListResult { +export function customXmlPartsListWrapper(editor: Editor, query?: CustomXmlPartsListInput): CustomXmlPartsListResult { const revision = getRevision(editor); const all = listCustomXmlParts(getConvertedXml(editor)); @@ -97,10 +94,7 @@ export function customXmlPartsListWrapper( }); } -export function customXmlPartsGetWrapper( - editor: Editor, - input: CustomXmlPartsGetInput, -): CustomXmlPartInfo | null { +export function customXmlPartsGetWrapper(editor: Editor, input: CustomXmlPartsGetInput): CustomXmlPartInfo | null { const record = readCustomXmlPart(getConvertedXml(editor), input.target); if (!record) return null; // Normalize null fields to match CustomXmlPartInfo shape (optional, not null). @@ -128,7 +122,12 @@ function failure( return { success: false, failure: { code, message } }; } -type WriteOutcome = { ok: true; payload: T } | { ok: false; code: FailureCode; message: string }; +type WriteFailure = { ok: false; code: FailureCode; message: string }; +type WriteOutcome = { ok: true; payload: T } | WriteFailure; + +function isWriteFailure(outcome: WriteOutcome): outcome is WriteFailure { + return outcome.ok === false; +} function targetNotFound(): WriteOutcome { return { ok: false, code: 'TARGET_NOT_FOUND', message: 'No custom XML part matched the supplied target.' }; @@ -155,9 +154,7 @@ export function customXmlPartsCreateWrapper( options?: MutationOptions, ): CustomXmlPartsCreateResult { rejectTrackedMode('customXml.parts.create', options); - const outcome = executeOutOfBandMutation< - WriteOutcome<{ id: string; partName: string; propsPartName: string }> - >( + const outcome = executeOutOfBandMutation>( editor, (dryRun) => { if (dryRun) { @@ -165,7 +162,7 @@ export function customXmlPartsCreateWrapper( const probe = safeValidate(() => createCustomXmlPart({}, { content: input.content, schemaRefs: input.schemaRefs }), ); - if (!probe.ok) return { changed: false, payload: probe }; + if (isWriteFailure(probe)) return { changed: false, payload: probe }; return { changed: false, payload: { ok: true, payload: { id: '{DRY-RUN}', partName: '', propsPartName: '' } }, @@ -178,12 +175,12 @@ export function customXmlPartsCreateWrapper( getConverter(editor), ), ); - if (!probe.ok) return { changed: false, payload: probe }; + if (isWriteFailure(probe)) return { changed: false, payload: probe }; return { changed: true, payload: { ok: true, payload: probe.payload } }; }, { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, ); - if (!outcome.ok) return failure(outcome.code, outcome.message); + if (isWriteFailure(outcome)) return failure(outcome.code, outcome.message); return { success: true, id: outcome.payload.id, @@ -205,10 +202,8 @@ export function customXmlPartsPatchWrapper( const partName = resolveTargetPartName(getConvertedXml(editor), input.target); if (!partName) return { changed: false, payload: targetNotFound() }; if (input.content !== undefined) { - const probe = safeValidate(() => - createCustomXmlPart({}, { content: input.content, schemaRefs: undefined }), - ); - if (!probe.ok) return { changed: false, payload: probe }; + const probe = safeValidate(() => createCustomXmlPart({}, { content: input.content, schemaRefs: undefined })); + if (isWriteFailure(probe)) return { changed: false, payload: probe }; } return { changed: false, payload: { ok: true, payload: true } }; } @@ -223,13 +218,13 @@ export function customXmlPartsPatchWrapper( getConverter(editor), ), ); - if (!probe.ok) return { changed: false, payload: probe }; + if (isWriteFailure(probe)) return { changed: false, payload: probe }; if (!probe.payload) return { changed: false, payload: targetNotFound() }; return { changed: true, payload: { ok: true, payload: true } }; }, { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, ); - if (!outcome.ok) return failure(outcome.code, outcome.message); + if (isWriteFailure(outcome)) return failure(outcome.code, outcome.message); return { success: true, target: input.target }; } @@ -254,7 +249,7 @@ export function customXmlPartsRemoveWrapper( }, { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, ); - if (!outcome.ok) return failure(outcome.code, outcome.message); + if (isWriteFailure(outcome)) return failure(outcome.code, outcome.message); return { success: true, target: input.target }; } From ee06aa0d20ee5409c8516a349522e2dbc03f6a55 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:59:24 -0300 Subject: [PATCH 13/14] wip(document-api): surface patch-minted itemID, type removedCustomXmlPaths, test name fix (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from another review round: #1 patch on a foreign Storage Part minted a fresh itemID but didn't surface it Scenario: customer targets by partName because the imported part had no Properties Part; patches schemaRefs; patchCustomXmlPart creates a new Properties Part with a fresh GUID; wrapper returned { success, target: input.target } leaving the caller unable to address the part by id without re-listing. Fix: - patchCustomXmlPart now returns { partName, id? } where id is the resolved (existing or freshly minted) itemID. - CustomXmlPartsMutationSuccess gains an optional id?: CustomXmlPartId field with JSDoc explaining the patch-foreign-part case. - Wrapper passes id through to the success result. - Schema customXmlPartMutation gains an optional id field (minLength: 1). - New integration test: import a Storage Part with no props, patch schemaRefs, assert the result includes a new GUID and get({ id }) finds the part. #2 removedCustomXmlPaths accessed via as unknown as casts Two readers (Editor.ts, custom-xml-wrappers.ts) and one writer (custom-xml-parts.js) all coupled via casts. A rename would break tombstone emission silently. Fix: added removedCustomXmlPaths?: Set to SuperConverter.d.ts with JSDoc. Dropped the cast in Editor.ts. The local type alias in custom-xml-wrappers.ts is still convenient as structural typing (it duck-types the converter without importing the full class), so leaving it. #3 Test name at customXml.test.ts:206 said 'rejects' but body asserted .not.toThrow(). Renamed to 'accepts patch with empty schemaRefs alongside valid content'. Out-of-scope items the reviewer flagged but already on branch: - DocxZipper Content_Types Override pruning for tombstoned customXml props (fixed in b47d3c941) - Schema minLength on target id/partName (fixed in b47d3c941) - ./-prefix in item-rels resolver (fixed in 7cb928e57 via resolveOpcTargetPath) Word-fixture observation re: ds:schemaRefs auto-fill from root namespace is real but our v1 stance is deliberate (omit/[]/populated distinct per ECMA-376 §22.5.2.3, see ced0fe30e). Verified: - 3432/3432 super-editor document-api-adapters + DocxZipper - 1428/1428 document-api package --- packages/document-api/src/contract/schemas.ts | 3 ++ .../src/customXml/customXml.test.ts | 4 +-- .../src/customXml/customXml.types.ts | 11 +++++++ .../src/editors/v1/core/Editor.ts | 3 +- .../core/super-converter/SuperConverter.d.ts | 8 +++++ .../core/super-converter/custom-xml-parts.js | 31 ++++++++++++----- .../custom-xml-wrappers.integration.test.ts | 33 +++++++++++++++++++ .../plan-engine/custom-xml-wrappers.ts | 10 +++--- 8 files changed, 86 insertions(+), 17 deletions(-) diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index b0100625ae..49bc5294bb 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -2597,6 +2597,9 @@ const customXmlPartTargetSchema: JsonSchema = { const customXmlPartMutation = refMutationSchemas( { target: customXmlPartTargetSchema, + // Optional: surfaced when patch resolves or mints an itemID. See + // CustomXmlPartsMutationSuccess JSDoc for the patch-foreign-part case. + id: { type: 'string', minLength: 1 }, }, ['target'], ); diff --git a/packages/document-api/src/customXml/customXml.test.ts b/packages/document-api/src/customXml/customXml.test.ts index 59fe06a2fa..4106179f9c 100644 --- a/packages/document-api/src/customXml/customXml.test.ts +++ b/packages/document-api/src/customXml/customXml.test.ts @@ -203,8 +203,8 @@ describe('customXml.parts.patch validation', () => { expect(() => executeCustomXmlPartsPatch(adapter, { target })).toThrow(DocumentApiValidationError); }); - it('rejects patch with empty schemaRefs cleared but valid content', () => { - // Empty schemaRefs is allowed (means "clear them"), content also valid. + it('accepts patch with empty schemaRefs alongside valid content', () => { + // Empty schemaRefs is allowed (means "clear them"); content also valid. const adapter = makeAdapter(); expect(() => executeCustomXmlPartsPatch(adapter, { target, content: VALID_XML, schemaRefs: [] })).not.toThrow(); }); diff --git a/packages/document-api/src/customXml/customXml.types.ts b/packages/document-api/src/customXml/customXml.types.ts index cbe29763f9..0bbab35f8e 100644 --- a/packages/document-api/src/customXml/customXml.types.ts +++ b/packages/document-api/src/customXml/customXml.types.ts @@ -156,6 +156,17 @@ export interface CustomXmlPartsMutationSuccess { success: true; /** Identifier the operation acted on (mirrors the resolved input target). */ target: CustomXmlPartTarget; + /** + * The resolved itemID GUID of the affected part, when one exists. + * + * `patch` may need to mint a fresh GUID — e.g. when `schemaRefs` is + * patched onto a Storage Part that didn't previously have a Properties + * Part. In that case the caller targeted by `partName` but now has an + * id they can use for subsequent operations; this field surfaces it. + * + * For `remove`, the field is omitted because the part is gone. + */ + id?: CustomXmlPartId; } export type CustomXmlPartsMutationResult = CustomXmlPartsMutationSuccess | AdapterMutationFailure; diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index 533ac17ec4..6f93715242 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -3335,8 +3335,7 @@ export class Editor extends EventEmitter { // Document API but originated in the imported DOCX. Without this, // the exporter would copy the original zip entry through, and the // removed part would reappear on the next import. - const removedCustomXmlPaths = (this.converter as unknown as { removedCustomXmlPaths?: Set }) - .removedCustomXmlPaths; + const removedCustomXmlPaths = this.converter.removedCustomXmlPaths; if (removedCustomXmlPaths instanceof Set) { for (const path of removedCustomXmlPaths) { updatedDocs[path] = null; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts b/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts index 74ef1a21d8..b95a972049 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts @@ -3,5 +3,13 @@ export class SuperConverter { static getStoredSuperdocVersion(...args: any[]): any; static setStoredSuperdocVersion(...args: any[]): void; static extractDocumentGuid(...args: any[]): string | null; + /** + * Set of package paths tombstoned by `customXml.parts.remove`. The + * exporter emits `updatedDocs[path] = null` for each entry so original + * imported parts don't survive into the exported zip. Mutated by + * `removeCustomXmlPart` (writer) and `createCustomXmlPart` (clears + * entries on index recycle); read by `Editor.exportDocx`. + */ + removedCustomXmlPaths?: Set; [key: string]: any; } diff --git a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js index 0f62fb5ce9..45c461a887 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js @@ -487,15 +487,20 @@ export function createCustomXmlPart(convertedXml, { content, schemaRefs }, conve /** * Replaces the content and/or schemaRefs of an existing part. Preserves - * the existing itemID and package part names. + * the existing itemID when present; generates a new one when the patch + * forces creation of a Properties Part on a foreign storage part that + * didn't have one. * * When `converter` is provided and the patched part is the cached * bibliography part, the bibliographyPart cache is invalidated so the * exporter's `syncBibliographyPartToPackage` doesn't overwrite the * patched content with stale sources. * - * Returns `{ partName }` of the part that was patched, or `null` when - * the target couldn't be resolved. + * Returns `{ partName, id }` where `id` is the resolved itemID GUID + * (existing or freshly minted), or `null` when the target couldn't be + * resolved. `id` is omitted only when no Properties Part exists or was + * created — i.e. when `schemaRefs` wasn't patched and the part already + * lacked props. * * @throws {Error} when content is provided but not well-formed. */ @@ -509,11 +514,12 @@ export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs } convertedXml[partName] = createXmlDocument(root, existingDecl); } + let resolvedId = null; + if (schemaRefs !== undefined) { let propsPartName = findPropsPartFor(convertedXml, partName); - let itemId = null; if (propsPartName) { - itemId = parsePropsPart(convertedXml[propsPartName])?.itemId ?? null; + resolvedId = parsePropsPart(convertedXml[propsPartName])?.itemId ?? null; } if (!propsPartName) { // Foreign part had no Properties Part; create one now so the @@ -522,19 +528,26 @@ export function patchCustomXmlPart(convertedXml, target, { content, schemaRefs } if (idx == null) return null; propsPartName = propsPartNameFromIndex(idx); const itemRelsPath = `customXml/_rels/item${idx}.xml.rels`; - itemId = `{${uuidv4().toUpperCase()}}`; + resolvedId = `{${uuidv4().toUpperCase()}}`; convertedXml[itemRelsPath] = createXmlDocument(buildItemRelsRoot(`itemProps${idx}.xml`)); } - if (!itemId) itemId = `{${uuidv4().toUpperCase()}}`; + if (!resolvedId) resolvedId = `{${uuidv4().toUpperCase()}}`; const existingDecl = convertedXml[propsPartName]?.declaration; - convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(itemId, schemaRefs), existingDecl); + convertedXml[propsPartName] = createXmlDocument(buildItemPropsRoot(resolvedId, schemaRefs), existingDecl); + } else { + // schemaRefs wasn't touched — read the existing id, if any, so the + // caller always learns the id when one exists. + const propsPartName = findPropsPartFor(convertedXml, partName); + if (propsPartName) { + resolvedId = parsePropsPart(convertedXml[propsPartName])?.itemId ?? null; + } } // If we just patched the bibliography part, invalidate the cache so // the exporter doesn't overwrite our content from converter.bibliographyPart. if (converter) invalidateConverterCachesForPath(converter, partName); - return { partName }; + return resolvedId ? { partName, id: resolvedId } : { partName }; } /** diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts index e1cb30a1d9..c8b906208f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.integration.test.ts @@ -573,6 +573,39 @@ describe('customXml.parts write-side', () => { editor.destroy(); }); + it('surfaces the fresh itemID when patch creates a Properties Part on a foreign Storage Part', async () => { + // Seed: a Storage Part with no Properties Part (foreign producer + // case). The caller targets it by partName since there's no id. + const editor = await createEditorWithEmptyPackage(); + const converted = (editor as unknown as { converter: { convertedXml: Record } }).converter + .convertedXml; + converted[PART_NAME] = makeStorageDoc(); + // No Properties Part exists for PART_NAME. + + // Before patch, get returns the part with no itemID (no Properties Part exists). + const beforePatch = editor.doc.customXml.parts.get({ target: { partName: PART_NAME } }); + expect(beforePatch).not.toBeNull(); + expect(beforePatch!.id, 'no Properties Part means no itemID').toBeUndefined(); + + // Patch schemaRefs — this mints a fresh GUID and writes a new + // Properties Part. Caller should learn the new id from the result. + const patched = editor.doc.customXml.parts.patch({ + target: { partName: PART_NAME }, + schemaRefs: ['urn:foreign'], + }); + expect(patched.success).toBe(true); + if (!patched.success) return; + expect(patched.id, 'patch should surface the new itemID').toBeDefined(); + expect(patched.id).toMatch(/^\{[0-9A-F-]+\}$/); + + // Caller can now address the part by id. + const info = editor.doc.customXml.parts.get({ target: { id: patched.id! } }); + expect(info).not.toBeNull(); + expect(info!.schemaRefs).toEqual(['urn:foreign']); + + editor.destroy(); + }); + it('removing the bibliography part does not resurrect it via syncBibliographyPartToPackage on export', async () => { // Seed: simulate a doc with a bibliography custom XML part already // loaded. The converter's bibliographyPart cache will hold sources. diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts index 757347574b..686b10f287 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/custom-xml-wrappers.ts @@ -195,7 +195,7 @@ export function customXmlPartsPatchWrapper( options?: MutationOptions, ): CustomXmlPartsMutationResult { rejectTrackedMode('customXml.parts.patch', options); - const outcome = executeOutOfBandMutation>( + const outcome = executeOutOfBandMutation>( editor, (dryRun) => { if (dryRun) { @@ -205,7 +205,7 @@ export function customXmlPartsPatchWrapper( const probe = safeValidate(() => createCustomXmlPart({}, { content: input.content, schemaRefs: undefined })); if (isWriteFailure(probe)) return { changed: false, payload: probe }; } - return { changed: false, payload: { ok: true, payload: true } }; + return { changed: false, payload: { ok: true, payload: { id: null } } }; } // Resolve first so a missing target doesn't get reported as INVALID_INPUT. const partName = resolveTargetPartName(getConvertedXml(editor), input.target); @@ -220,12 +220,14 @@ export function customXmlPartsPatchWrapper( ); if (isWriteFailure(probe)) return { changed: false, payload: probe }; if (!probe.payload) return { changed: false, payload: targetNotFound() }; - return { changed: true, payload: { ok: true, payload: true } }; + return { changed: true, payload: { ok: true, payload: { id: probe.payload.id ?? null } } }; }, { dryRun: options?.dryRun === true, expectedRevision: options?.expectedRevision }, ); if (isWriteFailure(outcome)) return failure(outcome.code, outcome.message); - return { success: true, target: input.target }; + const result: CustomXmlPartsMutationResult = { success: true, target: input.target }; + if (outcome.payload.id) result.id = outcome.payload.id; + return result; } export function customXmlPartsRemoveWrapper( From 8ba69b908d5793aaacc9b072b427477db26fe855 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 09:07:30 -0300 Subject: [PATCH 14/14] wip(document-api): revert SuperConverter.d.ts typing, keep local cast (SD-3105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer caught a regression: adding removedCustomXmlPaths?: Set as an explicit field on SuperConverter.d.ts in ee06aa0d2 triggered TypeScript weak-type errors at three call sites that pass SuperConverter into local structural types not including the new field: - Editor.ts:1103 → ConverterWithDocumentSettings - HeaderFooterSessionManager.ts:703, 2322 → ConverterLike - PresentationEditor.ts:6039 → ConverterWithDocumentSettings Verified by running types:check with and without the d.ts change — errors only appear with the typed field present, because the [key: string]: any index signature alone is enough to satisfy weak types, but an explicit named field forces TypeScript to require at least one property overlap with the target shape. Per reviewer's smaller-fix suggestion: revert the d.ts change, restore the cast in Editor.ts with an AIDEV-NOTE pointing at this regression so future maintainers don't try the same simplification. Properly cleaning up the converter declaration is a separate piece of work (would need to enumerate the actual fields ConverterWithDocumentSettings / ConverterLike consume from real producers). Not in scope here. Verified: - 3432/3432 super-editor document-api-adapters + DocxZipper - SuperConverter weak-type errors no longer in types:check output --- packages/super-editor/src/editors/v1/core/Editor.ts | 9 ++++++++- .../editors/v1/core/super-converter/SuperConverter.d.ts | 8 -------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index 6f93715242..84a9907fa4 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -3335,7 +3335,14 @@ export class Editor extends EventEmitter { // Document API but originated in the imported DOCX. Without this, // the exporter would copy the original zip entry through, and the // removed part would reappear on the next import. - const removedCustomXmlPaths = this.converter.removedCustomXmlPaths; + // AIDEV-NOTE: `removedCustomXmlPaths` is set by `removeCustomXmlPart` + // (super-converter/custom-xml-parts.js) on the converter instance. + // Typed via cast rather than on SuperConverter.d.ts because adding + // an explicit field there triggers weak-type errors against + // ConverterWithDocumentSettings / ConverterLike structural types in + // sibling files that don't reference this field. + const removedCustomXmlPaths = (this.converter as unknown as { removedCustomXmlPaths?: Set }) + .removedCustomXmlPaths; if (removedCustomXmlPaths instanceof Set) { for (const path of removedCustomXmlPaths) { updatedDocs[path] = null; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts b/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts index b95a972049..74ef1a21d8 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.d.ts @@ -3,13 +3,5 @@ export class SuperConverter { static getStoredSuperdocVersion(...args: any[]): any; static setStoredSuperdocVersion(...args: any[]): void; static extractDocumentGuid(...args: any[]): string | null; - /** - * Set of package paths tombstoned by `customXml.parts.remove`. The - * exporter emits `updatedDocs[path] = null` for each entry so original - * imported parts don't survive into the exported zip. Mutated by - * `removeCustomXmlPart` (writer) and `createCustomXmlPart` (clears - * entries on index recycle); read by `Editor.exportDocx`. - */ - removedCustomXmlPaths?: Set; [key: string]: any; }