diff --git a/__tests__/invite-claim.test.ts b/__tests__/invite-claim.test.ts index d241059..f974c3e 100644 --- a/__tests__/invite-claim.test.ts +++ b/__tests__/invite-claim.test.ts @@ -54,7 +54,7 @@ describe('claimInviteUse (invite max_uses TOCTOU fix)', () => { it('claims a use when under the cap and returns the new count', async () => { const admin = makeAdmin({ uses_count: 0, max_uses: 1 }) const result = await claimInviteUse(admin, 'inv-1', 0, 1) - expect(result).toBe(1) + expect(result).toEqual({ ok: true, count: 1 }) expect(admin._row.uses_count).toBe(1) }) @@ -62,7 +62,7 @@ describe('claimInviteUse (invite max_uses TOCTOU fix)', () => { const admin = makeAdmin({ uses_count: 1, max_uses: 1 }) // Second redeemer read uses_count=0 stale, but the row is already at 1. const result = await claimInviteUse(admin, 'inv-1', 0, 1) - expect(result).toBeNull() + expect(result).toEqual({ ok: false, reason: 'cap_reached' }) expect(admin._row.uses_count).toBe(1) // unchanged — not incremented past cap }) @@ -70,23 +70,67 @@ describe('claimInviteUse (invite max_uses TOCTOU fix)', () => { // max_uses=5, another redeemer already bumped it to 1 after our stale read of 0. const admin = makeAdmin({ uses_count: 1, max_uses: 5 }) const result = await claimInviteUse(admin, 'inv-1', 0, 5) - expect(result).toBe(2) // re-read to 1, then CAS-claimed to 2 + expect(result).toEqual({ ok: true, count: 2 }) // re-read to 1, then CAS-claimed to 2 expect(admin._row.uses_count).toBe(2) }) it('treats an unlimited invite (max_uses null) as always claimable', async () => { const admin = makeAdmin({ uses_count: 99, max_uses: null }) const result = await claimInviteUse(admin, 'inv-1', 99, null) - expect(result).toBe(100) + expect(result).toEqual({ ok: true, count: 100 }) }) - it('gives up (null) if it cannot win the CAS within the attempt budget', async () => { + it('reports contention (not cap_reached) when it loses the CAS every attempt under a high cap', async () => { + // A perpetually-contended invite with plenty of headroom (cap 1000): every + // CAS loses because a competing writer keeps advancing the row between our + // re-read and our next update, so we never win within the attempt budget. + // The row stays well under cap, so the failure is contention — a false + // 'cap_reached' here would wrongly turn away a legitimate joiner. + const row = { uses_count: 5, max_uses: 1000 as number | null } + const admin = { + from() { + return { + update() { + const builder: Record = { + eq() { + return builder + }, + select() { + // Always lose the CAS: a competitor also bumped the row. + row.uses_count += 1 + return Promise.resolve({ data: [] }) + }, + } + return builder + }, + select() { + const builder: Record = { + eq() { + return builder + }, + maybeSingle() { + return Promise.resolve({ data: { uses_count: row.uses_count } }) + }, + } + return builder + }, + } + }, + } as unknown as Parameters[0] + + const result = await claimInviteUse(admin, 'inv-1', 5, 1000, 5) + expect(result).toEqual({ ok: false, reason: 'contention' }) + expect(row.uses_count).toBeLessThan(1000) // never hit the cap + }) + + it('gives up (contention) if it cannot win the CAS within the attempt budget', async () => { // Caller's known count (0) is behind the live row (3), so the first CAS // guard misses; with a 1-attempt budget it re-reads and then gives up - // rather than looping — proving the retry is bounded, not infinite. + // rather than looping — proving the retry is bounded, not infinite. Cap is + // far off (10), so this is contention, not a reached cap. const admin = makeAdmin({ uses_count: 3, max_uses: 10 }) const result = await claimInviteUse(admin, 'inv-1', 0, 10, 1) - expect(result).toBeNull() + expect(result).toEqual({ ok: false, reason: 'contention' }) expect(admin._row.uses_count).toBe(3) // untouched }) }) diff --git a/lib/auth-actions.ts b/lib/auth-actions.ts index e386779..59b0e72 100644 --- a/lib/auth-actions.ts +++ b/lib/auth-actions.ts @@ -182,8 +182,16 @@ export async function joinSpace(formData: { // (The pre-check above is a fast UX reject; this is the authoritative guard.) let claimedCount: number | null = null if (invite) { - claimedCount = await claimInviteUse(admin, invite.id, invite.uses_count, invite.max_uses) - if (claimedCount === null) return { error: 'This invite has reached its use cap.' } + const claim = await claimInviteUse(admin, invite.id, invite.uses_count, invite.max_uses) + if (!claim.ok) { + return { + error: + claim.reason === 'contention' + ? 'Too many people are joining at once — please try again.' + : 'This invite has reached its use cap.', + } + } + claimedCount = claim.count } const { data: newMember, error: memberErr } = await admin diff --git a/lib/invite-claim.ts b/lib/invite-claim.ts index f0ec58b..4fee9c4 100644 --- a/lib/invite-claim.ts +++ b/lib/invite-claim.ts @@ -12,20 +12,27 @@ type Admin = ReturnType * 0-row result, re-reads, and retries while capacity remains. `uses_count` only * ever increases, so the CAS has no ABA hazard. * - * Returns the new `uses_count` on success (the caller stores it to roll the - * claim back if the subsequent membership insert fails), or `null` if the cap - * is reached. + * Returns a discriminated result: `{ ok: true, count }` with the new + * `uses_count` on success (the caller stores it to roll the claim back if the + * subsequent membership insert fails), or `{ ok: false, reason }` where the + * reason distinguishes a genuinely reached cap (`'cap_reached'`) from losing + * the compare-and-swap on every attempt under contention (`'contention'`) so + * the caller can surface a retryable message instead of a false cap error. */ +export type ClaimInviteResult = + | { ok: true; count: number } + | { ok: false; reason: 'cap_reached' | 'contention' } + export async function claimInviteUse( admin: Admin, inviteId: string, knownCount: number, maxUses: number | null, maxAttempts = 5, -): Promise { +): Promise { let current = knownCount for (let attempt = 0; attempt < maxAttempts; attempt++) { - if (maxUses !== null && current >= maxUses) return null + if (maxUses !== null && current >= maxUses) return { ok: false, reason: 'cap_reached' } const next = current + 1 const { data } = await admin .from('space_invites') @@ -33,15 +40,16 @@ export async function claimInviteUse( .eq('id', inviteId) .eq('uses_count', current) // compare-and-swap guard .select('id') - if (data && data.length > 0) return next + if (data && data.length > 0) return { ok: true, count: next } // Lost the race (or the row changed): re-read and retry while under cap. const { data: fresh } = await admin .from('space_invites') .select('uses_count') .eq('id', inviteId) .maybeSingle() - if (!fresh) return null + if (!fresh) return { ok: false, reason: 'cap_reached' } current = (fresh as { uses_count: number }).uses_count } - return null + // Exhausted the attempt budget while still under cap: pure contention. + return { ok: false, reason: 'contention' } }