Skip to content

fix(document-api): allow Document API to update contentLocked SDTs#3718

Open
mattConnHarbour wants to merge 2 commits into
mainfrom
matthew/sd-3429-allow-document-api-replacecontent-lock-bypass
Open

fix(document-api): allow Document API to update contentLocked SDTs#3718
mattConnHarbour wants to merge 2 commits into
mainfrom
matthew/sd-3429-allow-document-api-replacecontent-lock-bypass

Conversation

@mattConnHarbour

Copy link
Copy Markdown
Contributor

Summary

  • contentLocked SDTs block inner-content modification via the lock plugin
  • Document API should still be able to update them programmatically
  • Use whole-node replacement: replace the entire SDT node instead of modifying inner positions
  • This preserves all attributes (including ID) while bypassing lock plugin position checks

Changes

  • Add assertNotFullyLocked() - rejects only sdtContentLocked (fully locked)
  • Add requiresWholeNodeReplacement() - returns true for contentLocked
  • Add replaceEntireSdt() - atomic replacement preserving original attrs
  • Update text.setValue, text.clearValue, replaceContent, clearContent to use whole-node replacement for contentLocked SDTs

Lock mode behavior

Lock Mode Inner-content edit Whole-node replace Document API
unlocked ✅ normal
sdtLocked ✅ normal
contentLocked ✅ whole-node
sdtContentLocked ❌ rejected

Test plan

  • Create contentLocked SDT
  • Verify text.setValue updates it successfully
  • Verify the SDT ID is preserved after update
  • Verify sdtContentLocked SDTs are still rejected

Fixes SD-3429

🤖 Generated with Claude Code

@mattConnHarbour mattConnHarbour requested a review from a team as a code owner June 11, 2026 20:33
@linear-code

linear-code Bot commented Jun 11, 2026

Copy link
Copy Markdown

SD-3429

contentLocked SDTs block inner-content modification via the lock plugin,
but the Document API should still be able to update them programmatically.

Use whole-node replacement for contentLocked SDTs: replace the entire node
(wrapper + content) instead of modifying inner positions. This preserves
all attributes including the ID while bypassing the lock plugin's position
checks.

- Add assertNotFullyLocked() - rejects only sdtContentLocked (fully locked)
- Add requiresWholeNodeReplacement() - returns true for contentLocked
- Add replaceEntireSdt() - atomic replacement preserving original attrs
- Update text.setValue, text.clearValue, replaceContent, clearContent

SD-3429

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mattConnHarbour mattConnHarbour force-pushed the matthew/sd-3429-allow-document-api-replacecontent-lock-bypass branch from f6fb383 to 5954387 Compare June 11, 2026 20:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6fb3831be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +422 to +424
tr.replaceWith(from, to, updatedNode);
dispatchTransaction(editor, tr);
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Report failure when whole-node replacement is filtered

When this new contentLocked path targets an SDT nested inside a locked ancestor, the structured-content lock plugin can reject the dispatched replacement transaction without throwing; replaceEntireSdt still returns true, and executeSdtMutation then reports a successful mutation even though the document did not change. This is especially reachable because nested content controls are supported by the API, so the whole-node path should verify that the dispatch actually changed the document or preflight ancestor locks before returning success.

Useful? React with 👍 / 👎.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

- Detect silent transaction rejection in replaceEntireSdt by comparing
  doc references before/after dispatch (fixes nested locked controls
  false-success path)
- Add comprehensive regression tests for contentLocked whole-node
  replacement code path (SD-3429)
- Document the semantic bypass intent in lock-enforcement.ts and
  structured-content-lock-plugin.js
- Remove misleading "recovery" and "atomicity" comments

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants