Skip to content

refactor(db): require a transaction for advisory lock helpers - #8364

Merged
waleedlatif1 merged 2 commits into
stagingfrom
refactor/advisory-lock-transaction-type
Sep 28, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
refactor/advisory-lock-transaction-type

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • acquireAdvisoryXactLock / tryAcquireAdvisoryXactLock took DbOrTx, which also admits the pool-level db. A transaction-scoped advisory lock taken on the pool runs in autocommit and releases immediately, so the caller would run unprotected. No call site did this, but nothing stopped it
  • Both helpers and every lock wrapper that forwards to them now take DbTransaction, so that misuse fails at compile time. Also narrowed: the org mutation/membership locks and the MCP transaction lock timeout, the same xact-scoped hazard, and withTransactionRetry's callback, whose argument is always a transaction
  • There's no runtime change. Every hunk is a type annotation or import, except three signatures made behavior-identical for every caller:
    • requirePersonalTokenEnrollment(input, lockingTx?) replaces (input, executor, lock); no caller ever passed a tx without locking or locked without a tx
    • loadReviewPreview(row, prepared, lockingTx?): preview still reads on db, resolve still locks FOR UPDATE on its tx
    • the access-request use-case wrapper gives mutations a DbTransaction and reads db, with the same authorize → prepare → tx → lock → re-authorize → execute → audit order
  • Responds to a review thread on v0.9.4: db contention fixes, databricks genie, snowflake cortex, additional search connectors #8356

Type of Change

  • Other: type-level hardening

Testing

  • Type-check, lint, and check:audits pass
  • Unit suites across every touched module (1,008 files) and 15 integration suites touching the changed modules pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 28, 2026 1:59am UTC

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 42 files

Confidence score: 5/5

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

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Refactors database transaction handling for advisory locks.

The PR appears safe to merge; no outstanding finding or new actionable issue was identified.

Summary

This PR narrows transaction-scoped advisory-lock helpers and their callers to DbTransaction. The only changes since the previous review clarify in documentation why a pool-level client must not be used.

  • No new actionable issue was identified in those changes.

Reviews (2) · Last reviewed commit: "chore(db): document why the advisory loc..."

Comment thread apps/sim/lib/db/advisory-locks.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@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 42 files

Confidence score: 5/5

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

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 9070d1b into staging Sep 28, 2026
32 checks passed
@waleedlatif1
waleedlatif1 deleted the refactor/advisory-lock-transaction-type branch September 28, 2026 02:07

This branch was previously deployed

1 inactive deployment
Preview — 56a3764b Deployed Sep 28, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant