fix(isms): lock control-link edits before write (CS-701)#3369
Merged
Conversation
…approve Follow-up flagged on #3364: addControls/removeControl wrote the control-link rows and only then called invalidateApprovalIfNeeded (which holds the per-document lock), so the write itself ran outside the lock — it could interleave with approve() before the lock was taken. (Impact was limited because control links are not part of the frozen version snapshot, but the mutation should still be serialized like every other content edit.) Both transactions now take lockDocument() as their first statement, before the createMany/deleteMany. The conditional invalidate is unchanged (real-change-only), and the lock is re-entrant so re-taking it there is a no-op. Adds an ordering test. Tests: 325 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.
2 Skipped Deployments
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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
2 participants
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
Flagged on the production-deploy PR #3364 (P2):
addControls/removeControlwrote the control-link rows first and only then calledinvalidateApprovalIfNeeded(which holds the per-document lock). So the link write ran outside the lock and could interleave withapprove()before serialization kicked in.Impact was limited — control links are not part of the frozen version snapshot, so no published version could be corrupted — but the mutation should still be serialized like every other content edit, and the lock-before-write pattern is the correct one.
Fix
Both control transactions now take
lockDocument()as their first statement, beforecreateMany/deleteMany. The conditionalinvalidateApprovalIfNeeded(real-change-only) is unchanged, and the advisory lock is transaction-scoped + re-entrant, so re-taking it there is a no-op. Added an ordering test asserting the lock precedes the write.Tests
main.Completes the ISMS concurrency hardening (#3366, #3367). Flagged in #3364.
🤖 Generated with Claude Code
https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp
Summary by cubic
Lock control-link edits before writes so
addControls/removeControlrun inside the per-document lock and serialize withapprove(). Implements CS-701.lockDocumentfirst in both transactions, beforecreateMany/deleteMany, to prevent interleaving withapprove().invalidateApprovalIfNeededconditional; the lock is re-entrant so re-taking it is a no-op. Added a test asserting the lock precedes the write.Written for commit b075f7c. Summary will update on new commits.