diff --git a/packages/server/src/utils/inviteService.test.ts b/packages/server/src/utils/inviteService.test.ts index 569a98b6..b121bf16 100644 --- a/packages/server/src/utils/inviteService.test.ts +++ b/packages/server/src/utils/inviteService.test.ts @@ -531,13 +531,13 @@ describe('redeemInvite', () => { expect(redemptions).toHaveLength(0); }); - it('aborts transaction (no usedCount increment, no redemption row) if insertUser throws', () => { + it('does not bump usedCount or write a redemption when insertUser throws synchronously', () => { const adminId = seedAdmin(); const inv = createInvite({ name: 'a', maxUses: 5, expiresAt: null }, adminId); expect(() => redeemInvite(inv.token, () => { - throw new Error('username collision'); - })).toThrow('username collision'); + throw new Error('username taken'); + })).toThrow('username taken'); // usedCount unchanged const row = testDb.select().from(schema.inviteLinks).where(eq(schema.inviteLinks.id, inv.id)).get(); @@ -548,6 +548,34 @@ describe('redeemInvite', () => { expect(redemptions).toHaveLength(0); }); + it('rolls back insertUser writes when a later step throws (true SQLite ROLLBACK)', () => { + const adminId = seedAdmin(); + const inv = createInvite({ name: 'a', maxUses: 5, expiresAt: null }, adminId); + const newUserId = 'rollback-user-1'; + + // The callback writes a real users row, then throws AFTER the write. + // If the txn truly rolls back, the users row must not exist after the call. + expect(() => redeemInvite(inv.token, () => { + testDb.insert(schema.users).values({ + id: newUserId, + username: 'will-be-rolled-back', + passwordHash: 'x', + createdAt: Date.now(), + }).run(); + throw new Error('post-insert failure'); + })).toThrow('post-insert failure'); + + // Proves SQLite ROLLBACK reverted the user insert AND the would-be usedCount bump + const u = testDb.select().from(schema.users).where(eq(schema.users.id, newUserId)).get(); + expect(u).toBeUndefined(); + + const after = testDb.select().from(schema.inviteLinks).where(eq(schema.inviteLinks.id, inv.id)).get(); + expect(after?.usedCount).toBe(0); + + const redemptions = testDb.select().from(schema.inviteRedemptions).where(eq(schema.inviteRedemptions.inviteId, inv.id)).all(); + expect(redemptions).toHaveLength(0); + }); + it('throws InviteUnavailableError when token not found', () => { expect(() => redeemInvite('aaaaaaaaaaaaaaaaaaaaaa', () => ({ id: 'x', username: 'x' }))).toThrow(InviteUnavailableError); }); diff --git a/packages/server/src/utils/inviteService.ts b/packages/server/src/utils/inviteService.ts index 3ef0541c..6130a800 100644 --- a/packages/server/src/utils/inviteService.ts +++ b/packages/server/src/utils/inviteService.ts @@ -468,14 +468,23 @@ export function reinstateInvite(id: string, req: ReinstateInviteRequest): Reinst }); } +/** + * Discriminant union of reasons an invite cannot be redeemed. Surfaced as a + * typed public field on `InviteUnavailableError` so the HTTP register route + * can switch on it to produce user-facing copy without parsing the message + * string. Mirrors the non-active subset of `InviteStatus` plus `'not found'` + * for the missing-token case. + */ +export type InviteUnavailableReason = 'not found' | 'revoked' | 'expired' | 'exhausted'; + /** * Thrown when an invite cannot be redeemed because its current state forbids * it (token not found, revoked, expired, exhausted). Caller (HTTP register - * route) maps this to 403 Forbidden. The reason string is preserved in the - * message for debugging — the user-facing copy is constructed by the route. + * route) maps this to 403 Forbidden. The `reason` field is the structured + * discriminant; the message string is preserved for debugging/logging. */ export class InviteUnavailableError extends Error { - constructor(reason: string) { + constructor(public readonly reason: InviteUnavailableReason) { super(`Invite unavailable: ${reason}`); this.name = 'InviteUnavailableError'; } @@ -513,6 +522,9 @@ export function redeemInvite( if (!row) throw new InviteUnavailableError('not found'); const status = inviteStatus(row); if (status !== 'active') { + // 'active' is excluded by the guard above, so `status` is necessarily + // one of 'revoked' | 'expired' | 'exhausted' — all valid + // InviteUnavailableReason values. TS narrows the union here. throw new InviteUnavailableError(status); }