fix: converge legacy room mutations

src/actions/rooms.test.ts

@@ -0,0 +1,493 @@
+import { getTableName, type SQL } from "drizzle-orm";
+import { MySqlDialect } from "drizzle-orm/mysql-core";
+import { revalidatePath } from "next/cache";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { logAudit } from "@/lib/services/audit";
+import { rcon } from "@/lib/services/rcon";
+import { logStaffActivity } from "@/lib/services/staff-activity";
+import { notify } from "@/lib/services/webhook";
+
+type RecordedWrite = {
+	kind: "update" | "delete";
+	table: string;
+	values?: Record<string, unknown>;
+	condition: { sql: string; params: unknown[] };
+};
+
+const testState = vi.hoisted(() => ({
+	selectRows: [] as Array<Array<Record<string, unknown>>>,
+	readConditions: [] as Array<{ sql: string; params: unknown[] }>,
+	readLocks: [] as string[],
+	readOrders: [] as Array<{ sql: string; params: unknown[] }>,
+	committedWrites: [] as RecordedWrite[],
+	stagedWrites: null as RecordedWrite[] | null,
+	transactionCount: 0,
+	capabilityAllowed: true,
+	databaseWriteFailure: false,
+	auditFailure: false,
+	notifyFailure: false,
+	auditEntries: [] as Array<{
+		entry: Record<string, unknown>;
+		transactional: boolean;
+	}>,
+	rconSend: vi.fn(),
+	revalidate: vi.fn(),
+	notify: vi.fn(),
+	staffActivity: vi.fn(),
+}));
+
+const dialect = new MySqlDialect();
+
+function renderCondition(condition: SQL | undefined) {
+	if (!condition) return { sql: "", params: [] };
+	const query = dialect.sqlToQuery(condition);
+	return { sql: query.sql, params: [...query.params] };
+}
+
+function nextRows() {
+	return testState.selectRows.shift() ?? [];
+}
+
+function recordWrite(write: RecordedWrite) {
+	if (testState.databaseWriteFailure) throw new Error("database unavailable");
+	if (testState.stagedWrites) testState.stagedWrites.push(write);
+	else testState.committedWrites.push(write);
+}
+
+vi.mock("next/cache", () => ({ revalidatePath: testState.revalidate }));
+vi.mock("@/lib/safe-action-shared", () => ({
+	actionOk: (data?: Record<string, unknown>) => ({
+		ok: true,
+		data: data ?? {},
+	}),
+	handleActionError: () => ({ ok: false, error: "Internal server error" }),
+}));
+vi.mock("@/lib/permissions", async () => import("@/lib/permission-slugs"));
+vi.mock("@/lib/admin/guard", () => ({
+	requirePermission: async () => ({ id: 42, rank: 7, username: "operator" }),
+}));
+vi.mock("@/features/housekeeping/foundation/correlation", () => ({
+	createCorrelationId: () => "legacy-room-correlation",
+}));
+vi.mock("@/features/housekeeping/foundation/server-capability-context", () => ({
+	getHousekeepingCapabilityContext: async () => ({
+		actor: { id: 42, username: "operator", rank: 7 },
+		isSuperAdmin: false,
+		has: () => testState.capabilityAllowed,
+		hasAny: () => testState.capabilityAllowed,
+		hasAll: () => testState.capabilityAllowed,
+	}),
+}));
+vi.mock("@/lib/services/rcon", () => ({
+	rcon: { send: testState.rconSend },
+}));
+vi.mock("@/lib/services/audit", () => ({
+	logAudit: vi.fn(
+		async (entry: Record<string, unknown>, transaction?: unknown) => {
+			if (testState.auditFailure) throw new Error("audit unavailable");
+			testState.auditEntries.push({
+				entry,
+				transactional: transaction !== undefined,
+			});
+		},
+	),
+}));
+vi.mock("@/lib/services/webhook", () => ({
+	notify: testState.notify.mockImplementation(() => {
+		if (testState.notifyFailure) throw new Error("webhook unavailable");
+	}),
+}));
+vi.mock("@/lib/services/staff-activity", () => ({
+	logStaffActivity: testState.staffActivity,
+}));
+vi.mock("@/lib/db", async () => {
+	const schema = await import("@/db/schema");
+	const database = {
+		select: vi.fn(() => {
+			const query = {
+				from: vi.fn(() => query),
+				where: vi.fn((condition: SQL | undefined) => {
+					testState.readConditions.push(renderCondition(condition));
+					const rows = nextRows();
+					type QueryResult = Promise<typeof rows> & {
+						limit: (count: number) => QueryResult;
+						orderBy: (order: SQL) => QueryResult;
+						for: (lock: string) => QueryResult;
+					};
+					const result = Promise.resolve(rows) as QueryResult;
+					result.limit = vi.fn(() => result);
+					result.orderBy = vi.fn((order) => {
+						testState.readOrders.push(renderCondition(order));
+						return result;
+					});
+					result.for = vi.fn((lock) => {
+						testState.readLocks.push(lock);
+						return result;
+					});
+					return result;
+				}),
+			};
+			return query;
+		}),
+		update: vi.fn((table: object) => ({
+			set: (values: Record<string, unknown>) => ({
+				where: async (condition: SQL | undefined) => {
+					recordWrite({
+						kind: "update",
+						table: getTableName(table as never),
+						values,
+						condition: renderCondition(condition),
+					});
+					return [{ affectedRows: 1 }];
+				},
+			}),
+		})),
+		delete: vi.fn((table: object) => ({
+			where: async (condition: SQL | undefined) => {
+				recordWrite({
+					kind: "delete",
+					table: getTableName(table as never),
+					condition: renderCondition(condition),
+				});
+				return [{ affectedRows: 1 }];
+			},
+		})),
+		transaction: vi.fn(
+			async (run: (transaction: unknown) => Promise<unknown>) => {
+				testState.transactionCount += 1;
+				const staged: RecordedWrite[] = [];
+				testState.stagedWrites = staged;
+				try {
+					const result = await run(database);
+					testState.committedWrites.push(...staged);
+					return result;
+				} finally {
+					testState.stagedWrites = null;
+				}
+			},
+		),
+	};
+	return { ...schema, db: database };
+});
+
+import {
+	bulkDeleteRoomItems,
+	deleteRoom,
+	deleteRoomItem,
+	roomRconAction,
+	updateRoom,
+	updateRoomItem,
+} from "./rooms";
+
+function itemRow(overrides: Record<string, unknown> = {}) {
+	return {
+		id: 11,
+		userId: 90,
+		roomId: 7,
+		itemId: 1001,
+		wallPos: "",
+		x: 1,
+		y: 2,
+		z: 0,
+		rot: 0,
+		extraData: "",
+		wiredData: "",
+		limitedData: "0:0",
+		guildId: 0,
+		...overrides,
+	};
+}
+
+function roomRow(overrides: Record<string, unknown> = {}) {
+	return {
+		id: 7,
+		ownerId: 90,
+		ownerName: "owner",
+		name: "Test room",
+		description: "Before",
+		state: "open",
+		usersMax: 25,
+		...overrides,
+	};
+}
+
+function expectFailure(result: unknown, text: RegExp) {
+	expect(result).toMatchObject({
+		ok: false,
+		error: expect.stringMatching(text),
+	});
+	expect(result).toMatchObject({
+		error: expect.stringContaining("legacy-room-correlation"),
+	});
+}
+
+beforeEach(() => {
+	testState.selectRows = [];
+	testState.readConditions = [];
+	testState.readLocks = [];
+	testState.readOrders = [];
+	testState.committedWrites = [];
+	testState.stagedWrites = null;
+	testState.transactionCount = 0;
+	testState.capabilityAllowed = true;
+	testState.databaseWriteFailure = false;
+	testState.auditFailure = false;
+	testState.notifyFailure = false;
+	testState.auditEntries = [];
+	vi.clearAllMocks();
+	testState.rconSend.mockResolvedValue(true);
+});
+
+describe("legacy room actions through the Hotel mutation service", () => {
+	it.each([
+		[
+			"floor",
+			{ roomId: 7, itemId: 11, x: 4, y: 5, z: 1.25, rot: 6, extraData: "on" },
+			{ x: 4, y: 5, z: 1.25, rot: 6, extraData: "on" },
+		],
+		[
+			"wall",
+			{ roomId: 7, itemId: 11, wallPos: ":w=1,2 l=3,4 r", extraData: "red" },
+			{ wallPos: ":w=1,2 l=3,4 r", extraData: "red" },
+		],
+	])(
+		"updates a valid %s item transactionally",
+		async (_kind, payload, changes) => {
+			testState.selectRows = [[itemRow()]];
+			const result = await updateRoomItem(payload);
+			expect(result).toMatchObject({ ok: true });
+			expect(testState.transactionCount).toBe(1);
+			expect(testState.committedWrites).toEqual([
+				expect.objectContaining({
+					kind: "update",
+					table: "items",
+					values: changes,
+					condition: {
+						sql: expect.stringMatching(/`items`\.`id`.*`items`\.`room_id`/),
+						params: [11, 7],
+					},
+				}),
+			]);
+			expect(logAudit).toHaveBeenCalledWith(
+				expect.objectContaining({
+					action: "hotel.room-item.update",
+					correlationId: "legacy-room-correlation",
+					outcome: "success",
+				}),
+				expect.anything(),
+			);
+			expect(logStaffActivity).not.toHaveBeenCalled();
+			expect(testState.readLocks).toEqual(["update"]);
+			expect(revalidatePath).toHaveBeenCalledWith("/admin/rooms/7/furni");
+		},
+	);
+
+	it.each(["userId", "type", "ownerId"])(
+		"rejects the forbidden room-item field %s",
+		async (field) => {
+			const result = await updateRoomItem({
+				roomId: 7,
+				itemId: 11,
+				[field]: "forbidden",
+			});
+			expectFailure(result, /invalid/i);
+			expect(testState.committedWrites).toEqual([]);
+			expect(revalidatePath).not.toHaveBeenCalled();
+		},
+	);
+
+	it.each([
+		["update", () => updateRoom({ id: 0, name: "Invalid" })],
+		["delete", () => deleteRoom({ id: -1 })],
+		["item delete", () => deleteRoomItem({ roomId: 7, itemId: 0 })],
+	])("rejects an invalid id for %s", async (_name, action) => {
+		const result = await action();
+		expectFailure(result, /invalid/i);
+		expect(testState.committedWrites).toEqual([]);
+		expect(revalidatePath).not.toHaveBeenCalled();
+	});
+
+	it("rejects an item that is not in the supplied room", async () => {
+		testState.selectRows = [[]];
+		const result = await deleteRoomItem({ roomId: 7, itemId: 11 });
+		expectFailure(result, /not found/i);
+		expect(testState.readConditions).toEqual([
+			{
+				sql: expect.stringMatching(/`items`\.`id`.*`items`\.`room_id`/),
+				params: [11, 7],
+			},
+		]);
+		expect(testState.readLocks).toEqual(["update"]);
+		expect(testState.committedWrites).toEqual([]);
+		expect(revalidatePath).not.toHaveBeenCalled();
+	});
+
+	it("rejects an incomplete bulk selection atomically", async () => {
+		testState.selectRows = [[itemRow({ id: 11 })]];
+		const result = await bulkDeleteRoomItems({
+			roomId: 7,
+			itemIds: [11, 12],
+		});
+		expectFailure(result, /not found/i);
+		expect(testState.readConditions).toEqual([
+			{
+				sql: expect.stringMatching(/`items`\.`room_id`.*`items`\.`id` in/),
+				params: [7, 11, 12],
+			},
+		]);
+		expect(testState.readOrders).toEqual([
+			{ sql: expect.stringMatching(/`items`\.`id` asc/), params: [] },
+		]);
+		expect(testState.readLocks).toEqual(["update"]);
+		expect(testState.committedWrites).toEqual([]);
+		expect(revalidatePath).not.toHaveBeenCalled();
+	});
+
+	it("deduplicates a bounded bulk selection before deleting", async () => {
+		testState.selectRows = [[itemRow({ id: 11 }), itemRow({ id: 12 })]];
+		const result = await bulkDeleteRoomItems({
+			roomId: 7,
+			itemIds: [11, 11, 12],
+		});
+		expect(result).toMatchObject({ ok: true });
+		expect(testState.committedWrites).toEqual([
+			expect.objectContaining({
+				kind: "delete",
+				table: "items",
+				condition: {
+					sql: expect.stringMatching(/`items`\.`room_id`.*`items`\.`id` in/),
+					params: [7, 11, 12],
+				},
+			}),
+		]);
+		expect(testState.readOrders).toEqual([
+			{ sql: expect.stringMatching(/`items`\.`id` asc/), params: [] },
+		]);
+		expect(testState.readLocks).toEqual(["update"]);
+		expect(revalidatePath).toHaveBeenCalledWith("/admin/rooms/7/furni");
+	});
+
+	it("rejects a bulk selection over the service bound", async () => {
+		const result = await bulkDeleteRoomItems({
+			roomId: 7,
+			itemIds: Array.from({ length: 501 }, (_, index) => index + 1),
+		});
+		expectFailure(result, /invalid/i);
+		expect(testState.committedWrites).toEqual([]);
+	});
+
+	it("returns a denied capability as an actionable action failure", async () => {
+		testState.capabilityAllowed = false;
+		const result = await updateRoom({ id: 7, name: "Denied" });
+		expectFailure(result, /permission/i);
+		expect(testState.transactionCount).toBe(0);
+		expect(revalidatePath).not.toHaveBeenCalled();
+	});
+
+	it("rolls back a room update when its success audit fails", async () => {
+		testState.selectRows = [[roomRow()]];
+		testState.auditFailure = true;
+		const result = await updateRoom({ id: 7, name: "After" });
+		expectFailure(result, /could not be completed/i);
+		expect(testState.transactionCount).toBe(1);
+		expect(testState.committedWrites).toEqual([]);
+		expect(revalidatePath).not.toHaveBeenCalled();
+	});
+
+	it("does not audit or revalidate a failed database write", async () => {
+		testState.selectRows = [[roomRow()]];
+		testState.databaseWriteFailure = true;
+		const result = await updateRoom({ id: 7, description: "After" });
+		expectFailure(result, /could not be completed/i);
+		expect(testState.auditEntries).toEqual([]);
+		expect(testState.committedWrites).toEqual([]);
+		expect(revalidatePath).not.toHaveBeenCalled();
+	});
+
+	it("updates a room through the transactional Hotel path", async () => {
+		testState.selectRows = [[roomRow()]];
+		const result = await updateRoom({
+			id: 7,
+			name: "After",
+			description: "Changed",
+			state: "locked",
+			usersMax: 50,
+		});
+		expect(result).toMatchObject({ ok: true });
+		expect(testState.committedWrites).toEqual([
+			expect.objectContaining({
+				kind: "update",
+				table: "rooms",
+				values: {
+					name: "After",
+					description: "Changed",
+					state: "locked",
+					usersMax: 50,
+				},
+			}),
+		]);
+		expect(testState.readLocks).toEqual(["update"]);
+		expect(revalidatePath).toHaveBeenCalledWith("/admin/rooms/7");
+	});
+
+	it("commits room deletion even when its best-effort webhook throws", async () => {
+		testState.selectRows = [[roomRow()]];
+		testState.notifyFailure = true;
+		const result = await deleteRoom({ id: 7 });
+		expect(result).toMatchObject({ ok: true });
+		expect(testState.committedWrites).toEqual([
+			expect.objectContaining({ kind: "delete", table: "rooms" }),
+		]);
+		expect(notify).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "room_delete",
+				actor: "operator",
+				target: "Test room",
+			}),
+		);
+		expect(testState.readLocks).toEqual(["update"]);
+		expect(revalidatePath).toHaveBeenCalledWith("/admin/rooms");
+	});
+
+	it("rejects invalid runtime actions before RCON delivery", async () => {
+		const result = await roomRconAction({ roomId: 7, action: "explode" });
+		expectFailure(result, /invalid/i);
+		expect(rcon.send).not.toHaveBeenCalled();
+	});
+
+	it("returns failed RCON delivery instead of false success", async () => {
+		testState.rconSend.mockResolvedValue(false);
+		const result = await roomRconAction({ roomId: 7, action: "reload" });
+		expectFailure(result, /could not be completed/i);
+		expect(testState.auditEntries.map(({ entry }) => entry.outcome)).toEqual([
+			"intent",
+			"failure",
+		]);
+	});
+
+	it.each([
+		["reload", "reloadroom", { room_id: 7 }],
+		["kick", "kickall", { room_id: 7 }],
+		["lock", "updateroom", { room_id: 7, state: "locked" }],
+		["unlock", "updateroom", { room_id: 7, state: "open" }],
+	])(
+		"delivers the valid %s runtime action",
+		async (action, command, payload) => {
+			const result = await roomRconAction({ roomId: 7, action });
+			expect(result).toMatchObject({ ok: true });
+			expect(rcon.send).toHaveBeenCalledWith(command, payload);
+			expect(testState.auditEntries.map(({ entry }) => entry.outcome)).toEqual([
+				"intent",
+				"success",
+			]);
+			expect(
+				testState.auditEntries.every(({ transactional }) => !transactional),
+			).toBe(true);
+			if (action === "kick") {
+				expect(notify).toHaveBeenCalledWith(
+					expect.objectContaining({ action: "kick", target: "7" }),
+				);
+			}
+		},
+	);
+});

