fix(vouchers): reserve claims atomically before reward dispatch

docs/superpowers/evidence/2026-09-05-housekeeping-backend-review.md

@@ -27,7 +27,7 @@ The current UI route matrix cannot establish operation-level parity.
 
 | Area | Evidence / required follow-up |
 | --- | --- |
-| Voucher redemption | `src/actions/voucher.ts` reads eligibility, inserts used-row, delivers currency, then updates usage in separate operations. No atomic cap reservation. A failed reward can leave a consumed voucher. |
+| Voucher redemption | Claim reservation now locks the voucher and duplicate claim, commits usage/cap with an audit intent, then dispatches the reward. Failed or uncertain dispatch retains the reservation and returns the audit reference. Automatic recovery of reward increments remains intentionally unavailable without emulator acknowledgment/idempotency. |
 | Currency delivery | `src/lib/services/send-currency.ts` uses RCON followed by database fallback; socket dispatch is not emulator acknowledgment. Do not invent exactly-once guarantees or blindly replay increments. |
 | Reason enforcement | Reason propagation is fixed for the named paths, but operation-level required-reason policies and denied/failure auditing still need a complete cross-entrypoint inventory. |
 | Functional parity | Compare each query and mutation in Content, Economy, Hotel, People, System and Operations with retained legacy API/actions. A registered handler is not proof of complete functionality. |

docs/superpowers/evidence/2026-09-05-voucher-reservation.md

@@ -0,0 +1,22 @@
+# Voucher claim reservation
+
+The public redemption action now serializes claims on the selected voucher row.
+It validates amount, capacity and expiration, locks the duplicate-claim read,
+inserts the used row, increments usage and stores the audit intent in one transaction.
+No reward dispatch occurs until the transaction resolves successfully. A commit
+acknowledgment failure prevents dispatch and returns a correlation reference.
+
+The reward transport remains the existing sendCurrency implementation. A false
+or thrown result is unconfirmed, not proof that no increment occurred. Such a
+claim stays consumed and receives a partial audit outcome. It must not be blindly
+replayed or refunded; staff can inspect the correlated intent and final outcome.
+A completion-audit failure after successful dispatch does not report failed delivery.
+
+This does not guarantee exactly-once emulator processing, introduce automatic
+reward recovery, or claim that database reservation and external dispatch are
+one distributed transaction. No migration or live data change is performed.
+
+Tests exercise the real action and generated Drizzle SQL with controlled database,
+session, audit and currency transports. They prove boundary ordering and error
+mapping, not actual multi-connection MariaDB locking or emulator acceptance.
+The initial regression run had nine expected failures before implementation.

src/actions/voucher.test.ts

