6 changed files with 318 additions and 10 deletions
@@ -0,0 +1,38 @@
+# Housekeeping backend review checkpoint
+
+This is an incremental backend review, not a completion or deployment claim.
+The current UI route matrix cannot establish operation-level parity.
+
+## Delivered blocks
+
+### Audit reasons and external outcomes (555bc75f)
+
+- Content and Economy preserve normalized reasons through the real service and production audit adapters.
+- Hotel preserves reasons through command, service and audit.
+- Successful Hotel RCON execution with unavailable completion auditing returns partial completion with external completed; it does not emit a false RCON failure.
+- Regressions were observed failing before implementation.
+- Full pre-push suite: 1,877 passed, 5 skipped. TypeScript and scoped Biome passed. Remote CI check passed.
+- Independent scoped review: no Critical or Important findings.
+
+### Commerce editing concurrency
+
+- ASE marketplace cancellation reads and checks the listing under a transaction-held row lock; inactive listings return CONFLICT.
+- Legacy marketplace cancellation includes the row lock, update and staff activity in one transaction. Audit exceptions propagate for rollback.
+- ASE and legacy voucher edits lock the record and reject caps below recorded usage. Legacy edits reject missing vouchers rather than reporting a successful no-op.
+- Four ASE and two legacy regressions failed before fixes; the expanded focused suite has 14 passing tests.
+- Tests execute real Drizzle SQL generation against controlled transport responses. They do not simulate MariaDB locking or prove live multi-connection behavior.
+- Independent scoped review: no Critical or Important findings.
+
+## Open backend work
+
+| 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. |
+| 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. |
+| Commerce audit completeness | Review full before/after snapshots, voucher code-edit parity, and canonical audit coverage of legacy voucher actions. |
+| Validation and references | Review bounded numeric/string inputs, missing targets, foreign references, bulk all-or-nothing behavior and duplicate conflicts per operation. |
+| Live acceptance | Local DB and RCON refuse connections. This does not prevent source implementation; it prevents live integration claims. |
+
+Keep PR 53 draft. Preserve legacy pages, local untracked files, production routing and the existing database contents.
@@ -5,7 +5,7 @@ import { revalidatePath } from "next/cache";
 import { requirePermissionRateLimited } from "@/lib/admin/guard";
 import { db, MarketplaceItems } from "@/lib/db";
 import { PERMS } from "@/lib/permissions";
