Skip to content

fix(cards): prevent concurrent deletions from removing all user cards#376

Open
Ridanshi wants to merge 1 commit into
Dev-Card:mainfrom
Ridanshi:fix/card-delete-transaction-race
Open

fix(cards): prevent concurrent deletions from removing all user cards#376
Ridanshi wants to merge 1 commit into
Dev-Card:mainfrom
Ridanshi:fix/card-delete-transaction-race

Conversation

@Ridanshi

Copy link
Copy Markdown
Contributor

Summary

Fixes a transactional race condition in card deletion that could allow concurrent requests to remove all cards belonging to a user despite intended last-card protection.

Root Cause

The delete flow previously executed:

  1. ownership lookup
  2. card count validation
  3. delete operation

as separate non-transactional database operations.

Under concurrent requests, multiple deletes could simultaneously observe the same card count and proceed, resulting in users ending up with zero cards.

Fix

Moved the deletion guard and delete flow into a single Prisma $transaction using Serializable isolation.

The transaction now atomically performs:

  • card ownership validation
  • last-card protection
  • default-card promotion
  • delete operation

This guarantees that concurrent delete requests cannot violate the invariant that a user must always retain at least one card.

Concurrency Behavior

Before:

  • two concurrent deletes against 2 cards could both succeed
  • final state could become 0 cards

After:

  • only one delete can succeed
  • competing transaction safely aborts
  • user always retains at least one card

Tests Added

Added focused regression coverage for:

  • successful delete flow
  • default-card promotion
  • last-card protection
  • concurrent deletion scenarios
  • transactional invariant preservation
  • failure handling

Files Changed

  • apps/backend/src/routes/cards.ts
  • apps/backend/src/__tests__/cards.test.ts

Closes #356

@Ridanshi Ridanshi force-pushed the fix/card-delete-transaction-race branch from dea2add to 1f7baae Compare May 28, 2026 17:14
@Harxhit Harxhit added the gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. label May 28, 2026
@Harxhit

Harxhit commented May 31, 2026

Copy link
Copy Markdown
Collaborator

@Ridanshi Fix merge conflicts and please add terminal screen short proof for unit tests.

@Ridanshi Ridanshi force-pushed the fix/card-delete-transaction-race branch from 1f7baae to a16b19a Compare May 31, 2026 10:16
@Ridanshi

Copy link
Copy Markdown
Contributor Author

Conflicts resolved and branch updated.

Resolution summary:

  • Kept upstream's cardService-based delete flow after the refactor.
  • Applied the Serializable transaction isolation fix in cardService.deleteCard(), which is now the authoritative deletion path.
  • Removed obsolete route-level transaction logic and unused imports.

Unit tests rerun successfully:

npx vitest run src/tests/cards.test.ts --reporter=verbose

Result:
✓ 25/25 tests passed

This includes the concurrency regression coverage added in this PR:

  • runs the count check and deletion inside a single transaction
  • blocks a concurrent request whose transaction-internal count reflects a prior committed delete
  • allows exactly one of two racing deletes to succeed
  • user always retains at least one card regardless of which concurrent request wins

Terminal proof attached.
Screenshot 2026-05-31 155326

@Ridanshi

Ridanshi commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi, could someone please review this PR when convenient? I've completed the implementation and tested it locally. Happy to make any required changes. Thanks!

@ShantKhatri ShantKhatri requested a review from Harxhit June 6, 2026 17:26

@Harxhit Harxhit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address lint errors.

The DELETE /api/cards/:id handler performed an ownership lookup, a card
count check, and the delete as three separate non-transactional operations.
Under concurrent requests both could observe count > 1 and both proceed to
delete, leaving the user with zero cards.

Move the count guard, optional default-card promotion, and the delete into
a single Prisma $transaction with Serializable isolation. The database now
serializes concurrent count reads against the write, rolling back the second
conflicting transaction so the invariant (user always retains at least one
card) cannot be violated even under load.

Adds a focused test suite covering normal deletion, last-card rejection,
default-card promotion, and four concurrency-guard scenarios.
@Ridanshi Ridanshi force-pushed the fix/card-delete-transaction-race branch from a16b19a to 5068940 Compare June 8, 2026 15:52
@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown

@Ridanshi is attempting to deploy a commit to the Prashantkumar Khatri's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

CI — All Checks Passed

Backend — PASS

Check Result
Lint PASS
Test PASS
Typecheck PASS

Mobile — SKIP

Check Result
Lint -
Test -

Web — SKIP

Check Result
Check -
Build -

Last updated: Mon, 08 Jun 2026 15:53:43 GMT

@Ridanshi

Ridanshi commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

I've addressed the lint issues and pushed the fixes. All backend checks are now passing. Could you please re-review the PR?

@Harxhit

Harxhit commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

It looks like this PR still uses the previous PNPM workspace configuration. The repository has been migrated to NPM workspaces, so the dependency configuration should be updated accordingly before this can be merged.

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

Labels

gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent card deletion race condition allows users to end up with zero cards despite last-card protection

2 participants