@@ -0,0 +1,291 @@
+import { drizzle } from "drizzle-orm/mysql-proxy";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const state = vi.hoisted(() => ({
+	userId: "42",
+	count: 0,
+	amount: 50,
+	missing: false,
+	expired: false,
+	max: 1,
+	already: false,
+	failUpdate: false,
+	failCommit: false,
+	queries: [] as Array<{ sql: string; tx: boolean; params: unknown[] }>,
+	events: [] as string[],
+	inTransaction: false,
+	connection: undefined as unknown,
+	deliver: vi.fn(),
+	audit: vi.fn(),
+}));
+vi.mock("@/lib/auth", () => ({
+	auth: async () => ({ user: { id: state.userId } }),
+}));
+vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
+vi.mock("@/lib/rate-limit", () => ({
+	clientIp: async () => "127.0.0.1",
+	rateLimit: async () => ({ ok: true }),
+}));
+vi.mock("@/lib/services/rcon", () => ({ rcon: {} }));
+vi.mock("@/lib/services/send-currency", () => ({
+	currencyDb: {},
+	sendCurrency: state.deliver,
+}));
+vi.mock("@/lib/services/audit", () => ({ logAudit: state.audit }));
+vi.mock("@/lib/db", async () => {
+	const forward =
+		(method: string) =>
+		(...args: unknown[]) =>
+			Reflect.apply(
+				Reflect.get(state.connection as object, method),
+				state.connection,
+				args,
+			);
+	return {
+		...(await import("@/db/schema")),
+		db: {
+			select: forward("select"),
+			insert: forward("insert"),
+			update: forward("update"),
+			transaction: async (run: (tx: unknown) => Promise<unknown>) => {
+				state.events.push("begin");
+				state.inTransaction = true;
+				try {
+					const result = await run(state.connection);
+					if (state.failCommit)
+						throw new Error("commit acknowledgement unavailable");
+					state.events.push("commit");
+					return result;
+				} catch (error) {
+					state.events.push("rollback");
+					throw error;
+				} finally {
+					state.inTransaction = false;
+				}
+			},
+		},
+	};
+});
+
+import { redeem } from "./voucher";
+
+function input() {
+	const form = new FormData();
+	form.set("code", "PROMO");
+	return form;
+}
+beforeEach(() => {
+	state.userId = "42";
+	state.count = 0;
+	state.amount = 50;
+	state.missing = false;
+	state.expired = false;
+	state.max = 1;
+	state.already = false;
+	state.failUpdate = false;
+	state.failCommit = false;
+	state.queries = [];
+	state.events = [];
+	state.inTransaction = false;
+	state.audit.mockReset();
+	state.deliver.mockReset();
+	state.audit.mockImplementation(async () => {
+		state.events.push("audit");
+	});
+	state.deliver.mockImplementation(async () => {
+		state.events.push("deliver");
+		return true;
+	});
+	state.connection = drizzle(async (sql, params, method) => {
+		state.queries.push({ sql, params, tx: state.inTransaction });
+		if (sql.startsWith("update") && state.failUpdate)
+			throw new Error("storage unavailable");
+		if (method !== "all") return { rows: [{ affectedRows: 1, insertId: 9 }] };
+		if (sql.includes("website_used_shop_vouchers"))
+			return { rows: state.already ? [[9]] : [] };
+		return {
+			rows: state.missing
+				? []
+				: [
+						[
+							"7",
+							state.amount,
+							state.max,
+							state.count,
+							state.expired ? "2000-01-01 00:00:00" : null,
+						],
+					],
+		};
+	});
+});
+
+describe("Voucher redemption integrity", () => {
+	it("never dispatches after an uncertain commit acknowledgement", async () => {
+		state.failCommit = true;
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toContain(
+			state.audit.mock.calls[0]?.[0].correlationId,
+		);
+		expect(state.deliver).not.toHaveBeenCalled();
+	});
+	it("locks duplicate claims using the authenticated user and selected voucher", async () => {
+		await redeem(null, input());
+		const query = state.queries.find(
+			(item) =>
+				item.sql.startsWith("select") &&
+				item.sql.includes("website_used_shop_vouchers"),
+		);
+		expect(query?.sql).toMatch(/for update$/);
+		expect(query?.params).toEqual([42, 7n, 1]);
+	});
+	it.each(["missing", "expired"] as const)(
+		"rejects a %s voucher without changing storage",
+		async (kind) => {
+			state[kind] = true;
+			expect((await redeem(null, input()))?.ok).toBe(false);
+			expect(state.queries).toHaveLength(1);
+			expect(state.deliver).not.toHaveBeenCalled();
+		},
+	);
+	it.each([0, -1, 0.5])(
+		"rejects invalid reward amount %s before reserving",
+		async (amount) => {
+			state.amount = amount;
+			expect((await redeem(null, input()))?.ok).toBe(false);
+			expect(state.queries).toHaveLength(1);
+			expect(state.deliver).not.toHaveBeenCalled();
+		},
+	);
+	it("increments the reserved counter and expires the final claim before delivery", async () => {
+		state.count = 4;
+		state.max = 5;
+		await redeem(null, input());
+		const update = state.queries.find((query) =>
+			query.sql.startsWith("update"),
+		);
+		expect(update?.sql).toContain("`use_count` = ?");
+		expect(update?.sql).toContain("`expires_at` = ?");
+		expect(update?.params[0]).toBe(5);
+		expect(state.audit.mock.calls[0]?.[0]).toMatchObject({
+			before: { useCount: 4 },
+			after: { useCount: 5, amount: 50 },
+		});
+	});
+	it("does not expire a voucher with remaining capacity", async () => {
+		state.max = 5;
+		await redeem(null, input());
+		const update = state.queries.find((query) =>
+			query.sql.startsWith("update"),
+		);
+		expect(update?.sql).not.toContain("`expires_at` =");
+	});
+	it("treats false delivery as unconfirmed and keeps the same audit reference", async () => {
+		state.deliver.mockResolvedValueOnce(false);
+		const result = await redeem(null, input());
+		const intent = state.audit.mock.calls[0]?.[0];
+		const outcome = state.audit.mock.calls[1]?.[0];
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toContain(intent.correlationId);
+		expect(outcome).toMatchObject({
+			correlationId: intent.correlationId,
+			outcome: "partial",
+			after: { reward: "unconfirmed" },
+		});
+	});
+	it("preserves the pending intent if delivery and completion audit both fail", async () => {
+		state.deliver.mockRejectedValueOnce(new Error("uncertain delivery"));
+		state.audit
+			.mockResolvedValueOnce(undefined)
+			.mockRejectedValueOnce(new Error("audit unavailable"));
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toContain(
+			state.audit.mock.calls[0]?.[0].correlationId,
+		);
+		expect(state.deliver).toHaveBeenCalledTimes(1);
+	});
+	it("never uses a submitted user id for the reward", async () => {
+		const form = input();
+		form.set("userId", "999");
+		await redeem(null, form);
+		expect(state.deliver).toHaveBeenCalledWith(
+			expect.anything(),
+			42,
+			"credits",
+			50,
+		);
+	});
+	it("reserves usage and audit under lock before sending currency", async () => {
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(true);
+		expect(state.queries[0]?.sql).toMatch(/for update$/);
+		expect(state.queries.every((query) => query.tx)).toBe(true);
+		expect(state.events).toEqual([
+			"begin",
+			"audit",
+			"commit",
+			"deliver",
+			"audit",
+		]);
+		expect(state.audit).toHaveBeenCalledWith(
+			expect.objectContaining({ outcome: "intent", domain: "economy" }),
+			state.connection,
+		);
+	});
+	it("rejects an exhausted unexpired voucher before awarding", async () => {
+		state.count = 1;
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+		expect(state.queries.some((query) => query.sql.startsWith("insert"))).toBe(
+			false,
+		);
+	});
+	it("does not deliver if reservation persistence fails", async () => {
+		state.failUpdate = true;
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+		expect(state.events).toContain("rollback");
+	});
+	it("blocks delivery if the durable intent cannot be stored", async () => {
+		state.audit.mockRejectedValueOnce(new Error("audit unavailable"));
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+		expect(state.events).toContain("rollback");
+	});
+	it("provides a correlated partial result for failed reward delivery", async () => {
+		state.deliver.mockRejectedValueOnce(new Error("transport unavailable"));
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toMatch(/reference:/i);
+		expect(state.audit).toHaveBeenLastCalledWith(
+			expect.objectContaining({
+				outcome: "partial",
+				after: expect.objectContaining({ reward: "unconfirmed" }),
+			}),
+		);
+		expect(state.deliver).toHaveBeenCalledTimes(1);
+	});
+	it("does not misreport delivered currency when completion auditing fails", async () => {
+		state.audit
+			.mockResolvedValueOnce(undefined)
+			.mockRejectedValueOnce(new Error("audit unavailable"));
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(true);
+		expect(result?.message).toMatch(/reference:/i);
+		expect(state.deliver).toHaveBeenCalledTimes(1);
+	});
+	it("rejects a previous claim without delivering again", async () => {
+		state.already = true;
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+	});
+	it.each(["0", "-1", "1.5"])(
+		"rejects invalid session id %s",
+		async (userId) => {
+			state.userId = userId;
+			expect((await redeem(null, input()))?.ok).toBe(false);
+			expect(state.queries).toEqual([]);
+		},
+	);
+});

