fix(isms): centrally serialize all content edits against approve (CS-701)#3367
Merged
Conversation
…ral lock) Follow-up to the approve-race fix: cubic correctly noted the advisory lock only protected paths that also take it, so a register UPDATE/DELETE (or narrative / control / regenerate) could still interleave while approve() loads and freezes the version, leaving an approved version that omits the edit — or an edit that never invalidated the approval. Enforced centrally instead of per-call-site: invalidateApprovalIfNeeded now takes the per-document advisory lock as its first action. Every content-mutation path already calls it (register create/update/delete, narrative save, control add/remove, regenerate), so all edits now serialize against approve() (which takes the same lock). The lock is transaction-scoped and re-entrant, so create paths that already acquire it for position allocation are unaffected. Adds utils/approval.spec.ts (lock-before-status ordering + revert/no-op behavior) and $executeRaw to the two tx mocks that exercise the real invalidate. Tests: 324 api ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
claudfuen
pushed a commit
that referenced
this pull request
Jul 7, 2026
# [3.99.0](v3.98.1...v3.99.0) (2026-07-07) ### Bug Fixes * **integrations:** don't persist unverified GitHub installation_id ([#3370](#3370)) ([9c6f6fa](9c6f6fa)) * **isms:** lock before writing control links so edits serialize with approve ([#3369](#3369)) ([a21f866](a21f866)) * **isms:** production-deploy review follow-ups (CS-701) ([#3365](#3365)) ([d756d4a](d756d4a)), closes [#3364](#3364) * **isms:** serialize approve against ALL register/content edits (central lock) ([#3367](#3367)) ([5c88a3d](5c88a3d)) * **isms:** serialize approve/decline to prevent concurrent double-publish (CS-701) ([#3366](#3366)) ([b158bc9](b158bc9)), closes [#3364](#3364) ### Features * **integrations:** add read-only GitHub App integration (CS-710) ([#3363](#3363)) ([a5a9243](a5a9243)) * **isms:** document versioning and history (CS-701) ([#3361](#3361)) ([d95dc2e](d95dc2e))
Contributor
|
🎉 This PR is included in version 3.99.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Follow-up to #3366. On the production-deploy review (#3364), cubic correctly flagged (P1) that #3366's approve-race fix was incomplete: the advisory lock only protected paths that also take it, so a register update/delete (or narrative save / control link / regenerate) could still interleave while
approve()loadsEXPORT_DOCUMENT_INCLUDEand freezes the version — leaving an approved version that omits the edit, or an edit that never invalidated the approval.Fix — enforced centrally (as cubic suggested)
Rather than add the lock to ~12 scattered transactions,
invalidateApprovalIfNeedednow takes the per-document advisory lock as its first action. Every content-mutation path already calls it — register create/update/delete, narrative save, control add/remove, and regenerate — so all edits now serialize againstapprove()(which takes the same lock). Either the edit is frozen into the version, or it correctly reverts the document to draft; no interleaving can do neither.The lock is transaction-scoped and re-entrant, so the register create paths that already acquire it for
max(position)allocation are unaffected.Tests
utils/approval.spec.ts(lock-taken-before-status-read ordering + revert / no-op behavior).$executeRawadded to the two tx mocks that exercise the realinvalidateApprovalIfNeeded.main(incl. fix(isms): serialize approve/decline to prevent concurrent double-publish (CS-701) #3366).Closes the residual flagged on #3364; completes #3366.
🤖 Generated with Claude Code
https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp
Summary by cubic
Centralizes the per-document advisory lock so all ISMS content edits serialize against approval, fixing a race that could freeze an incomplete version or let edits bypass invalidation. Supports CS-701 by ensuring published version snapshots are always accurate.
Bug Fixes
invalidateApprovalIfNeedednow acquires the per-document advisory lock first, so every edit path (register update/delete, narrative save, control link, regenerate) serializes againstapprove().Tests
utils/approval.spec.tsto assert lock-before-status ordering and revert/no-op behavior.$executeRawin@db-backed specs.Written for commit 96dfb64. Summary will update on new commits.