Emit delete tombstone when provider_account_id changes#1320
Conversation
The ClickHouse connected_accounts table is keyed by (project_id, branch_id, user_id, provider, provider_account_id), so updating providerAccountId in place on ProjectUserOAuthAccount orphaned the old CH sort-key row: the sync re-emitted the row under the new key while the old key sat untouched with no tombstone, and FINAL couldn't collapse them. verify-data-integrity would then report a row count mismatch for connected_accounts. Record a delete tombstone for the old row state before the update runs, so the sync pipeline marks the stale sort key as deleted.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes a ClickHouse data integrity issue where updating Confidence Score: 5/5Safe to merge — targeted fix that correctly follows the established tombstone pattern. The tombstone is recorded inside the same transaction, before the Prisma update, so No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant onUpdate
participant Prisma as Prisma (tx)
participant DeletedRow as DeletedRow table
participant CH as ClickHouse
Client->>onUpdate: PATCH provider (new account_id)
onUpdate->>Prisma: update allowSignIn / allowConnectedAccounts (if changed)
onUpdate->>onUpdate: detect account_id changed
onUpdate->>Prisma: recordExternalDbSyncDeletion (snapshots old providerAccountId)
Prisma->>DeletedRow: INSERT tombstone with old row state (old providerAccountId)
onUpdate->>Prisma: tx.projectUserOAuthAccount.update(new providerAccountId)
Prisma-->>onUpdate: updated record
onUpdate-->>Client: response
Note over DeletedRow,CH: Sync pipeline picks up tombstone
DeletedRow->>CH: DELETE row at old (project_id, branch_id, user_id, provider, old_account_id)
Prisma->>CH: UPSERT row at new (project_id, branch_id, user_id, provider, new_account_id)
Reviews (1): Last reviewed commit: "Emit delete tombstone when provider_acco..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/oauth-providers/crud.tsx`:
- Around line 306-312: The tombstone for ProjectUserOAuthAccount is being
recorded after other mutations in the same transaction causing the snapshot to
capture a mutated row; move the call to recordExternalDbSyncDeletion(tx, {
tableName: "ProjectUserOAuthAccount", tenancyId: auth.tenancy.id,
oauthAccountId: params.provider_id }) to occur immediately after
existingOAuthAccount is loaded (i.e., before any updates/deletes to
ProjectUserOAuthAccount happen in this transaction) so the function snapshots
the true pre-change row; ensure tx, auth.tenancy.id, params.provider_id and
existingOAuthAccount are used unchanged when relocating the call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c982e76b-5fe2-4241-ab9d-3e8e746cc206
📒 Files selected for processing (1)
apps/backend/src/app/api/latest/oauth-providers/crud.tsx
BilalG1
deleted the
fix-connected-accounts-ch-orphan-on-account-id-update
branch
2 participants
The ClickHouse connected_accounts table is keyed by (project_id, branch_id, user_id, provider, provider_account_id), so updating providerAccountId in place on ProjectUserOAuthAccount orphaned the old CH sort-key row: the sync re-emitted the row under the new key while the old key sat untouched with no tombstone, and FINAL couldn't collapse them. verify-data-integrity would then report a row count mismatch for connected_accounts.
Record a delete tombstone for the old row state before the update runs, so the sync pipeline marks the stale sort key as deleted.
Summary by CodeRabbit