src/actions/voucher.ts

@@ -1,30 +1,18 @@
 "use server";
 
-import { and, eq, sql } from "drizzle-orm";
+import { randomUUID } from "node:crypto";
+import { and, eq } from "drizzle-orm";
 import { revalidatePath } from "next/cache";
 import { auth } from "@/lib/auth";
 import { db, WebsiteShopVouchers, WebsiteUsedShopVouchers } from "@/lib/db";
 import { clientIp, rateLimit } from "@/lib/rate-limit";
+import { type AuditEntry, logAudit } from "@/lib/services/audit";
 import { rcon } from "@/lib/services/rcon";
 import { currencyDb, sendCurrency } from "@/lib/services/send-currency";
 
-/** Feedback returned to the <RedeemForm/> client component via useActionState. */
 export type RedeemState = { ok: boolean; message: string } | null;
 
-/**
+/** Reserve each claim atomically before attempting a non-replayable reward. */
- * Redeem a shop voucher for the SIGNED-IN user. Faithful to AtomCMS's
- * ShopVoucherController:
- *   - the user id is re-read from the session (auth()), NEVER from FormData,
- *     so a crafted form cannot redeem on another account;
- *   - a code that is missing or expired is rejected;
- *   - each voucher may be redeemed once per user (website_used_shop_vouchers);
- *   - on success the reward `amount` is granted, the used-row is inserted,
- *     use_count is incremented, and the voucher is expired once max_uses is hit.
- *
- * The reward is delivered through sendCurrency({ rcon, db: currencyDb }); the
- * voucher schema carries a single `amount`, granted as the website credits
- * wallet currency.
- */
 export async function redeem(
 	_prev: RedeemState,
 	formData: FormData,
@@ -33,15 +21,13 @@ export async function redeem(
 	if (!session?.user?.id) {
 		return { ok: false, message: "You must be signed in to redeem a voucher." };
 	}
-
 	const userId = Number(session.user.id);
-	if (!Number.isFinite(userId)) {
+	if (!Number.isSafeInteger(userId) || userId <= 0) {
 		return {
 			ok: false,
 			message: "Your session is invalid. Please sign in again.",
 		};
 	}
-
 	await clientIp();
 	if (!(await rateLimit(`voucher-redeem:${userId}`, 5, 60_000)).ok) {
 		return {
@@ -49,132 +35,140 @@ export async function redeem(
 			message: "You're redeeming too fast. Please wait a moment and try again.",
 		};
 	}
-
+	const rawCode = formData.get("code");
-	const code = String(formData.get("code") ?? "")
+	const code =
-		.normalize("NFC")
+		typeof rawCode === "string" ? rawCode.normalize("NFC").trim() : "";
-		.trim();
+	if (!code || code.length > 255) {
-	if (!code) {
+		return { ok: false, message: "Please enter a valid voucher code." };
-		return { ok: false, message: "Please enter a voucher code." };
 	}
 
-	// Look up the code (website_shop_vouchers.code is unique).
+	const correlationId = randomUUID();
-	let voucher: {
+	let claim: { amount: number; audit: AuditEntry } | { rejection: string };
-		id: bigint;
-		amount: number;
-		maxUses: number;
-		useCount: number;
-		expiresAt: Date | null;
-	} | null;
 	try {
-		const [row] = await db
+		claim = await db.transaction(async (transaction) => {
-			.select({
+			const [voucher] = await transaction
-				id: WebsiteShopVouchers.id,
+				.select({
-				amount: WebsiteShopVouchers.amount,
+					id: WebsiteShopVouchers.id,
-				maxUses: WebsiteShopVouchers.maxUses,
+					amount: WebsiteShopVouchers.amount,
-				useCount: WebsiteShopVouchers.useCount,
+					maxUses: WebsiteShopVouchers.maxUses,
-				expiresAt: WebsiteShopVouchers.expiresAt,
+					useCount: WebsiteShopVouchers.useCount,
-			})
+					expiresAt: WebsiteShopVouchers.expiresAt,
-			.from(WebsiteShopVouchers)
+				})
-			.where(eq(WebsiteShopVouchers.code, code))
+				.from(WebsiteShopVouchers)
-			.limit(1);
+				.where(eq(WebsiteShopVouchers.code, code))
-		voucher = row ?? null;
+				.limit(1)
-	} catch {
+				.for("update");
-		return {
+			const now = new Date();
-			ok: false,
+			if (
-			message: "We couldn't reach the server. Please try again.",
+				!voucher ||
-		};
+				(voucher.expiresAt && voucher.expiresAt.getTime() <= now.getTime()) ||
-	}
+				!Number.isSafeInteger(voucher.maxUses) ||
-
+				voucher.maxUses <= 0 ||
-	// Not found OR already expired -> generic "no active voucher" (matches AtomCMS).
+				!Number.isSafeInteger(voucher.useCount) ||
-	if (
+				voucher.useCount < 0 ||
-		!voucher ||
+				voucher.useCount >= voucher.maxUses ||
-		(voucher.expiresAt && voucher.expiresAt.getTime() <= Date.now())
+				!Number.isSafeInteger(voucher.amount) ||
-	) {
+				voucher.amount <= 0
-		return {
+			) {
-			ok: false,
+				return {
-			message: "No active voucher with the given code was found.",
+					rejection: "No active voucher with the given code was found.",
-		};
+				};
-	}
+			}
-
+			// A locking read avoids a stale repeatable-read snapshot after waiting
-	// One redemption per user.
+			// for another claim on the same voucher. All claims lock voucher first.
-	try {
+			const [already] = await transaction
-		const [already] = await db
+				.select({ id: WebsiteUsedShopVouchers.id })
-			.select({ id: WebsiteUsedShopVouchers.id })
+				.from(WebsiteUsedShopVouchers)
-			.from(WebsiteUsedShopVouchers)
+				.where(
-			.where(
+					and(
-				and(
+						eq(WebsiteUsedShopVouchers.userId, userId),
-					eq(WebsiteUsedShopVouchers.userId, userId),
+						eq(WebsiteUsedShopVouchers.voucherId, voucher.id),
-					eq(WebsiteUsedShopVouchers.voucherId, voucher.id),
+					),
-				),
+				)
-			)
+				.limit(1)
-			.limit(1);
+				.for("update");
-		if (already) {
+			if (already)
-			return { ok: false, message: "You can only use each shop voucher once." };
+				return { rejection: "You can only use each shop voucher once." };
-		}
+			await transaction.insert(WebsiteUsedShopVouchers).values({
-	} catch {
+				userId,
-		return {
+				voucherId: voucher.id,
-			ok: false,
+			});
-			message: "We couldn't reach the server. Please try again.",
+			const useCount = voucher.useCount + 1;
-		};
+			await transaction
-	}
+				.update(WebsiteShopVouchers)
-
+				.set({
-	// Record the redemption first so a successful grant can never be double-claimed.
+					useCount,
-	try {
+					...(useCount >= voucher.maxUses ? { expiresAt: now } : {}),
-		await db.insert(WebsiteUsedShopVouchers).values({
+					updatedAt: now,
-			userId,
+				})
-			voucherId: voucher.id,
+				.where(eq(WebsiteShopVouchers.id, voucher.id));
+			const audit: AuditEntry = {
+				userId,
+				action: "economy.voucher.redeem",
+				target: "ShopVoucher",
+				domain: "economy",
+				correlationId,
+				before: {
+					voucherId: voucher.id.toString(),
+					useCount: voucher.useCount,
+				},
+				after: {
+					voucherId: voucher.id.toString(),
+					userId,
+					amount: voucher.amount,
+					useCount,
+					reward: "pending",
+				},
+			};
+			await logAudit({ ...audit, outcome: "intent" }, transaction);
+			return { amount: voucher.amount, audit };
 		});
 	} catch {
-		// Most likely a race (another tab redeemed it) — treat as already used.
+		// A commit acknowledgement can itself be uncertain. Never send a reward
-		return { ok: false, message: "You can only use each shop voucher once." };
+		// after any reservation error; the durable intent supports investigation.
+		return {
+			ok: false,
+			message: `We could not confirm your voucher reservation. Contact staff if needed. Reference: ${correlationId}`,
+		};
 	}
+	if ("rejection" in claim) return { ok: false, message: claim.rejection };
 
-	// Grant the reward. The voucher carries a single amount, delivered as credits.
+	let delivered = false;
 	try {
-		await sendCurrency(
+		delivered = await sendCurrency(
 			{ rcon, db: currencyDb },
 			userId,
 			"credits",
-			voucher.amount,
+			claim.amount,
 		);
 	} catch {
-		// sendCurrency already falls back to a direct DB write; if it still throws,
+		// An increment may already have been dispatched. Keep the reservation,
-		// the used-row stands and the balance simply wasn't credited — surface that.
+		// expose its reference, and do not automatically replay or refund it.
+	}
+	let auditSaved = true;
+	try {
+		await logAudit({
+			...claim.audit,
+			outcome: delivered ? "success" : "partial",
+			after: {
+				...claim.audit.after,
+				reward: delivered ? "dispatched" : "unconfirmed",
+			},
+		});
+	} catch {
+		auditSaved = false;
+	}
+	revalidatePath("/redeem");
+	if (!delivered) {
 		return {
 			ok: false,
-			message:
+			message: `Your voucher is reserved, but reward delivery could not be confirmed. Contact staff. Reference: ${correlationId}`,
-				"Your voucher was accepted but the reward could not be delivered. Contact staff.",
 		};
 	}
-
-	// Bump use_count and expire the voucher once the cap is reached.
-	try {
-		await db
-			.update(WebsiteShopVouchers)
-			.set({ useCount: sql`${WebsiteShopVouchers.useCount} + 1` })
-			.where(eq(WebsiteShopVouchers.id, voucher.id));
-		const [updated] = await db
-			.select({
-				maxUses: WebsiteShopVouchers.maxUses,
-				useCount: WebsiteShopVouchers.useCount,
-			})
-			.from(WebsiteShopVouchers)
-			.where(eq(WebsiteShopVouchers.id, voucher.id))
-			.limit(1);
-		if (updated?.maxUses && updated.useCount >= updated.maxUses) {
-			await db
-				.update(WebsiteShopVouchers)
-				.set({ expiresAt: new Date() })
-				.where(eq(WebsiteShopVouchers.id, voucher.id));
-		}
-	} catch {
-		// Reward already delivered; the counter bump is best-effort.
-	}
-
-	revalidatePath("/redeem");
-
 	return {
 		ok: true,
-		message: `Success! Your balance has been increased by ${voucher.amount.toLocaleString()} credits.`,
+		message: auditSaved
+			? `Your voucher was accepted and the ${claim.amount.toLocaleString()} credit reward was sent.`
+			: `Your voucher reward was sent, but its audit could not be completed. Reference: ${correlationId}`,
 	};
 }