fix(housekeeping): preserve audit reasons and completed effects
CI / check (pull_request) Successful in 1m42s
CI / deploy (pull_request) Skipped
CI / e2e (pull_request) Skipped

This commit is contained in:
Simo committed 2026-09-05 10:06:02 +02:00
1 parent 866f38818b
commit 555bc75f9e
10 files changed
+307 -13

No files matched your search

@@ -0,0 +1,45 @@
# Housekeeping Backend Integrity Implementation Plan
> **For agentic workers:** Use superpowers:executing-plans task-by-task. Keep checkpoints independently verifiable.
**Goal:** Complete the approved backend review, starting with reproducible audit and external-completion defects.
**Architecture:** Retain the six domain services and production adapters. Follow every operation from its public entrypoint through authorization, validation, storage, external effects and audit. Route coverage is not functional completion.
**Tech Stack:** TypeScript, Drizzle/MySQL, Vitest, Next.js, pnpm.
**Spec:** Backend scope approved in conversation on 2026-09-05: complete operations, permissions, validation, transactions, concurrency, audit and partial outcomes; preserve UI and production routing.
## Global constraints
- Work in the canonical checkout on `codex/housekeeping-rebuild-stepwise`; no worktrees.
- Preserve untracked local files and existing administration routes.
- Keep PR 53 draft and update English and Dutch evidence after each verified push.
- No production writes or deployment. Missing local services block live acceptance, not implementation.
## Task 1: Preserve reasons through service and production audit
Files: Content and Economy `services/mutations.ts`, `services/mutations-production.ts`; new `src/features/housekeeping/backend-audit-integrity.test.ts`.
Interfaces: optional normalized `reason` in mutation context; existing `AuditEntry.reason` at persistence.
- [ ] Exercise real service plus production adapter with captured audit writes. For database and external operations assert `entry.reason === "Remove obsolete resource"` from a whitespace-padded invocation.
- [ ] Run `pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/backend-audit-integrity.test.ts`; verify missing reason fails.
- [ ] Normalize once at the service boundary, conditionally include the reason in the adapter context, persist in every adapter audit outcome.
- [ ] Repeat focused tests, including absent reasons and audit-failure outcomes.
## Task 2: Preserve completed Hotel external effects when audit fails
Files: Hotel `services/mutations-production.ts` and its existing test.
Interfaces: existing `HotelMutationSnapshot.completion`, with `status: "partial", external: "completed", audit: "unavailable"`.
- [ ] Execute `room.runtime` with successful effect but failing success-audit writer; assert resolved snapshot with partial completion and no failure audit.
- [ ] Verify failure against current production adapter.
- [ ] Separate external-execution errors from completion-audit errors. Preserve intent-before-effect and original external failure semantics.
- [ ] Test failed intent prevents execution and failed failure-audit does not replace the original exception.
- [ ] Run focused suites, typecheck, scoped Biome, full suite and review the diff; commit exact paths and push with normal hooks.
## Subsequent independently verified blocks
These are open audit scope, not completed tasks: Economy marketplace/voucher concurrency; operation-level parity and validation across Content, Economy, Hotel, People, System and Operations; all corresponding API and legacy entrypoints. Each block requires concrete findings, regression tests and its own implementation steps before changes. Do not mark the whole backend complete from Tasks 1-2.
@@ -0,0 +1,183 @@
import { describe, expect, it } from "vitest";
import type { AuditEntry } from "@/lib/services/audit";
import { createContentMutationService } from "./domains/content/services/mutations";
import {
ContentCommittedExternalFailure,
createContentProductionMutationAdapter,
} from "./domains/content/services/mutations-production";
import { createEconomyMutationService } from "./domains/economy/services/mutations";
import {
createEconomyProductionMutationAdapter,
EconomyCommittedExternalFailure,
} from "./domains/economy/services/mutations-production";
import { createHotelCommands } from "./domains/hotel/commands/hotel-commands";
import { createHotelMutationService } from "./domains/hotel/services/mutations";
import { createHotelProductionMutationAdapter } from "./domains/hotel/services/mutations-production";
import type { HousekeepingCapabilityContext } from "./foundation/contracts";
const capability: HousekeepingCapabilityContext = {
actor: { id: 42, username: "operator", rank: 7 },
isSuperAdmin: false,
has: () => true,
hasAny: () => true,
hasAll: () => true,
};
describe("Backend audit reason integrity", () => {
it("retains the Hotel command reason through execution and partial audit completion", async () => {
const entries: AuditEntry[] = [];
const service = createHotelMutationService(
createHotelProductionMutationAdapter({
transaction: async (run) => run({}),
writeAudit: async (entry) => {
entries.push(entry);
if (entry.outcome === "success") throw new Error("audit unavailable");
},
executeOperation: async () => ({
before: null,
after: { dispatched: true },
}),
}),
async () => capability,
);
const command = createHotelCommands(service).find(
(item) => item.operation === "room.runtime",
);
if (!command) throw new Error("missing runtime command");
const result = await command.execute(
{
capability,
correlationId: "hotel-reason",
ipAddress: "127.0.0.1",
reason: " Reload after maintenance ",
},
{ roomId: 7, action: "reload" },
);
expect(result).toMatchObject({
ok: true,
correlationId: "hotel-reason",
completion: {
status: "partial",
external: "completed",
audit: "unavailable",
},
});
expect(entries.map((entry) => entry.reason)).toEqual([
"Reload after maintenance",
"Reload after maintenance",
]);
});
for (const domain of ["content", "economy"] as const) {
it.each(["failure", "partial", "absent", "blank"] as const)(
`${domain} preserves audit reason semantics for %s`,
async (scenario) => {
const entries: AuditEntry[] = [];
const dependencies = {
transaction: async <T>(run: (tx: unknown) => Promise<T>) => run({}),
writeAudit: async (entry: AuditEntry) => {
entries.push(entry);
},
executeOperation: async () => {
if (scenario === "failure")
throw new Error("transport unavailable");
if (scenario === "partial") {
const snapshot = {
before: { id: "7" },
after: { id: "7", saved: true },
};
throw domain === "content"
? new ContentCommittedExternalFailure(snapshot)
: new EconomyCommittedExternalFailure(snapshot);
}
return { before: null, after: { saved: true } };
},
};
const invocation = {
expectedActorId: 42,
correlationId: "outcome-check",
...(scenario === "absent"
? {}
: { reason: scenario === "blank" ? " " : " Maintenance " }),
};
const result =
domain === "content"
? await createContentMutationService(
createContentProductionMutationAdapter(dependencies),
async () => capability,
).execute(invocation, "theme.update", {})
: await createEconomyMutationService(
createEconomyProductionMutationAdapter(dependencies),
async () => capability,
).execute(invocation, "catalog-page.change", {});
expect(result.ok).toBe(scenario !== "failure");
if (scenario === "partial")
expect(result).toMatchObject({
completion: {
status: "partial",
external: "failed",
audit: "persisted",
},
});
expect(entries.map((entry) => entry.outcome)).toEqual([
"intent",
scenario === "failure" || scenario === "partial"
? scenario
: "success",
]);
expect(entries.map((entry) => entry.reason)).toEqual(
scenario === "absent" || scenario === "blank"
? [undefined, undefined]
: ["Maintenance", "Maintenance"],
);
},
);
for (const external of [false, true]) {
it(`${domain} retains the normalized reason in ${external ? "external" : "database"} audit records`, async () => {
const entries: AuditEntry[] = [];
const dependencies = {
transaction: async <T>(run: (tx: unknown) => Promise<T>) => run({}),
writeAudit: async (entry: AuditEntry) => {
entries.push(entry);
},
executeOperation: async () => ({ before: { id: "7" }, after: null }),
};
const invocation = {
expectedActorId: 42,
correlationId: "reason-check",
reason: " Remove obsolete resource ",
};
const result =
domain === "content"
? await createContentMutationService(
createContentProductionMutationAdapter(dependencies),
async () => capability,
).execute(invocation, external ? "media.delete" : "tag.change", {
action: "delete",
id: "7",
})
: await createEconomyMutationService(
createEconomyProductionMutationAdapter(dependencies),
async () => capability,
).execute(
invocation,
external ? "badge.upload" : "voucher.change",
{ action: "delete", id: "7" },
);
expect(result.ok).toBe(true);
expect(
entries.map((entry) => ({
outcome: entry.outcome,
reason: entry.reason,
})),
).toEqual(
external
? [
{ outcome: "intent", reason: "Remove obsolete resource" },
{ outcome: "success", reason: "Remove obsolete resource" },
]
: [{ outcome: "success", reason: "Remove obsolete resource" }],
);
});
}
}
});
@@ -82,6 +82,7 @@ function auditEntry(
action: `content.${operation}`,
target: "Content",
correlationId: context.correlationId,
...(context.reason === undefined ? {} : { reason: context.reason }),
domain: "content",
outcome,
before:
@@ -67,6 +67,7 @@ export interface ContentMutationContext {
readonly capability: HousekeepingCapabilityContext;
readonly correlationId: string;
readonly legacy: boolean;
readonly reason?: string;
}
export interface ContentMutationAdapter {
@@ -158,10 +159,8 @@ export function createContentMutationService(
invocation.correlationId,
);
}
if (
requiresHousekeepingReason(false, input) &&
!normalizeHousekeepingReason(invocation.reason)
) {
const reason = normalizeHousekeepingReason(invocation.reason);
if (requiresHousekeepingReason(false, input) && !reason) {
return fail(
"VALIDATION",
"errors.housekeeping.validation",
@@ -174,6 +173,7 @@ export function createContentMutationService(
capability,
correlationId: invocation.correlationId,
legacy: invocation.legacy === true,
...(reason === undefined ? {} : { reason }),
});
return ok(snapshot, invocation.correlationId, snapshot.completion);
} catch (error) {
@@ -75,6 +75,7 @@ function auditEntry(
action: `economy.${operation}`,
target: "Economy",
correlationId: context.correlationId,
...(context.reason === undefined ? {} : { reason: context.reason }),
domain: "economy",
outcome,
before:
@@ -63,6 +63,7 @@ export interface EconomyMutationContext {
readonly capability: HousekeepingCapabilityContext;
readonly correlationId: string;
readonly legacy: boolean;
readonly reason?: string;
}
export interface EconomyMutationAdapter {
execute(
@@ -134,10 +135,8 @@ export function createEconomyMutationService(
invocation.correlationId,
);
}
if (
requiresHousekeepingReason(false, input) &&
!normalizeHousekeepingReason(invocation.reason)
) {
const reason = normalizeHousekeepingReason(invocation.reason);
if (requiresHousekeepingReason(false, input) && !reason) {
return fail(
"VALIDATION",
"errors.housekeeping.validation",
@@ -150,6 +149,7 @@ export function createEconomyMutationService(
capability,
correlationId: invocation.correlationId,
legacy: invocation.legacy === true,
...(reason === undefined ? {} : { reason }),
});
return ok(snapshot, invocation.correlationId, snapshot.completion);
} catch (error) {
@@ -57,6 +57,7 @@ export function createHotelCommands(
{
correlationId: context.correlationId,
expectedActorId: context.capability.actor.id,
...(context.reason === undefined ? {} : { reason: context.reason }),
},
operation,
input,
@@ -45,6 +45,51 @@ function dependencies() {
}
describe("Hotel production mutation adapter", () => {
it("retains completed RCON delivery when the completion audit fails", async () => {
const deps = dependencies();
deps.writeAudit.mockImplementation(async (entry) => {
if (entry.outcome === "success") throw new Error("audit unavailable");
});
const result = await deps.adapter.execute(
"room.runtime",
{ roomId: 7, action: "reload" },
context,
);
expect(result).toMatchObject({
after: { operation: "room.runtime", state: "after" },
completion: {
status: "partial",
external: "completed",
audit: "unavailable",
},
});
expect(deps.writeAudit.mock.calls.map(([entry]) => entry.outcome)).toEqual([
"intent",
"success",
]);
expect(deps.executeOperation).toHaveBeenCalledTimes(1);
});
it("does not execute RCON when intent persistence fails", async () => {
const deps = dependencies();
deps.writeAudit.mockRejectedValueOnce(new Error("intent unavailable"));
await expect(
deps.adapter.execute("room.runtime", {}, context),
).rejects.toThrow("intent unavailable");
expect(deps.executeOperation).not.toHaveBeenCalled();
});
it("preserves the original RCON failure if failure auditing is unavailable", async () => {
const deps = dependencies();
const failure = new Error("RCON unavailable");
deps.executeOperation.mockRejectedValueOnce(failure);
deps.writeAudit.mockImplementation(async (entry) => {
if (entry.outcome === "failure") throw new Error("audit unavailable");
});
await expect(
deps.adapter.execute("room.runtime", {}, context),
).rejects.toBe(failure);
});
it("classifies every operation exactly once", () => {
const classified = [
...HOTEL_DATABASE_OPERATIONS,
@@ -41,6 +41,7 @@ function auditEntry(
action: `hotel.${operation}`,
target: "Hotel",
correlationId: context.correlationId,
...(context.reason === undefined ? {} : { reason: context.reason }),
domain: "hotel",
outcome,
before: snapshot?.before ? { ...snapshot.before } : undefined,
@@ -70,16 +71,13 @@ export function createHotelProductionMutationAdapter(
}
await dependencies.writeAudit(auditEntry(operation, context, "intent"));
let snapshot: HotelMutationSnapshot;
try {
const snapshot = await dependencies.executeOperation(
snapshot = await dependencies.executeOperation(
operation,
input,
context,
);
await dependencies.writeAudit(
auditEntry(operation, context, "success", snapshot),
);
return snapshot;
} catch (error) {
try {
await dependencies.writeAudit(
@@ -90,6 +88,21 @@ export function createHotelProductionMutationAdapter(
}
throw error;
}
try {
await dependencies.writeAudit(
auditEntry(operation, context, "success", snapshot),
);
return snapshot;
} catch {
return {
...snapshot,
completion: {
status: "partial",
external: "completed",
audit: "unavailable",
},
};
}
},
};
}
@@ -1,5 +1,6 @@
import { PERMS } from "@/lib/permission-slugs";
import { satisfiesCapability } from "../../../foundation/capability-context";
import { normalizeHousekeepingReason } from "../../../foundation/commands/reason-policy";
import {
anyCapability,
fail,
@@ -48,12 +49,14 @@ export interface HotelMutationInvocation {
readonly correlationId: string;
readonly expectedActorId: number;
readonly legacy?: boolean;
readonly reason?: string;
}
export interface HotelMutationContext {
readonly capability: HousekeepingCapabilityContext;
readonly correlationId: string;
readonly legacy: boolean;
readonly reason?: string;
}
export interface HotelMutationAdapter {
@@ -116,10 +119,12 @@ export function createHotelMutationService(
);
}
try {
const reason = normalizeHousekeepingReason(invocation.reason);
const snapshot = await adapter.execute(operation, input, {
capability,
correlationId: invocation.correlationId,
legacy: invocation.legacy === true,
...(reason === undefined ? {} : { reason }),
});
return ok(snapshot, invocation.correlationId, snapshot.completion);
} catch (error) {