Skip to content

fix(isms): centrally serialize all content edits against approve (CS-701)#3367

Merged
tofikwest merged 1 commit into
mainfrom
tofik/isms-central-approval-lock
Jul 7, 2026
Merged

fix(isms): centrally serialize all content edits against approve (CS-701)#3367
tofikwest merged 1 commit into
mainfrom
tofik/isms-central-approval-lock

Conversation

@tofikwest

@tofikwest tofikwest commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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() loads EXPORT_DOCUMENT_INCLUDE and 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, 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, and regenerate — so all edits now serialize against approve() (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

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

    • invalidateApprovalIfNeeded now acquires the per-document advisory lock first, so every edit path (register update/delete, narrative save, control link, regenerate) serializes against approve().
    • Prevents interleaving under READ COMMITTED: either the edit is included in the approved version or the document cleanly reverts to draft.
    • Lock is transaction-scoped and re-entrant, so existing create paths using the lock for position allocation are unaffected.
  • Tests

    • Added utils/approval.spec.ts to assert lock-before-status ordering and revert/no-op behavior.
    • Updated test transaction mocks to include $executeRaw in @db-backed specs.

Written for commit 96dfb64. Summary will update on new commits.

Review in cubic

…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
@linear

linear Bot commented Jul 7, 2026

Copy link
Copy Markdown

CS-701

@vercel

vercel Bot commented Jul 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
comp-framework-editor Ready Ready Preview, Comment Jul 7, 2026 8:38pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
app Skipped Skipped Jul 7, 2026 8:38pm
portal Skipped Skipped Jul 7, 2026 8:38pm

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@tofikwest tofikwest merged commit 5c88a3d into main Jul 7, 2026
10 checks passed
@tofikwest tofikwest deleted the tofik/isms-central-approval-lock branch July 7, 2026 20:40
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))
@claudfuen

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.99.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants