fix(housekeeping): correct people partial audit evidence

.superpowers/sdd/2026-08-26-housekeeping-completion/task-12-report.md

@@ -254,4 +254,57 @@ pnpm exec biome check --formatter-enabled=false <28 exact changed TypeScript/TSX
 Checked 28 files. No fixes applied.
 ```
 
+The approved Node engine warning remains: the repository requests Node `>=26.8.1 <27`, while the host runs Node `v26.7.0` with pnpm `11.24.0`. No database operation, deployment, push, or pull-request update was performed.
+
+## Official review fix round 3
+
+The round-2 re-review reported 0 Critical, 2 Important, and 2 adjacent Minor findings. This round addresses all four findings without changing the nine-route manifest, the public command IDs, or legacy external call ordering and permissions.
+
+### RED evidence
+
+```text
+Focused external-audit, bulk-production, and dispatcher matrix
+Test Files 2 failed (2)
+Tests 15 failed | 54 passed (69)
+
+The ten external-only false/throw cases persisted optimistic desired after-state instead of confirmed unchanged or unknown delivery evidence. Four bulk currency/badge false/throw cases reported completed=0 and Database error after a committed database write. The dispatcher accepted one ok:false result carrying impossible completion metadata.
+```
+
+### GREEN implementation
+
+- External-only alert, disconnect, mute, unmute, and send-currency keep desired state in intent/success evidence. Confirmed RCON `false` now writes a dedicated unchanged/no-delivery failure snapshot; an exception writes unknown delivery with a null after-state. Correlation and failure outcome stay identical across intent/outcome records.
+- Bulk currency and badge count a successful database write before RCON. RCON false/throw is additive `externalSyncFailures` sync debt, never a database failure; `failedIds` remains reserved for database/business failures and the result is typed partial with an explicit no-automatic-retry warning.
+- Webhook notification remains explicit fire-and-forget best effort (`void notify(...)`) and no longer participates in mutation completion. The impossible promise-rejection test was replaced with the real void contract.
+- The dispatcher runtime schema now accepts `completion` only for `ok: true`, matching the TypeScript `HousekeepingResult` contract; failure envelopes containing it are rejected as malformed.
+
+### Final verification after review fix round 3
+
+```text
+Focused external audit + production bulk + dispatcher
+Test Files 3 passed (3)
+Tests 73 passed (73)
+
+People + foundation + legacy wrappers + staff-smoke matrix
+Test Files 48 passed (48)
+Tests 470 passed (470)
+
+pnpm test:housekeeping
+Test Files 58 passed (58)
+Tests 516 passed (516)
+
+pnpm test
+Test Files 206 passed | 3 skipped (209)
+Tests 1359 passed | 5 skipped (1364)
+
+pnpm typecheck
+tsc --noEmit
+Exit 0
+
+pnpm exec biome check --formatter-enabled=false <4 exact changed source/test files>
+Checked 4 files. No fixes applied.
+
+git diff --check
+Exit 0
+```
+
 The approved Node engine warning remains: the repository requests Node `>=26.8.1 <27`, while the host runs Node `v26.7.0` with pnpm `11.24.0`. No database operation, deployment, push, or pull-request update was performed.

src/features/housekeeping/domains/people/services/mutations-production-workflows.test.ts

@@ -578,6 +578,117 @@ describe("People production workflow adapter", () => {
 		]);
 	});
 
+	it.each([
+		[
+			"user.alert",
+			"alertUser",
+			{ userId: 7, message: "Hello" },
+			{ userId: 7, alertDelivered: false },
+		],
+		[
+			"user.disconnect",
+			"disconnectUser",
+			{ userId: 7, reason: "Stuck session" },
+			target({ online: true }),
+		],
+		[
+			"user.mute",
+			"muteUser",
+			{ userId: 7, duration: 30, reason: "Chat abuse" },
+			target({ online: true }),
+		],
+		[
+			"user.unmute",
+			"unmuteUser",
+			{ userId: 7, reason: "Appeal accepted" },
+			target({ online: true }),
+		],
+		[
+			"user.send-currency",
+			"giveCredits",
+			{ userId: 7, amount: 25, reason: "Event award" },
+			target({ online: true }),
+		],
+	] as const)(
+		"audits confirmed external-only %s failure without optimistic state",
+		async (operation, rconMethod, input, failureAfter) => {
+			if (operation !== "user.alert") mocks.selectQueue.push([target()]);
+			mocks.rcon[rconMethod].mockResolvedValueOnce(false);
+			const correlationId = `confirmed-${operation}`;
+
+			const result = await peopleMutationService.execute(
+				{ ...invocation, correlationId, legacy: false },
+				operation,
+				input,
+			);
+
+			expect(result).toMatchObject({
+				ok: false,
+				error: { code: "DEPENDENCY_UNAVAILABLE" },
+				correlationId,
+			});
+			const entries = mocks.audit.mock.calls.map(
+				(call) => call[0] as AuditEntry,
+			);
+			expect(entries.map(({ outcome }) => outcome)).toEqual([
+				"intent",
+				"failure",
+			]);
+			expect(entries[1]).toMatchObject({ correlationId, after: failureAfter });
+			expect(entries[1]?.after).not.toEqual(entries[0]?.after);
+		},
+	);
+
+	it.each([
+		["user.alert", "alertUser", { userId: 7, message: "Hello" }],
+		[
+			"user.disconnect",
+			"disconnectUser",
+			{ userId: 7, reason: "Stuck session" },
+		],
+		[
+			"user.mute",
+			"muteUser",
+			{ userId: 7, duration: 30, reason: "Chat abuse" },
+		],
+		["user.unmute", "unmuteUser", { userId: 7, reason: "Appeal accepted" }],
+		[
+			"user.send-currency",
+			"giveCredits",
+			{ userId: 7, amount: 25, reason: "Event award" },
+		],
+	] as const)(
+		"audits unknown external-only %s delivery with a null after-state",
+		async (operation, rconMethod, input) => {
+			if (operation !== "user.alert") mocks.selectQueue.push([target()]);
+			mocks.rcon[rconMethod].mockRejectedValueOnce(
+				new Error("RCON unavailable"),
+			);
+			const correlationId = `unknown-${operation}`;
+
+			const result = await peopleMutationService.execute(
+				{ ...invocation, correlationId, legacy: false },
+				operation,
+				input,
+			);
+
+			expect(result).toMatchObject({
+				ok: false,
+				error: { code: "DEPENDENCY_UNAVAILABLE" },
+				correlationId,
+			});
+			const entries = mocks.audit.mock.calls.map(
+				(call) => call[0] as AuditEntry,
+			);
+			expect(entries.map(({ outcome }) => outcome)).toEqual([
+				"intent",
+				"failure",
+			]);
+			expect(entries[1]).toMatchObject({ correlationId });
+			expect(entries[1]?.after).toBeUndefined();
+		},
+	);
+
 	it("executes bulk ban, currency, and badge with database, RCON, and legacy activity outcomes", async () => {
 		await expect(
 			peopleMutationService.execute(invocation, "users.bulk-ban", {
@@ -625,81 +736,131 @@ describe("People production workflow adapter", () => {
 		);
 	});
 
-	it("reports bulk currency as partial after database commit when RCON reports false", async () => {
+	it.each(["false", "throw"] as const)(
-		mocks.rcon.giveCredits.mockResolvedValue(false);
+		"reports bulk currency database completion separately when RCON reports %s",
-		const result = await peopleMutationService.execute(
+		async (failureMode) => {
-			{ ...invocation, correlationId: "bulk-currency-partial", legacy: false },
+			if (failureMode === "false") {
-			"users.bulk-currency",
+				mocks.rcon.giveCredits.mockResolvedValue(false);
-			{ userIds: [7], type: "credits", amount: 50 },
+			} else {
-		);
+				mocks.rcon.giveCredits.mockRejectedValue(new Error("RCON unavailable"));
-
+			}
-		expect(result).toMatchObject({
+			const result = await peopleMutationService.execute(
-			ok: true,
+				{
-			completion: {
+					...invocation,
-				status: "partial",
+					correlationId: "bulk-currency-partial",
-				external: "failed",
+					legacy: false,
-				audit: "persisted",
-			},
-			data: {
-				after: {
-					completed: 0,
-					total: 1,
-					failedIds: [{ userId: 7, reason: "Database error" }],
 				},
-			},
+				"users.bulk-currency",
-			correlationId: "bulk-currency-partial",
+				{ userIds: [7], type: "credits", amount: 50 },
-		});
+			);
-		expect(
+
-			mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome),
+			expect(result).toMatchObject({
-		).toEqual(["intent", "partial"]);
+				ok: true,
-	});
+				completion: {
-	it("returns typed partial after committed update or reset when notification fails", async () => {
+					status: "partial",
-		mocks.notify.mockRejectedValue(new Error("notification unavailable"));
+					external: "failed",
+					audit: "persisted",
+				},
+				data: {
+					after: {
+						completed: 1,
+						total: 1,
+						failedIds: [],
+						externalSyncFailures: [
+							{
+								userId: 7,
+								reason:
+									"Database applied; emulator sync failed. Do not retry automatically.",
+							},
+						],
+					},
+				},
+				correlationId: "bulk-currency-partial",
+			});
+			expect(
+				mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome),
+			).toEqual(["intent", "partial"]);
+		},
+	);
+
+	it.each(["false", "throw"] as const)(
+		"reports bulk badge database completion separately when RCON reports %s",
+		async (failureMode) => {
+			mocks.selectQueue.push([], [{ maxSlot: 2 }]);
+			if (failureMode === "false") {
+				mocks.rcon.giveBadge.mockResolvedValue(false);
+			} else {
+				mocks.rcon.giveBadge.mockRejectedValue(new Error("RCON unavailable"));
+			}
+			const result = await peopleMutationService.execute(
+				{ ...invocation, correlationId: "bulk-badge-partial", legacy: false },
+				"users.bulk-badge",
+				{ userIds: [7], badgeCode: "ADM" },
+			);
+
+			expect(result).toMatchObject({
+				ok: true,
+				completion: {
+					status: "partial",
+					external: "failed",
+					audit: "persisted",
+				},
+				data: {
+					after: {
+						completed: 1,
+						total: 1,
+						failedIds: [],
+						externalSyncFailures: [
+							{
+								userId: 7,
+								reason:
+									"Database applied; emulator sync failed. Do not retry automatically.",
+							},
+						],
+					},
+				},
+				correlationId: "bulk-badge-partial",
+			});
+			expect(
+				mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome),
+			).toEqual(["intent", "partial"]);
+		},
+	);
+
+	it("keeps best-effort notification outside update and reset completion", async () => {
 		mocks.selectQueue.push([target()]);
 		const updated = await peopleMutationService.execute(
-			{ ...invocation, correlationId: "update-notify-partial", legacy: false },
+			{ ...invocation, correlationId: "update-notify", legacy: false },
 			"user.update",
 			{ userId: 7, fields: { motto: "Updated" } },
 		);
-		expect(updated).toMatchObject({
+		expect(updated).toMatchObject({ ok: true, correlationId: "update-notify" });
-			ok: true,
+		expect(updated).not.toHaveProperty("completion");
-			completion: {
-				status: "partial",
-				external: "failed",
-				audit: "persisted",
-			},
-			correlationId: "update-notify-partial",
-		});
 		expect(
 			mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome),
-		).toEqual(["intent", "partial"]);
+		).toEqual(["intent", "success"]);
 
 		vi.clearAllMocks();
 		mocks.resolveServerContext.mockResolvedValue(context());
 		mocks.audit.mockResolvedValue(undefined);
 		mocks.hashPassword.mockResolvedValue("hashed-password");
-		mocks.notify.mockRejectedValue(new Error("notification unavailable"));
 		mocks.selectQueue.push([target()]);
 		const reset = await peopleMutationService.execute(
-			{ ...invocation, correlationId: "reset-notify-partial", legacy: false },
+			{ ...invocation, correlationId: "reset-notify", legacy: false },
 			"user.reset-password",
 			{ userId: 7, reason: "Verified owner" },
 		);
 		expect(reset).toMatchObject({
 			ok: true,
-			completion: {
-				status: "partial",
-				external: "failed",
-				audit: "persisted",
-			},
 			data: { output: { newPassword: expect.any(String) } },
-			correlationId: "reset-notify-partial",
+			correlationId: "reset-notify",
 		});
+		expect(reset).not.toHaveProperty("completion");
 		expect(
 			mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome),
-		).toEqual(["intent", "partial"]);
+		).toEqual(["intent", "success"]);
+		expect(mocks.notify).toHaveBeenCalledTimes(1);
 	});
-
 	it("preserves legacy throw and false semantics while commands retain partial truth", async () => {
 		mocks.selectQueue.push(
 			[target()],

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

@@ -118,6 +118,8 @@ class PeopleMutationFailure extends Error {
 	}
 }
 
+class ConfirmedExternalNoopFailure extends PeopleMutationFailure {}
+
 function operationCapability(operation: PeopleMutationOperation) {
 	if (operation === "user.ban" || operation === "user.unban") {
 		return anyCapability(PERMS.USERS_BAN);
@@ -307,7 +309,7 @@ function userIds(value: unknown, legacy: boolean): number[] {
 
 async function requireRcon(result: boolean): Promise<void> {
 	if (!result) {
-		throw new PeopleMutationFailure(
+		throw new ConfirmedExternalNoopFailure(
 			"DEPENDENCY_UNAVAILABLE",
 			"errors.housekeeping.dependencyUnavailable",
 		);
@@ -402,10 +404,16 @@ async function finalizeExternalWithAudit(
 	snapshot: PeopleMutationSnapshot,
 	execute: () => Promise<void>,
 	mutationCommitted = true,
+	confirmedFailureSnapshot?: PeopleMutationSnapshot,
 ): Promise<PeopleMutationSnapshot> {
 	try {
 		await execute();
 	} catch (error) {
+		const failureSnapshot = mutationCommitted
+			? snapshot
+			: error instanceof ConfirmedExternalNoopFailure
+				? (confirmedFailureSnapshot ?? { before: snapshot.before, after: null })
+				: { before: snapshot.before, after: null };
 		let audit: HousekeepingPartialCompletion["audit"] = "persisted";
 		try {
 			await logAudit(
@@ -414,7 +422,7 @@ async function finalizeExternalWithAudit(
 					operation,
 					target,
 					targetId,
-					snapshot,
+					failureSnapshot,
 					mutationCommitted ? "partial" : "failure",
 				),
 			);
@@ -482,6 +490,7 @@ async function runExternalWithAudit(
 	targetId: number | undefined,
 	snapshot: PeopleMutationSnapshot,
 	execute: () => Promise<void>,
+	confirmedFailureSnapshot: PeopleMutationSnapshot,
 ): Promise<PeopleMutationSnapshot> {
 	await logAudit(
 		canonicalAuditEntry(
@@ -501,6 +510,7 @@ async function runExternalWithAudit(
 		snapshot,
 		execute,
 		false,
+		confirmedFailureSnapshot,
 	);
 }
 async function executeUserMutation(
@@ -524,6 +534,7 @@ async function executeUserMutation(
 			userId,
 			snapshot,
 			async () => requireRcon(await rcon.alertUser(userId, message)),
+			{ before: null, after: { userId, alertDelivered: false } },
 		);
 	}
 
@@ -592,13 +603,12 @@ async function executeUserMutation(
 			snapshot,
 			async () => {
 				invalidateLoginCache(target.username);
-				const notification = notify({
+				void notify({
 					action: "user_edit",
 					actor: context.capability.actor.username,
 					target: target.username,
 					targetId: userId,
 				});
-				if (!context.legacy) await notification;
 			},
 		);
 	}
@@ -704,7 +714,7 @@ async function executeUserMutation(
 			snapshot,
 			async () => {
 				await requireRcon(await rcon.disconnectUser(userId));
-				await notify({
+				void notify({
 					action: "ban",
 					actor: context.capability.actor.username,
 					target: target.username,
@@ -787,7 +797,7 @@ async function executeUserMutation(
 			);
 			return value;
 		});
-		notify({
+		void notify({
 			action: "unban",
 			actor: context.capability.actor.username,
 			target: target.username,
@@ -805,12 +815,13 @@ async function executeUserMutation(
 			snapshot,
 			async () => {
 				await requireRcon(await rcon.disconnectUser(userId));
-				await notify({
+				void notify({
 					action: "disconnect",
 					actor: context.capability.actor.username,
 					target: target.username,
 				});
 			},
+			{ before, after: before },
 		);
 	}
 
@@ -838,6 +849,7 @@ async function executeUserMutation(
 					await requireRcon(await rcon.unmuteUser(userId));
 				}
 			},
+			{ before, after: before },
 		);
 	}
 
@@ -877,13 +889,12 @@ async function executeUserMutation(
 			snapshot,
 			async () => {
 				invalidateLoginCache(target.username);
-				const notification = notify({
+				void notify({
 					action: "user_edit",
 					actor: context.capability.actor.username,
 					target: target.username,
 					details: "Password reset",
 				});
-				if (!context.legacy) await notification;
 			},
 		);
 	}
@@ -898,6 +909,7 @@ async function executeUserMutation(
 			userId,
 			snapshot,
 			async () => requireRcon(await rcon.giveCredits(userId, amount)),
+			{ before, after: before },
 		);
 	}
 
@@ -1014,8 +1026,10 @@ async function executeBulkMutation(
 	const data = record(input);
 	const ids = userIds(data.userIds, context.legacy);
 	const failures: Array<{ userId: number; reason: string }> = [];
+	const externalSyncFailures: Array<{ userId: number; reason: string }> = [];
+	const externalSyncFailureReason =
+		"Database applied; emulator sync failed. Do not retry automatically.";
 	let completed = 0;
-	let externalFailed = false;
 	let mutationReason: string | undefined;
 
 	if (operation === "users.bulk-unban") {
@@ -1126,16 +1140,12 @@ async function executeBulkMutation(
 			| "pixels"
 			| "points";
 		for (const userId of ids) {
-			let databaseCommitted = false;
 			try {
 				if (type === "credits") {
 					await db
 						.update(User)
 						.set({ credits: sql`${User.credits} + ${amount}` })
 						.where(eq(User.id, userId));
-					databaseCommitted = true;
-					const delivered = await rcon.giveCredits(userId, amount);
-					if (!context.legacy) await requireRcon(delivered);
 				} else {
 					const currencyType = type === "pixels" ? 0 : 101;
 					await db
@@ -1144,19 +1154,25 @@ async function executeBulkMutation(
 						.onDuplicateKeyUpdate({
 							set: { amount: sql`${UsersCurrency.amount} + ${amount}` },
 						});
-					databaseCommitted = true;
-					if (type === "pixels") {
-						const delivered = await rcon.giveDuckets(userId, amount);
-						if (!context.legacy) await requireRcon(delivered);
-					} else {
-						const delivered = await rcon.givePointsGotw(userId, amount);
-						if (!context.legacy) await requireRcon(delivered);
-					}
 				}
-				completed += 1;
 			} catch {
-				if (databaseCommitted) externalFailed = true;
 				failures.push({ userId, reason: "Database error" });
+				continue;
+			}
+			completed += 1;
+			try {
+				const delivered =
+					type === "credits"
+						? await rcon.giveCredits(userId, amount)
+						: type === "pixels"
+							? await rcon.giveDuckets(userId, amount)
+							: await rcon.givePointsGotw(userId, amount);
+				if (!context.legacy) await requireRcon(delivered);
+			} catch {
+				externalSyncFailures.push({
+					userId,
+					reason: externalSyncFailureReason,
+				});
 			}
 		}
 		await logStaffActivity({
@@ -1168,7 +1184,7 @@ async function executeBulkMutation(
 	} else {
 		const badgeCode = normalizedText(data.badgeCode, 20);
 		for (const userId of ids) {
-			let databaseCommitted = false;
+			let shouldSync = false;
 			try {
 				const [existing] = await db
 					.select({ id: UsersBadges.id })
@@ -1190,14 +1206,22 @@ async function executeBulkMutation(
 						slotId: (aggregate?.maxSlot ?? 0) + 1,
 						badgeCode,
 					});
-					databaseCommitted = true;
+					shouldSync = true;
-					const delivered = await rcon.giveBadge(userId, badgeCode);
-					if (!context.legacy) await requireRcon(delivered);
 				}
 				completed += 1;
 			} catch {
-				if (databaseCommitted) externalFailed = true;
 				failures.push({ userId, reason: "Database error" });
+				continue;
+			}
+			if (!shouldSync) continue;
+			try {
+				const delivered = await rcon.giveBadge(userId, badgeCode);
+				if (!context.legacy) await requireRcon(delivered);
+			} catch {
+				externalSyncFailures.push({
+					userId,
+					reason: externalSyncFailureReason,
+				});
 			}
 		}
 		await logStaffActivity({
@@ -1208,11 +1232,13 @@ async function executeBulkMutation(
 		});
 	}
 
+	const hasPartialCompletion =
+		failures.length > 0 || externalSyncFailures.length > 0;
 	const completion: HousekeepingPartialCompletion | undefined =
-		failures.length > 0
+		hasPartialCompletion
 			? {
 					status: "partial",
-					external: externalFailed ? "failed" : "not-required",
+					external: externalSyncFailures.length > 0 ? "failed" : "not-required",
 					audit: "persisted",
 				}
 			: undefined;
@@ -1222,6 +1248,7 @@ async function executeBulkMutation(
 			completed,
 			total: ids.length,
 			failedIds: failures,
+			...(externalSyncFailures.length > 0 ? { externalSyncFailures } : {}),
 			...(mutationReason ? { reason: mutationReason } : {}),
 		},
 		...(completion === undefined ? {} : { completion }),
@@ -1234,7 +1261,7 @@ async function executeBulkMutation(
 				"User",
 				undefined,
 				snapshot,
-				failures.length > 0 ? "partial" : "success",
+				hasPartialCompletion ? "partial" : "success",
 			),
 		);
 	} catch (auditOutcomeError) {

src/features/housekeeping/foundation/commands/dispatcher.test.ts

@@ -952,6 +952,22 @@ describe("dispatchHousekeepingCommand", () => {
 			"invalid failure error",
 			{ ok: false, error: null, correlationId: "forged" },
 		],
+		[
+			"failure completion metadata",
+			{
+				ok: false,
+				error: {
+					code: "DEPENDENCY_UNAVAILABLE",
+					messageKey: "errors.housekeeping.dependencyUnavailable",
+				},
+				correlationId: "forged",
+				completion: {
+					status: "partial",
+					external: "failed",
+					audit: "persisted",
+				},
+			},
+		],
 		[
 			"throwing getter",
 			Object.defineProperty({}, "ok", {

src/features/housekeeping/foundation/commands/dispatcher.ts

@@ -71,13 +71,6 @@ const housekeepingResultSchema = z.discriminatedUnion("ok", [
 		ok: z.literal(false),
 		error: housekeepingErrorSchema,
 		correlationId: z.string().min(1).max(160),
-		completion: z
-			.strictObject({
-				status: z.literal("partial"),
-				external: z.enum(["not-required", "completed", "failed"]),
-				audit: z.enum(["persisted", "unavailable"]),
-			})
-			.optional(),
 	}),
 ]);