src/actions/rooms.ts

@@ -1,140 +1,163 @@
 "use server";
 
-import { and, eq, inArray } from "drizzle-orm";
 import { revalidatePath } from "next/cache";
+import {
+	executeLegacyHotelMutation,
+	HotelMutationFailure,
+} from "@/features/housekeeping/domains/hotel/services/mutations";
+import type { HousekeepingErrorCode } from "@/features/housekeeping/foundation/contracts";
 import { requirePermission } from "@/lib/admin/guard";
-import { db, Items, Rooms } from "@/lib/db";
 import { PERMS } from "@/lib/permissions";
-import { rcon } from "@/lib/services/rcon";
-import { logStaffActivity } from "@/lib/services/staff-activity";
+import {
+	type ActionResult,
+	actionOk,
+	handleActionError,
+} from "@/lib/safe-action-shared";
 import { notify } from "@/lib/services/webhook";
 
-export async function updateRoomItem(payload: Record<string, unknown>) {
-	const staff = await requirePermission(PERMS.ROOMS_EDIT);
-	const { roomId, itemId, ...data } = payload as {
-		roomId: number;
-		itemId: number;
-		[key: string]: unknown;
+const HOTEL_ACTION_ERRORS = {
+	UNAUTHENTICATED: "Your session expired. Sign in again and retry.",
+	FORBIDDEN: "You do not have permission to perform this room action.",
+	VALIDATION: "Invalid room action. Check the supplied fields and try again.",
+	NOT_FOUND: "The room or furniture item was not found in the supplied room.",
+	CONFLICT: "The room changed before this action completed. Refresh and retry.",
+	RATE_LIMITED: "Too many room actions. Wait a moment and retry.",
+	DEPENDENCY_UNAVAILABLE:
+		"The room action could not be completed. Check the database or emulator connection and try again.",
+	TIMEOUT:
+		"The room action timed out. Check its current state before retrying.",
+	INTERNAL: "The room action failed unexpectedly. Please retry.",
+} satisfies Record<HousekeepingErrorCode, string>;
+
+function actionFailure(error: unknown): ActionResult<never> {
+	if (!(error instanceof HotelMutationFailure)) return handleActionError(error);
+	const reference = error.correlationId
+		? ` Reference: ${error.correlationId}`
+		: "";
+	return {
+		ok: false,
+		error: `${HOTEL_ACTION_ERRORS[error.code]}${reference}`,
+		...(error.fieldErrors
+			? {
+					fieldErrors: Object.fromEntries(
+						Object.entries(error.fieldErrors).map(([field, errors]) => [
+							field,
+							[...errors],
+						]),
+					),
+				}
+			: {}),
 	};
-	await db
-		.update(Items)
-		.set(data as Partial<typeof Items.$inferInsert>)
-		.where(eq(Items.id, itemId));
-	await logStaffActivity({
-		staffId: staff.id,
-		action: "room_item_update",
-		description: `Updated item #${itemId} in room #${roomId}`,
-		targetType: "room_item",
-		targetId: itemId,
-	});
-	revalidatePath(`/admin/rooms/${roomId}/furni`);
 }
 
-export async function bulkDeleteRoomItems({
-	roomId,
-	itemIds,
-}: {
+function notifyBestEffort(payload: Parameters<typeof notify>[0]): void {
+	try {
+		notify(payload);
+	} catch {
+		// A committed mutation remains successful if notification dispatch fails.
+	}
+}
+
+export async function updateRoomItem(
+	payload: Record<string, unknown>,
+): Promise<ActionResult> {
+	const staff = await requirePermission(PERMS.ROOMS_EDIT);
+	try {
+		await executeLegacyHotelMutation(staff, "room-item.update", payload);
+		revalidatePath(`/admin/rooms/${String(payload.roomId)}/furni`);
+		return actionOk();
+	} catch (error) {
+		return actionFailure(error);
+	}
+}
+
+export async function bulkDeleteRoomItems(payload: {
 	roomId: number;
 	itemIds: number[];
-}) {
+}): Promise<ActionResult> {
 	const staff = await requirePermission(PERMS.ROOMS_EDIT);
-	await db
-		.delete(Items)
-		.where(and(inArray(Items.id, itemIds), eq(Items.roomId, roomId)));
-	await logStaffActivity({
-		staffId: staff.id,
-		action: "room_items_bulk_delete",
-		description: `Deleted ${itemIds.length} item(s) from room #${roomId}`,
-		targetType: "room_item",
-	});
-	revalidatePath(`/admin/rooms/${roomId}/furni`);
+	try {
+		await executeLegacyHotelMutation(staff, "room-item.bulk-delete", payload);
+		revalidatePath(`/admin/rooms/${payload.roomId}/furni`);
+		return actionOk();
+	} catch (error) {
+		return actionFailure(error);
+	}
 }
 
-export async function deleteRoomItem({
-	roomId,
-	itemId,
-}: {
+export async function deleteRoomItem(payload: {
 	roomId: number;
 	itemId: number;
-}) {
+}): Promise<ActionResult> {
 	const staff = await requirePermission(PERMS.ROOMS_EDIT);
-	await db.delete(Items).where(eq(Items.id, itemId));
-	await logStaffActivity({
-		staffId: staff.id,
-		action: "room_item_delete",
-		description: `Deleted item #${itemId} from room #${roomId}`,
-		targetType: "room_item",
-		targetId: itemId,
-	});
-	revalidatePath(`/admin/rooms/${roomId}/furni`);
+	try {
+		await executeLegacyHotelMutation(staff, "room-item.delete", payload);
+		revalidatePath(`/admin/rooms/${payload.roomId}/furni`);
+		return actionOk();
+	} catch (error) {
+		return actionFailure(error);
+	}
 }
 
-export async function roomRconAction({
-	roomId,
-	action,
-}: {
+export async function roomRconAction(payload: {
 	roomId: number;
 	action: string;
-}) {
+}): Promise<ActionResult> {
 	const staff = await requirePermission(PERMS.ROOMS_EDIT);
-	if (action === "reload") {
-		await rcon.send("reloadroom", { room_id: roomId });
-	} else if (action === "kick") {
-		await rcon.send("kickall", { room_id: roomId });
-		notify({
+	try {
+		await executeLegacyHotelMutation(staff, "room.runtime", payload);
+		if (payload.action === "kick") {
+			notifyBestEffort({
 				action: "kick",
 				actor: staff.username,
-			target: String(roomId),
-			details: action,
+				target: String(payload.roomId),
+				details: payload.action,
 			});
-	} else if (action === "lock") {
-		await rcon.send("updateroom", { room_id: roomId, state: "locked" });
-	} else if (action === "unlock") {
-		await rcon.send("updateroom", { room_id: roomId, state: "open" });
+		}
+		return actionOk();
+	} catch (error) {
+		return actionFailure(error);
 	}
 }
 
-export async function deleteRoom({ id }: { id: number }) {
+export async function deleteRoom(payload: {
+	id: number;
+}): Promise<ActionResult> {
 	const staff = await requirePermission(PERMS.ROOMS_DELETE);
-	const [room] = await db
-		.select({ name: Rooms.name })
-		.from(Rooms)
-		.where(eq(Rooms.id, id))
-		.limit(1);
-	await db.delete(Rooms).where(eq(Rooms.id, id));
-	await logStaffActivity({
-		staffId: staff.id,
-		action: "room_delete",
-		description: `Deleted room #${id}`,
-		targetType: "room",
-		targetId: id,
-	});
-	notify({
+	try {
+		const snapshot = await executeLegacyHotelMutation(
+			staff,
+			"room.delete",
+			payload,
+		);
+		notifyBestEffort({
 			action: "room_delete",
 			actor: staff.username,
-		target: room?.name ?? `#${id}`,
+			target:
+				typeof snapshot.before?.name === "string"
+					? snapshot.before.name
+					: `#${payload.id}`,
 		});
 		revalidatePath("/admin/rooms");
+		return actionOk();
+	} catch (error) {
+		return actionFailure(error);
+	}
 }
 
-export async function updateRoom({
-	id,
-	...data
-}: {
+export async function updateRoom(payload: {
 	id: number;
 	name?: string;
 	description?: string;
 	state?: string;
 	usersMax?: number;
-}) {
+}): Promise<ActionResult> {
 	const staff = await requirePermission(PERMS.ROOMS_EDIT);
-	await db.update(Rooms).set(data).where(eq(Rooms.id, id));
-	await logStaffActivity({
-		staffId: staff.id,
-		action: "room_update",
-		description: `Updated room #${id}`,
-		targetType: "room",
-		targetId: id,
-	});
-	revalidatePath(`/admin/rooms/${id}`);
+	try {
+		await executeLegacyHotelMutation(staff, "room.update", payload);
+		revalidatePath(`/admin/rooms/${payload.id}`);
+		return actionOk();
+	} catch (error) {
+		return actionFailure(error);
+	}
 }

src/app/admin/rooms/[id]/furni/edit-item-dialog.tsx

@@ -71,7 +71,7 @@ export function EditItemDialog({ roomId, item, open, onOpenChange }: Props) {
 					extraData,
 				};
 		run(() => updateRoomItem(payload), {
-			successMessage: "Item updated and room reloaded",
+			successMessage: "Item updated successfully.",
 			onSuccess: () => onOpenChange(false),
 		});
 	}

src/features/housekeeping/domains/hotel/services/mutations-runtime.ts

@@ -1,7 +1,7 @@
 import "server-only";
 
 import { randomBytes } from "node:crypto";
-import { and, eq, inArray } from "drizzle-orm";
+import { and, asc, eq, inArray } from "drizzle-orm";
 import { z } from "zod";
 import type {
 	HotelMutationContext,
@@ -253,7 +253,8 @@ async function executeRoomMutation(
 			.select()
 			.from(Rooms)
 			.where(eq(Rooms.id, value.id))
-			.limit(1);
+			.limit(1)
+			.for("update");
 		requireFound(before);
 		const { id, ...changes } = value;
 		await database.update(Rooms).set(changes).where(eq(Rooms.id, id));
@@ -268,7 +269,8 @@ async function executeRoomMutation(
 			.select()
 			.from(Rooms)
 			.where(eq(Rooms.id, value.id))
-			.limit(1);
+			.limit(1)
+			.for("update");
 		requireFound(before);
 		await database.delete(Rooms).where(eq(Rooms.id, value.id));
 		return { before: snapshotRow(before), after: null };
@@ -279,7 +281,8 @@ async function executeRoomMutation(
 			.select()
 			.from(Items)
 			.where(and(eq(Items.id, value.itemId), eq(Items.roomId, value.roomId)))
-			.limit(1);
+			.limit(1)
+			.for("update");
 		requireFound(before);
 		const { itemId, roomId: _roomId, ...changes } = value;
 		await database
@@ -297,7 +300,8 @@ async function executeRoomMutation(
 			.select()
 			.from(Items)
 			.where(and(eq(Items.id, value.itemId), eq(Items.roomId, value.roomId)))
-			.limit(1);
+			.limit(1)
+			.for("update");
 		requireFound(before);
 		await database
 			.delete(Items)
@@ -309,7 +313,9 @@ async function executeRoomMutation(
 	const before = await database
 		.select()
 		.from(Items)
-		.where(and(eq(Items.roomId, value.roomId), inArray(Items.id, itemIds)));
+		.where(and(eq(Items.roomId, value.roomId), inArray(Items.id, itemIds)))
+		.orderBy(asc(Items.id))
+		.for("update");
 	if (before.length !== itemIds.length) {
 		throw new HotelMutationFailure("NOT_FOUND", "errors.housekeeping.notFound");
 	}

src/features/housekeeping/domains/hotel/services/mutations.ts

@@ -80,6 +80,7 @@ export class HotelMutationFailure extends Error {
 		readonly code: HousekeepingErrorCode,
 		readonly messageKey: string,
 		readonly fieldErrors?: Readonly<Record<string, readonly string[]>>,
+		readonly correlationId?: string,
 	) {
 		super(messageKey);
 		this.name = "HotelMutationFailure";
@@ -187,12 +188,15 @@ export async function executeLegacyHotelMutation(
 			result.error.code,
 			result.error.messageKey,
 			result.error.fieldErrors,
+			result.correlationId,
 		);
 	}
 	if (result.completion?.external === "failed") {
 		throw new HotelMutationFailure(
 			"DEPENDENCY_UNAVAILABLE",
 			"errors.housekeeping.dependencyUnavailable",
+			undefined,
+			result.correlationId,
 		);
 	}
 	return result.data;