Skip to content

Daily help review XP cap is a check then act race, not atomic #400

Description

@jakharmonika364

What happened?

process-review-event.ts enforces a daily cap on help review XP by reading a count first, then deciding whether to allow the new event:

const { count: todaysReviewCount } = await sb
  .from('xp_events')
  .select('id', { count: 'exact', head: true })
  .eq('user_id', reviewer.id)
  .eq('source', XP_SOURCE.HELP_REVIEW)
  .gte('created_at', dayStartIso);

const cap = applyCap('review', todaysReviewCount ?? 0, 1);
if (!cap.allowed) {
  return { skipped: true, reason: 'daily_review_cap_reached' };
}

There is no transaction tying the read to the later insert. Two qualifying reviews by the same reviewer on two different PRs, arriving close together, can both read the same pre insert count, both pass the cap check, and both insert, going over the daily cap. The per row idempotency key is user_id, source, ref_id, which doesn't help here since ref_id is different for each review.

Steps to Reproduce

  1. Have the same reviewer submit two qualifying help reviews on two different PRs within a short window, close enough that both webhook deliveries process before either one's insert lands
  2. Both reviews get XP, putting the reviewer over the documented daily cap

Expected Behavior

The daily cap should hold even under concurrent reviews from the same user.

Where does this occur?

API (XP pipeline, help review webhook handler)

Additional Context

Could be fixed with a unique constraint plus a counting trick, or by moving the check into the same transaction as the insert instead of two separate round trips.

Metadata

Metadata

Assignees

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions