fix(isms): serialize approve/decline to prevent concurrent double-publish (CS-701)

Merged
TO
4mo ago
+125-37

Why

The production-deploy PR #3364 flagged a concurrent-approve race in isms.service.ts (CS-701, already on main). This PR fixes it — plus a closely-related decline race found while auditing. Base is main; it reaches release on the next deploy.

Root cause (verified)

assertPendingApprovalBy runs before db.$transaction. Under READ COMMITTED, a plain in-transaction status re-read does not serialize — the losing approval reads the still-committed needs_review. @@unique([documentId, version]) only converts the perfectly simultaneous interleaving into an error; the staggered interleaving silently creates a second published version and overwrites currentVersionId, orphaning the first.

Fix

  • approve(): takes a per-document advisory lock, then atomically claims the approval via updateMany({ where: { status: 'needs_review', approverId } }) inside the transaction, aborting when count !== 1. Exactly one caller freezes a version; losers abort before any derivation/version creation. The pre-transaction guard is kept for normal-case error messages.
  • decline(): same atomic claim, so an approve/decline race can no longer leave a declined document that still owns a live currentVersionId.
  • Renamed lockDocumentForPositions → lockDocument (it now serializes approval too), updating the four register-create callers.

Audit

I ran an adversarial audit of the whole approve/publish/version/export flow (concurrency, data-integrity, and cross-org/IDOR lenses, each independently verified). Confirmed-safe: getVersionExport org-scoping, S3 key derivation (no IDOR), snapshot parsing, currentVersionId integrity, and every migrated-doc read path. The only residual is a narrow, self-healing edit-during-approve window for register update/delete — low severity, noted as a follow-up rather than plumbing advisory locks through every edit path in a production hotfix.

Testing

  • 321 API ISMS tests passing, including new approve + decline race tests; typecheck clean.

Flagged in #3364. Follows #3361 (CS-701) and #3365.

🤖 Generated with Claude Code

https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp

Summary by cubic

Prevents double-publish by serializing ISMS approve/decline with a per-document advisory lock and an in-transaction atomic claim. Ensures CS-701 versioning keeps exactly one published version and consistent history.

  • Bug Fixes

    • approve() takes a per-document advisory lock and atomically claims the approval with updateMany(...) inside the transaction; only one caller proceeds, others abort before version creation.
    • decline() uses the same atomic claim to avoid approve/decline races that could leave a declined doc with a live currentVersionId.

  • Refactors

    • Renamed lockDocumentForPositions to lockDocument and updated register-create callers; the lock now also serializes approvals.
    • Added tests for racing approve/decline paths; kept the pre-transaction guard for clear, user-facing errors.