fix(content-controls): mutate locked SDTs via AttrSteps and inner-range writes (SD-3123) (#3287)

* fix(content-controls): mutate locked SDTs via AttrSteps and inner-range writes (SD-3123)

Per ECMA-376, sdtLocked protects the SDT wrapper from removal but the content
stays editable. Before this fix, programmatic mutations flowed through
editor.commands.updateStructuredContentById, which dispatches
tr.replaceWith(pos, pos + node.nodeSize, ...) spanning the entire SDT. The
structured-content lock plugin's filterTransaction reads that as wrapper
damage and silently filters the transaction on sdtLocked controls -
producing false-success no-ops on operations the API allows (text.setValue,
replaceContent, appendContent, prependContent, setLockMode).

Two engine changes:

- applyAttrsUpdate (sdt-properties-write.ts) - switch from
  editor.commands.updateStructuredContentById to per-key tr.setNodeAttribute.
  AttrSteps have no from/to range and are explicitly skipped by the lock
  plugin's filterTransaction. This is the path that patch, setLockMode,
  patchRawProperties, setType, and the date/binding/multiline family use.
- replaceSdtTextContent (content-controls-wrappers.ts) - replace the SDT's
  inner content range (pos+1 to pos+nodeSize-1) instead of rewriting the
  whole wrapper. The lock plugin classifies inner-range steps as
  wouldModifyContent: allowed on sdtLocked, still blocked on contentLocked
  / sdtContentLocked (both per spec).

The search in applyAttrsUpdate uses SDT_NODE_NAMES (imported from
target-resolution.ts) so it resolves the same nodes the upstream
resolveSdtByTarget would resolve. documentSection / documentPartObject are
intentionally excluded - they have their own write paths and could collide
on id.

No changes to API-level lock guards (assertNotSdtLocked on patch, setType,
setBinding, etc.) - relaxing those is a separate behavior decision and out
of scope here.

Tests:
- 4 new regression tests for sdtLocked (setLockMode, text.setValue,
  text.clearValue, replaceContent) that assert the actual inner-range
  positions (pos+1, pos+nodeSize-1), not just call counts.
- 11 existing tests migrated from "expect updateStructuredContentById
  called" to "expect tr.setNodeAttribute / tr.replaceWith / tr.delete".
- 19 metadata-mutation operations added to CC_DIRECT_DISPATCH_OPS in
  conformance (no synthetic-failure mode after this refactor, matching the
  existing precedent for wrap/unwrap/etc.).

All pass: 46 wrappers + 1170 conformance + 104 SDT-adjacent + 1398
document-api. Zero new TypeScript errors.

Related: SD-3131, SD-3139, IT-1046.

* test(content-controls): add clearContent sdtLocked regression test (SD-3123)

Author: caio-pizzol

packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts

@@ -627,6 +627,7 @@ function makeTextEditor(
     addMark: vi.fn(),
     removeMark: vi.fn(),
     replaceWith: vi.fn(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     insert: vi.fn(),
     setMeta: vi.fn(),
     mapping: { map: (pos: number) => pos },
@@ -1294,6 +1295,7 @@ function makeTableEditor(
     delete: vi.fn().mockReturnThis(),
     setNodeMarkup: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     insert: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: {
@@ -1638,15 +1640,26 @@ const NON_RECEIPT_MUTATION_OPS: ReadonlySet<OperationId> = new Set([
 ] as OperationId[]);
 
 /**
- * Content-control operations whose handlers always return `true` because they
- * build and dispatch their own ProseMirror transaction directly (via
- * `editor.view!.dispatch(tr)`) rather than delegating to an editor command whose
- * boolean result propagates to the domain-command executor.
+ * Content-control operations excluded from the structured-failure conformance
+ * check because they have no synthetic-failure path that
+ * `makeNoOpSdtEditor` can simulate.
  *
- * Because the handler always returns `true`, the `domain.command` executor marks
- * the step effect as `'changed'` and `executeSdtMutation` returns success.
- * There is no code path that produces the `NO_OP` structured failure for these
- * operations, so they are excluded from the failureCase conformance check.
+ * The originals (wrap, unwrap, copy, move, insertBefore, insertAfter, group
+ * wrap/ungroup, repeatingSection insertItem/cloneItem/deleteItem) build and
+ * dispatch their own PM transaction directly via `editor.view!.dispatch(tr)`
+ * rather than delegating to an editor command whose boolean result propagates
+ * back through the executor. The SD-3123 additions (patch, setLockMode,
+ * setType, setBinding, clearBinding, patchRawProperties, text.setMultiline,
+ * the date family, the checkbox family, the choiceList family, and
+ * repeatingSection.setAllowInsertDelete) no longer route through
+ * `editor.commands.updateStructuredContentById`; the synthetic
+ * `updateStructuredContentById = vi.fn(() => false)` mock that previously
+ * drove the failure case has no effect on the AttrStep / inner-range write
+ * path.
+ *
+ * In both groups, the operations can still fail in production (missing target,
+ * lock violation, schema invalidation in PM dispatch). They just don't have a
+ * clean synthetic failure mode reachable from the mock editor.
  */
 const CC_DIRECT_DISPATCH_OPS: ReadonlySet<OperationId> = new Set([
   'contentControls.wrap',
@@ -1661,6 +1674,28 @@ const CC_DIRECT_DISPATCH_OPS: ReadonlySet<OperationId> = new Set([
   'contentControls.repeatingSection.insertItemAfter',
   'contentControls.repeatingSection.cloneItem',
   'contentControls.repeatingSection.deleteItem',
+  // SD-3123: synthetic noop-mock failure (updateStructuredContentById=false)
+  // no longer applies — these now write via tr.setNodeAttribute (metadata)
+  // or tr.replaceWith on the SDT inner range (content).
+  'contentControls.patch',
+  'contentControls.patchRawProperties',
+  'contentControls.setLockMode',
+  'contentControls.setType',
+  'contentControls.setBinding',
+  'contentControls.clearBinding',
+  'contentControls.text.setMultiline',
+  'contentControls.date.setValue',
+  'contentControls.date.clearValue',
+  'contentControls.date.setDisplayFormat',
+  'contentControls.date.setDisplayLocale',
+  'contentControls.date.setStorageFormat',
+  'contentControls.date.setCalendar',
+  'contentControls.checkbox.setState',
+  'contentControls.checkbox.toggle',
+  'contentControls.checkbox.setSymbolPair',
+  'contentControls.choiceList.setItems',
+  'contentControls.choiceList.setSelected',
+  'contentControls.repeatingSection.setAllowInsertDelete',
 ] as OperationId[]);
 
 const HAS_STRUCTURED_FAILURE_RESULT = (operationId: OperationId): boolean =>
@@ -2367,6 +2402,7 @@ function makeTocEditor(commandOverrides: Record<string, unknown> = {}): Editor {
     insert: vi.fn().mockReturnThis(),
     setNodeMarkup: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: { map: (pos: number) => pos },
     docChanged: true,
@@ -2439,6 +2475,7 @@ function makeImageEditor(): Editor {
     insert: vi.fn().mockReturnThis(),
     setNodeMarkup: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: { map: (pos: number) => pos },
     docChanged: true,
@@ -2513,6 +2550,7 @@ function makeMultiBlockImageEditor(): Editor {
     insert: vi.fn().mockReturnThis(),
     setNodeMarkup: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: { map: (pos: number) => pos },
     docChanged: true,
@@ -2675,6 +2713,7 @@ function makeSdtEditor(overrideAttrs: Record<string, unknown> = {}, textContent
     addMark: vi.fn().mockReturnThis(),
     removeMark: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     insert: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: { map: (pos: number) => pos },
@@ -2769,6 +2808,7 @@ function makeSdtEditorWithRepeatingSectionItems(): Editor {
     addMark: vi.fn().mockReturnThis(),
     removeMark: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     insert: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: { map: (pos: number) => pos },
@@ -2888,6 +2928,7 @@ function makeCaptionImageEditor(
     insert: vi.fn().mockReturnThis(),
     delete: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     setNodeMarkup: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     mapping: { map: (pos: number) => pos },
@@ -2953,6 +2994,7 @@ function makeRefEditor(
     addMark: vi.fn().mockReturnThis(),
     removeMark: vi.fn().mockReturnThis(),
     replaceWith: vi.fn().mockReturnThis(),
+    setNodeAttribute: vi.fn().mockReturnThis(),
     insert: vi.fn().mockReturnThis(),
     setMeta: vi.fn().mockReturnThis(),
     setNodeMarkup: vi.fn().mockReturnThis(),
@@ -11984,6 +12026,7 @@ describe('document-api adapter conformance', () => {
         insert: vi.fn().mockReturnThis(),
         setNodeMarkup: vi.fn().mockReturnThis(),
         replaceWith: vi.fn().mockReturnThis(),
+        setNodeAttribute: vi.fn().mockReturnThis(),
         setMeta: vi.fn().mockReturnThis(),
         mapping: { map: (pos: number) => pos },
         docChanged: true,

packages/super-editor/src/editors/v1/document-api-adapters/helpers/content-controls/sdt-properties-write.ts

@@ -11,7 +11,7 @@
 
 import type { Editor } from '../../../core/Editor.js';
 import type { ContentControlTarget } from '@superdoc/document-api';
-import { resolveSdtByTarget } from './target-resolution.js';
+import { resolveSdtByTarget, SDT_NODE_NAMES } from './target-resolution.js';
 
 // ---------------------------------------------------------------------------
 // XML element helpers for sdtPr.elements
@@ -68,13 +68,64 @@ function removeSdtPrChild(sdtPr: SdtPrElement, childName: string): SdtPrElement
 // ---------------------------------------------------------------------------
 
 /**
- * Apply an attribute patch to an SDT node via updateStructuredContentById.
- * Returns true if the command executed successfully.
+ * PM node-type names that carry a content-control SDT identity. Imported from
+ * `target-resolution.ts` so the search here resolves the same nodes the
+ * upstream `resolveSdtByTarget` would resolve. `documentSection` and
+ * `documentPartObject` are intentionally not in `SDT_NODE_NAMES` — they have
+ * their own write paths and could otherwise collide on `id` and cause this
+ * loop to find and mutate the wrong node.
+ */
+const SDT_NODE_TYPES = new Set<string>(SDT_NODE_NAMES);
+
+/**
+ * Apply an attribute patch to an SDT node.
+ *
+ * Uses `tr.setNodeAttribute` per key, which emits PM AttrSteps. AttrSteps
+ * have no `from`/`to` range and are explicitly skipped by the structured-
+ * content lock plugin's `filterTransaction`, so this path can mutate
+ * metadata (id, tag, alias, lockMode, controlType, sdtPr, appearance, ...)
+ * on `sdtLocked` and `sdtContentLocked` controls without tripping the
+ * wrapper-damage check.
+ *
+ * The previous implementation delegated to `editor.commands.updateStructuredContentById`,
+ * which dispatches `tr.replaceWith(pos, pos + node.nodeSize, ...)`. That
+ * step's range covered the entire SDT, which the lock plugin read as
+ * wrapper damage and silently filtered for locked controls — producing
+ * false-success mutations.
+ *
+ * Returns true if a matching SDT was found and the transaction dispatched,
+ * false if no SDT matched the given id or the editor cannot dispatch.
  */
 export function applyAttrsUpdate(editor: Editor, nodeId: string, attrsPatch: Record<string, unknown>): boolean {
-  const updateCmd = editor.commands?.updateStructuredContentById;
-  if (typeof updateCmd !== 'function') return false;
-  return Boolean(updateCmd(nodeId, { attrs: attrsPatch }));
+  if (!editor?.state) return false;
+
+  let foundPos: number | null = null;
+  editor.state.doc.descendants((node, pos) => {
+    if (foundPos !== null) return false;
+    if (SDT_NODE_TYPES.has(node.type.name) && String(node.attrs.id) === String(nodeId)) {
+      foundPos = pos;
+      return false;
+    }
+    return true;
+  });
+
+  if (foundPos === null) return false;
+
+  const tr = editor.state.tr;
+  for (const [key, value] of Object.entries(attrsPatch)) {
+    tr.setNodeAttribute(foundPos, key, value);
+  }
+
+  if (tr.steps.length === 0) return true;
+
+  if (editor.view?.dispatch) {
+    editor.view.dispatch(tr);
+  } else if (typeof editor.dispatch === 'function') {
+    editor.dispatch(tr);
+  } else {
+    return false;
+  }
+  return true;
 }
 
 // ---------------------------------------------------------------------------