-import { logStaffActivity } from "@/lib/services/staff-activity";
+import { logStaffActivityInTransaction } from "@/lib/services/staff-activity";

 /** Cancel an active marketplace listing (state 1 → 0). */
 export async function cancelMarketplaceListing(
@@ -13,9 +13,10 @@ export async function cancelMarketplaceListing(
 ): Promise<void> {
 	const staff = await requirePermissionRateLimited(PERMS.SHOP_EDIT);
 	const id = Number(formData.get("id"));
-	if (!(id > 0)) return;
+	if (!Number.isSafeInteger(id) || id <= 0) return;

-	const [listing] = await db
+	await db.transaction(async (transaction) => {
+		const [listing] = await transaction
 			.select({
 				id: MarketplaceItems.id,
 				state: MarketplaceItems.state,
@@ -25,20 +26,25 @@ export async function cancelMarketplaceListing(
 			})
 			.from(MarketplaceItems)
 			.where(eq(MarketplaceItems.id, id))
-		.limit(1);
+			.limit(1)
+			.for("update");
 		if (listing?.state !== 1) return;

-	await db
+		await transaction
 			.update(MarketplaceItems)
 			.set({ state: 0 })
 			.where(eq(MarketplaceItems.id, id));

-	await logStaffActivity({
+		await logStaffActivityInTransaction(
+			{
 				staffId: staff.id,
 				action: "marketplace_cancel",
 				description: `Cancelled marketplace listing #${id} (item ${listing.itemId}, user ${listing.userId}, price ${listing.price})`,
 				targetType: "marketplace",
 				targetId: id,
+			},
+			transaction,
+		);
 	});
 	revalidatePath("/admin/marketplace");
 }
@@ -119,7 +119,18 @@ export async function updateVoucher(input: {
 	}

 	try {
-		await db
+		const result = await db.transaction(async (transaction) => {
+			const [existing] = await transaction
+				.select({ useCount: WebsiteShopVouchers.useCount })
+				.from(WebsiteShopVouchers)
+				.where(eq(WebsiteShopVouchers.id, id))
+				.limit(1)
+				.for("update");
+			if (!existing) return actionError("Voucher not found");
+			if (maxUses < existing.useCount) {
+				return actionError("Maximum uses cannot be lower than recorded usage");
+			}
+			await transaction
 				.update(WebsiteShopVouchers)
 				.set({
 					code,
@@ -129,6 +140,9 @@ export async function updateVoucher(input: {
 					updatedAt: new Date(),
 				})
 				.where(eq(WebsiteShopVouchers.id, id));
+			return actionOk();
+		});
+		if (!result.ok) return result;
 		revalidatePath("/admin/vouchers");
 		return actionOk();
 	} catch (error) {
@@ -0,0 +1,87 @@
+import { drizzle } from "drizzle-orm/mysql-proxy";
+import { describe, expect, it, vi } from "vitest";
+import { executeEconomyDatabaseMutation } from "./mutation-runtime-commerce";
+
+vi.mock("@/lib/db", async () => ({ ...(await import("@/db/schema")), db: {} }));
+
+const context = {
+	capability: {
+		actor: { id: 42, username: "operator", rank: 7 },
+		isSuperAdmin: false,
+		has: () => true,
+		hasAny: () => true,
+		hasAll: () => true,
+	},
+	correlationId: "commerce-concurrency",
+	legacy: false,
+};
+
+function connection(rows: unknown[][]) {
+	const statements: Array<{ sql: string; params: unknown[] }> = [];
+	const database = drizzle(async (sql, params, method) => {
+		statements.push({ sql, params });
+		return {
+			rows: method === "all" ? rows : [{ affectedRows: 1, insertId: 0 }],
+		};
+	});
+	return { database, statements };
+}
+
+describe("Economy commerce concurrency", () => {
+	it("locks an active marketplace listing before cancelling it", async () => {
+		const { database, statements } = connection([[7, 1, 42, 99, 50]]);
+		const snapshot = await executeEconomyDatabaseMutation(
+			"marketplace.cancel",
+			{ id: 7 },
+			context,
+			database,
+		);
+		expect(statements[0]?.sql).toMatch(/for update$/);
+		expect(statements[0]?.params).toEqual([7, 1]);
+		expect(statements[1]?.sql).toMatch(/^update/);
+		expect(snapshot).toMatchObject({
+			before: { state: 1 },
+			after: { state: 0 },
+		});
+	});
+
+	it("reports a conflict without writing when a listing is already sold", async () => {
+		const { database, statements } = connection([[7, 2, 42, 99, 50]]);
+		await expect(
+			executeEconomyDatabaseMutation(
+				"marketplace.cancel",
+				{ id: 7 },
+				context,
+				database,
+			),
+		).rejects.toMatchObject({ code: "CONFLICT" });
+		expect(statements).toHaveLength(1);
+	});
+
+	it("locks voucher state before rejecting a cap below recorded usage", async () => {
+		const { database, statements } = connection([["7", 50, 10, 6]]);
+		await expect(
+			executeEconomyDatabaseMutation(
+				"voucher.change",
+				{ action: "update", id: "7", maxUses: 5 },
+				context,
+				database,
+			),
+		).rejects.toMatchObject({ code: "CONFLICT" });
+		expect(statements[0]?.sql).toMatch(/for update$/);
+		expect(statements).toHaveLength(1);
+	});
+
+	it("accepts a cap equal to recorded usage without resetting the counter", async () => {
+		const { database, statements } = connection([["7", 50, 10, 6]]);
+		const snapshot = await executeEconomyDatabaseMutation(
+			"voucher.change",
+			{ action: "update", id: "7", maxUses: 6 },
+			context,
+			database,
+		);
+		expect(statements[0]?.sql).toMatch(/for update$/);
+		expect(statements[1]?.sql).not.toContain("`use_count` =");
+		expect(snapshot).toMatchObject({ after: { maxUses: 6, useCount: 6 } });
+	});
+});
@@ -30,6 +30,7 @@ import type {
 	EconomyMutationOperation,
 	EconomyMutationSnapshot,
 } from "./mutations";
+import { EconomyMutationFailure } from "./mutations";

 type EconomyDatabase = typeof db;

@@ -153,9 +154,15 @@ async function marketplaceCancel(
 		})
 		.from(MarketplaceItems)
 		.where(eq(MarketplaceItems.id, id))
-		.limit(1);
+		.limit(1)
+		.for("update");
 	if (!listing) throw notFound();
-	if (listing.state !== 1) throw validation({ id: ["listing is not active"] });
+	if (listing.state !== 1) {
+		throw new EconomyMutationFailure(
+			"CONFLICT",
+			"errors.housekeeping.conflict",
+		);
+	}
 	await connection
 		.update(MarketplaceItems)
 		.set({ state: 0 })
@@ -216,7 +223,8 @@ async function voucherChange(
 		})
 		.from(WebsiteShopVouchers)
 		.where(eq(WebsiteShopVouchers.id, id))
-		.limit(1);
+		.limit(1)
+		.for("update");
 	if (!existing) throw notFound();
 	const before = {
 		id: id.toString(),
@@ -233,6 +241,12 @@ async function voucherChange(
 	const values: Partial<typeof WebsiteShopVouchers.$inferInsert> = {};
 	if (hasOwn(data, "amount")) values.amount = positiveInteger(data.amount);
 	if (hasOwn(data, "maxUses")) values.maxUses = positiveInteger(data.maxUses);
+	if (values.maxUses !== undefined && values.maxUses < existing.useCount) {
+		throw new EconomyMutationFailure(
+			"CONFLICT",
+			"errors.housekeeping.conflict",
+		);
+	}
 	if (hasOwn(data, "expiresAt")) {
 		const parsed = data.expiresAt
 			? new Date(text(data.expiresAt, 64, true))