From 91c9efcbb4c8ade3a7b19bb6ce1fd2a7e558f715 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sat, 29 Aug 2026 14:39:38 +0200 Subject: [PATCH] fix(housekeeping): complete people workflow fidelity --- .../task-12-report.md | 49 ++ src/actions/admin-applications.ts | 3 +- src/actions/admin-ip.test.ts | 12 +- src/actions/admin-ip.ts | 7 +- src/actions/admin-teams.test.ts | 10 +- src/actions/admin-teams.ts | 6 +- src/actions/admin-wordfilter.ts | 7 +- src/actions/housekeeping-command.test.ts | 42 +- src/actions/housekeeping-command.ts | 25 - src/actions/people-shared-wrappers.test.ts | 15 +- .../commands/community-commands.test.ts | 44 ++ .../people/commands/community-commands.ts | 32 +- .../housekeeping/domains/people/models.ts | 6 +- .../people/pages/people-command-form.tsx | 122 ++-- .../pages/people-primary-pages.test.tsx | 68 ++- .../people/queries/people-queries.test.ts | 7 +- .../domains/people/queries/staff.ts | 6 +- .../services/mutations-audit-contract.test.ts | 101 ++-- .../mutations-production-workflows.test.ts | 543 +++++++++++++++++- .../services/mutations-production.test.ts | 6 +- .../mutations-reason-production.test.ts | 31 +- .../mutations-transactional-audit.test.ts | 2 +- .../domains/people/services/mutations.ts | 395 +++++++++---- .../foundation/commands/dispatcher.test.ts | 14 +- .../foundation/commands/dispatcher.ts | 68 ++- .../foundation/contracts/index.ts | 1 + .../foundation/contracts/result.ts | 26 +- .../foundation-source-contract.test.ts | 1 - src/lib/services/audit.test.ts | 6 +- 29 files changed, 1365 insertions(+), 290 deletions(-) diff --git a/.superpowers/sdd/2026-08-26-housekeeping-completion/task-12-report.md b/.superpowers/sdd/2026-08-26-housekeeping-completion/task-12-report.md index fb047c7e..0538f78b 100644 --- a/.superpowers/sdd/2026-08-26-housekeeping-completion/task-12-report.md +++ b/.superpowers/sdd/2026-08-26-housekeeping-completion/task-12-report.md @@ -205,4 +205,53 @@ git diff --check e1b31ff7738eb5cc59e7765c8ed7290d62130972 -- 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. +## Official review fix round 2 + +The round-1 re-review reported 0 Critical, 7 Important, and no Minor findings. This round addresses all seven findings without changing the nine-route Task 12 manifest or exposing any raw production adapter. + +### RED evidence + +```text +Typed partial/result contract: 5 failed / 8 passed before completion metadata and audit-outcome handling were added. +Observed active-ban and absent-settings snapshots: 2 focused failures before deterministic active reads and null-preserving trade snapshots. +BIGINT preservation: 8 focused failures across application, team, IP, and wordfilter before decimal string/BigInt boundaries. +Real form/reset workflow: 2 primary-page failures before the actual action adapter and one-time credential result were added. +Production operation closure: 2 failed / 10 passed before unban observed-after and bulk false-RCON partial truth. +Post-commit notification/legacy parity: 2 failed / 12 passed before update/reset transactional intent and legacy throw/false mapping. +Cumulative gate exposed one unsupported custom Zod schema, one stale direct-alert expectation, and one stale numeric audit-ID expectation; each received a minimal regression-preserving fix. +``` + +### GREEN implementation + +- Mixed database/external operations now commit sanitized intent with the mutation and return correlated typed `partial` completion when RCON, cache, notification, or final audit persistence fails afterward. Pre-mutation external failure and intent persistence failure remain blocking. The dispatcher and public server action preserve one serializable partial result and emit no contradictory generic failure evidence. +- Ban and unban reuse the permanent-or-unexpired Task 11 filter, deterministic timestamp/ID ordering, and observed before/after reads. An absent `UsersSettings` row remains null before and after a no-op trade settings update. +- Legacy alert calls RCON without a target query or hierarchy guard. Legacy wrappers retain their prior false/throw behavior while new Housekeeping commands report synchronization false as partial truth. +- Application, team, IP, and wordfilter identifiers remain canonical decimal strings/`BigInt` through wrappers and Drizzle, including values above `Number.MAX_SAFE_INTEGER`. The cloneable command regex accepts the full unsigned BIGINT range and rejects overflow without a Zod custom refinement. +- Reset-password returns the generated credential once in the current authorized form result. It is rendered through an accessible `output`, excluded from durable service evidence, and recursively redacted by the canonical audit sanitizer. +- Production tests execute all twenty Task 12 operation IDs with meaningful database/RCON/audit assertions. The primary-page test invokes the actual form action adapter, and the unused multi-accounts-to-command-form boundary exception was removed. + +### Final verification after review fix round 2 + +```text +Focused People + foundation + wrappers + audit + action + staff-smoke matrix +Test Files 45 passed (45) +Tests 459 passed (459) + +pnpm test:housekeeping +Test Files 58 passed (58) +Tests 502 passed (502) + +pnpm test +Test Files 206 passed | 3 skipped (209) +Tests 1345 passed | 5 skipped (1350) + +pnpm typecheck +tsc --noEmit +Exit 0 + +pnpm exec biome check --formatter-enabled=false <28 exact changed TypeScript/TSX files> +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. \ No newline at end of file diff --git a/src/actions/admin-applications.ts b/src/actions/admin-applications.ts index 382c3962..c2906f26 100644 --- a/src/actions/admin-applications.ts +++ b/src/actions/admin-applications.ts @@ -14,8 +14,7 @@ export async function dismissApplication(formData: FormData): Promise { const staff = await requirePermission(PERMS.USERS_EDIT); const rawId = formPositiveBigInt(formData, "id"); if (!rawId) return; - const applicationId = Number(rawId); - if (!Number.isSafeInteger(applicationId) || applicationId <= 0) return; + const applicationId = rawId.toString(); await peopleMutationService.execute( createPeopleMutationInvocation(staff, createCorrelationId()), diff --git a/src/actions/admin-ip.test.ts b/src/actions/admin-ip.test.ts index e74d1308..467f1f1f 100644 --- a/src/actions/admin-ip.test.ts +++ b/src/actions/admin-ip.test.ts @@ -51,8 +51,8 @@ it("preserves all four IP actions and /admin revalidation", async () => { "ip.action", { action: "add-blacklist", ipAddress: "198.51.100.1", asn: "" }, ], - ["ip.action", { action: "delete-whitelist", id: 42 }], - ["ip.action", { action: "delete-blacklist", id: 99 }], + ["ip.action", { action: "delete-whitelist", id: "42" }], + ["ip.action", { action: "delete-blacklist", id: "99" }], ]); expect(revalidatePath).toHaveBeenCalledTimes(4); }); @@ -62,3 +62,11 @@ it("keeps empty IP input as a no-op after authorization", async () => { expect(requirePermission).toHaveBeenCalledWith("admin.settings.edit"); expect(execute).not.toHaveBeenCalled(); }); + +it("preserves an IP rule ID above Number.MAX_SAFE_INTEGER", async () => { + await deleteBlacklist(form({ id: "9007199254740993" })); + expect(execute).toHaveBeenCalledWith(expect.anything(), "ip.action", { + action: "delete-blacklist", + id: "9007199254740993", + }); +}); diff --git a/src/actions/admin-ip.ts b/src/actions/admin-ip.ts index 2e625a47..a69dee37 100644 --- a/src/actions/admin-ip.ts +++ b/src/actions/admin-ip.ts @@ -7,6 +7,7 @@ import { } from "@/features/housekeeping/domains/people/services/mutations"; import { createCorrelationId } from "@/features/housekeeping/foundation/contracts"; import { requirePermission } from "@/lib/admin/guard"; +import { formPositiveBigInt } from "@/lib/form-data"; import { PERMS } from "@/lib/permissions"; function parse(formData: FormData, key: string): string { @@ -26,16 +27,16 @@ async function run( ): Promise { const staff = await requirePermission(PERMS.SETTINGS_EDIT); const adding = action.startsWith("add-"); + const rawId = adding ? null : formPositiveBigInt(formData, "id"); const input = adding ? { action, ipAddress: parse(formData, "ipAddress"), asn: parse(formData, "asn"), } - : { action, id: Number(formData.get("id")) }; + : { action, id: rawId?.toString() ?? "" }; if (adding && !("ipAddress" in input && input.ipAddress)) return; - const id = "id" in input ? input.id : undefined; - if (!adding && !(Number.isSafeInteger(id) && Number(id) > 0)) return; + if (!adding && !rawId) return; const result = await peopleMutationService.execute( createPeopleMutationInvocation(staff, createCorrelationId()), "ip.action", diff --git a/src/actions/admin-teams.test.ts b/src/actions/admin-teams.test.ts index 3eb5b0b8..b9e91b27 100644 --- a/src/actions/admin-teams.test.ts +++ b/src/actions/admin-teams.test.ts @@ -50,7 +50,7 @@ it("preserves create and delete team payloads plus /admin revalidation", async ( hiddenRank: false, }, ], - ["team.change", { action: "delete", teamId: 42 }], + ["team.change", { action: "delete", teamId: "42" }], ]); expect(revalidatePath).toHaveBeenCalledTimes(2); }); @@ -59,3 +59,11 @@ it("preserves empty rank name as a no-op", async () => { await createTeam(form({ rankName: "" })); expect(execute).not.toHaveBeenCalled(); }); + +it("preserves a team ID above Number.MAX_SAFE_INTEGER", async () => { + await deleteTeam(form({ id: "9007199254740993" })); + expect(execute).toHaveBeenCalledWith(expect.anything(), "team.change", { + action: "delete", + teamId: "9007199254740993", + }); +}); diff --git a/src/actions/admin-teams.ts b/src/actions/admin-teams.ts index ae214e06..92583625 100644 --- a/src/actions/admin-teams.ts +++ b/src/actions/admin-teams.ts @@ -7,6 +7,7 @@ import { } from "@/features/housekeeping/domains/people/services/mutations"; import { createCorrelationId } from "@/features/housekeeping/foundation/contracts"; import { requirePermission } from "@/lib/admin/guard"; +import { formPositiveBigInt } from "@/lib/form-data"; import { PERMS } from "@/lib/permissions"; function text(formData: FormData, key: string): string { @@ -37,8 +38,9 @@ export async function createTeam(formData: FormData): Promise { export async function deleteTeam(formData: FormData): Promise { const staff = await requirePermission(PERMS.USERS_EDIT); - const teamId = Number(formData.get("id")); - if (!Number.isSafeInteger(teamId) || teamId <= 0) return; + const rawTeamId = formPositiveBigInt(formData, "id"); + if (!rawTeamId) return; + const teamId = rawTeamId.toString(); const result = await peopleMutationService.execute( createPeopleMutationInvocation(staff, createCorrelationId()), "team.change", diff --git a/src/actions/admin-wordfilter.ts b/src/actions/admin-wordfilter.ts index 0af1ad09..ce6da022 100644 --- a/src/actions/admin-wordfilter.ts +++ b/src/actions/admin-wordfilter.ts @@ -7,6 +7,7 @@ import { } from "@/features/housekeeping/domains/people/services/mutations"; import { createCorrelationId } from "@/features/housekeeping/foundation/contracts"; import { requirePermission } from "@/lib/admin/guard"; +import { positiveBigInt } from "@/lib/api"; import { PERMS } from "@/lib/permissions"; import { type ActionResult, @@ -36,9 +37,9 @@ export async function addWord(input: { export async function deleteWord(input: { id: string }): Promise { const staff = await requirePermission(PERMS.WORDFILTER_EDIT); - const id = Number(String(input.id ?? "").normalize("NFC")); - if (!Number.isSafeInteger(id) || id <= 0) - return actionError("Missing word id"); + const parsedId = positiveBigInt(String(input.id ?? "").normalize("NFC")); + if (!parsedId) return actionError("Missing word id"); + const id = parsedId.toString(); const result = await peopleMutationService.execute( createPeopleMutationInvocation(staff, createCorrelationId()), "word-filter.update", diff --git a/src/actions/housekeeping-command.test.ts b/src/actions/housekeeping-command.test.ts index 5febc4fb..51038f4d 100644 --- a/src/actions/housekeeping-command.test.ts +++ b/src/actions/housekeeping-command.test.ts @@ -84,6 +84,13 @@ registerHousekeepingCommand({ ipAddress: commandContext.ipAddress, }, commandContext.correlationId, + input.value === "partial" + ? { + status: "partial", + external: "failed", + audit: "persisted", + } + : undefined, ); }, }); @@ -136,6 +143,28 @@ describe("executeHousekeepingCommand", () => { expect(sealRegistryMock).not.toHaveBeenCalled(); }); + it("preserves one returned partial completion and its correlation through the real action dispatcher", async () => { + const result = await executeHousekeepingCommand({ + commandId: "system.server-action.serializable", + input: { value: "partial" }, + }); + + expect(result).toMatchObject({ + ok: true, + data: { value: "partial" }, + completion: { + status: "partial", + external: "failed", + audit: "persisted", + }, + }); + expect(auditEntries).toHaveLength(1); + expect(auditEntries[0]).toMatchObject({ + outcome: "partial", + correlationId: result.correlationId, + }); + expect(() => JSON.stringify(result)).not.toThrow(); + }); it("strictly rejects spoofed server-owned metadata before execution", async () => { const result = await executeHousekeepingCommand({ commandId: "system.server-action.serializable", @@ -185,7 +214,7 @@ describe("executeHousekeepingCommand", () => { expect(commandExecutions).toEqual([]); }); - it("maps completed-operation audit failures to a typed partial result", async () => { + it("returns one truthful partial completion when outcome audit persistence fails", async () => { auditWriteMock.mockImplementation(async (entry: AuditEntry) => { if (entry.outcome === "success") { throw new Error("success audit unavailable"); @@ -200,13 +229,16 @@ describe("executeHousekeepingCommand", () => { expect(commandExecutions).toEqual(["changed"]); expect(result).toMatchObject({ - ok: false, - error: { - code: "INTERNAL", - messageKey: "errors.housekeeping.partial", + ok: true, + data: { value: "changed" }, + completion: { + status: "partial", + external: "not-required", + audit: "persisted", }, }); expect(auditEntries.map((entry) => entry.outcome)).toEqual(["partial"]); expect(auditEntries[0]?.correlationId).toBe(result.correlationId); + expect(() => JSON.stringify(result)).not.toThrow(); }); }); diff --git a/src/actions/housekeeping-command.ts b/src/actions/housekeeping-command.ts index ae456205..820adca5 100644 --- a/src/actions/housekeeping-command.ts +++ b/src/actions/housekeeping-command.ts @@ -1,10 +1,8 @@ "use server"; import "@/features/housekeeping/foundation/commands/bootstrap"; -import { AuditOutcomePersistenceError } from "@/features/housekeeping/foundation/commands/audit-envelope"; import { dispatchHousekeepingCommand } from "@/features/housekeeping/foundation/commands/dispatcher"; import { - fail, type HousekeepingResult, mapUnknownError, } from "@/features/housekeeping/foundation/contracts"; @@ -29,29 +27,6 @@ export async function executeHousekeepingCommand( (await rateLimit(key, attempts, windowMs)).ok, }); } catch (error) { - if ( - error instanceof AuditOutcomePersistenceError && - isHousekeepingResult(error.operationResult) - ) { - return fail( - "INTERNAL", - "errors.housekeeping.partial", - error.operationResult.correlationId, - ); - } return mapUnknownError(error); } } - -function isHousekeepingResult( - value: unknown, -): value is HousekeepingResult { - return ( - typeof value === "object" && - value !== null && - "ok" in value && - typeof (value as { ok?: unknown }).ok === "boolean" && - "correlationId" in value && - typeof (value as { correlationId?: unknown }).correlationId === "string" - ); -} diff --git a/src/actions/people-shared-wrappers.test.ts b/src/actions/people-shared-wrappers.test.ts index 62fd2a9d..6b7807f4 100644 --- a/src/actions/people-shared-wrappers.test.ts +++ b/src/actions/people-shared-wrappers.test.ts @@ -114,11 +114,22 @@ describe("legacy application and word-filter wrappers", () => { expect(execute).toHaveBeenCalledWith( expect.objectContaining({ expectedActorId: 1 }), "application.decide", - { applicationId: 9, decision: "dismiss" }, + { applicationId: "9", decision: "dismiss" }, ); expect(revalidatePath).toHaveBeenCalledWith("/admin/applications"); }); + it("preserves application and wordfilter IDs above Number.MAX_SAFE_INTEGER", async () => { + await dismissApplication(form({ id: "9007199254740993" })); + await expect(deleteWord({ id: "9007199254740993" })).resolves.toEqual({ + ok: true, + data: {}, + }); + expect(execute.mock.calls.slice(-2).map((call) => call[2])).toEqual([ + { applicationId: "9007199254740993", decision: "dismiss" }, + { action: "delete", id: "9007199254740993" }, + ]); + }); it("preserves word-filter ActionResult shapes and /admin revalidation", async () => { await expect(addWord({ word: "spam" })).resolves.toEqual({ ok: true, @@ -130,7 +141,7 @@ describe("legacy application and word-filter wrappers", () => { }); expect(execute.mock.calls.slice(-2).map((call) => call[2])).toEqual([ { action: "add", word: "spam" }, - { action: "delete", id: 12 }, + { action: "delete", id: "12" }, ]); expect(revalidatePath).toHaveBeenCalledWith("/admin/wordfilter"); }); diff --git a/src/features/housekeeping/domains/people/commands/community-commands.test.ts b/src/features/housekeeping/domains/people/commands/community-commands.test.ts index ece5d357..028473d3 100644 --- a/src/features/housekeeping/domains/people/commands/community-commands.test.ts +++ b/src/features/housekeeping/domains/people/commands/community-commands.test.ts @@ -95,6 +95,50 @@ describe("People community and staff commands", () => { }, ); + it.each([ + [ + "people.application.decide", + { applicationId: "9007199254740993", decision: "dismiss" }, + ], + ["people.team.change", { action: "delete", teamId: "9007199254740993" }], + [ + "people.ip.action", + { action: "delete-blacklist", id: "9007199254740993" }, + ], + ["people.word-filter.update", { action: "delete", id: "9007199254740993" }], + ] as const)( + "accepts a canonical BIGINT identifier for %s", + (commandId, input) => { + const command = commands.find((entry) => entry.id === commandId); + if (!command) throw new Error(`command missing: ${commandId}`); + expect(command.input.safeParse(input).success).toBe(true); + }, + ); + it.each([ + [ + "people.application.decide", + { applicationId: "18446744073709551616", decision: "dismiss" }, + ], + [ + "people.team.change", + { action: "delete", teamId: "18446744073709551616" }, + ], + [ + "people.ip.action", + { action: "delete-blacklist", id: "18446744073709551616" }, + ], + [ + "people.word-filter.update", + { action: "delete", id: "18446744073709551616" }, + ], + ] as const)( + "rejects an overflowing BIGINT identifier for %s", + (commandId, input) => { + const command = commands.find((entry) => entry.id === commandId); + if (!command) throw new Error(`command missing: ${commandId}`); + expect(command.input.safeParse(input).success).toBe(false); + }, + ); it("delegates the canonical operation without a redirect", async () => { const execute = vi.fn(async (context, operation, input) => ({ ok: true as const, diff --git a/src/features/housekeeping/domains/people/commands/community-commands.ts b/src/features/housekeeping/domains/people/commands/community-commands.ts index 076968ef..37d976a9 100644 --- a/src/features/housekeeping/domains/people/commands/community-commands.ts +++ b/src/features/housekeeping/domains/people/commands/community-commands.ts @@ -53,6 +53,30 @@ function communityCommand( } const positiveId = z.number().int().positive(); +const MAX_UNSIGNED_BIGINT = "18446744073709551615"; + +function boundedDecimalPattern(maximum: string): RegExp { + const alternatives = [`[1-9]\\d{0,${maximum.length - 2}}`]; + for (let index = 0; index < maximum.length; index += 1) { + const maximumDigit = Number(maximum[index]); + const minimumDigit = index === 0 ? 1 : 0; + const upperDigit = maximumDigit - 1; + if (upperDigit < minimumDigit) continue; + const digit = + upperDigit === minimumDigit + ? String(minimumDigit) + : `[${minimumDigit}-${upperDigit}]`; + alternatives.push( + `${maximum.slice(0, index)}${digit}\\d{${maximum.length - index - 1}}`, + ); + } + alternatives.push(maximum); + return new RegExp(`^(?:${alternatives.join("|")})$`, "u"); +} + +const unsignedBigIntId = z + .string() + .regex(boundedDecimalPattern(MAX_UNSIGNED_BIGINT)); const requiredText = (max: number) => z.string().min(1).max(max).regex(/\S/u); export function createCommunityCommands( @@ -70,7 +94,7 @@ export function createCommunityCommands( operation: "application.decide", capability: PERMS.USERS_EDIT, input: z.object({ - applicationId: positiveId, + applicationId: unsignedBigIntId, decision: z.literal("dismiss"), }), }), @@ -87,7 +111,7 @@ export function createCommunityCommands( staffColor: z.string().min(1).max(255).optional(), hiddenRank: z.boolean().optional(), }), - z.object({ action: z.literal("delete"), teamId: positiveId }), + z.object({ action: z.literal("delete"), teamId: unsignedBigIntId }), ]), }), communityCommand(service, { @@ -102,7 +126,7 @@ export function createCommunityCommands( }), z.object({ action: z.enum(["delete-whitelist", "delete-blacklist"]), - id: positiveId, + id: unsignedBigIntId, }), ]), }), @@ -123,7 +147,7 @@ export function createCommunityCommands( capability: PERMS.WORDFILTER_EDIT, input: z.discriminatedUnion("action", [ z.object({ action: z.literal("add"), word: requiredText(255) }), - z.object({ action: z.literal("delete"), id: positiveId }), + z.object({ action: z.literal("delete"), id: unsignedBigIntId }), ]), }), ] as const; diff --git a/src/features/housekeeping/domains/people/models.ts b/src/features/housekeeping/domains/people/models.ts index 836800e6..954a801a 100644 --- a/src/features/housekeeping/domains/people/models.ts +++ b/src/features/housekeeping/domains/people/models.ts @@ -111,7 +111,7 @@ export interface PeopleGuildDetail extends PeopleGuildSummary { } export interface PeopleStaffApplication { - readonly id: number; + readonly id: string; readonly userId: number; readonly username: string | null; readonly rankId: number; @@ -120,9 +120,9 @@ export interface PeopleStaffApplication { } export interface PeopleTeam { - readonly id: number; + readonly id: string; readonly name: string; - readonly rank: number; + readonly rank: string; readonly hidden: boolean; readonly badge: string | null; readonly jobDescription: string | null; diff --git a/src/features/housekeeping/domains/people/pages/people-command-form.tsx b/src/features/housekeeping/domains/people/pages/people-command-form.tsx index 90010265..fc5993b0 100644 --- a/src/features/housekeeping/domains/people/pages/people-command-form.tsx +++ b/src/features/housekeeping/domains/people/pages/people-command-form.tsx @@ -18,9 +18,8 @@ export interface PeopleCommandField { }>[]; } -interface PeopleCommandFormProps { +export interface PeopleCommandSubmission { readonly commandId: string; - readonly buttonLabel: string; readonly input: Readonly>; readonly fields?: readonly PeopleCommandField[]; readonly requiresReason?: boolean; @@ -28,6 +27,10 @@ interface PeopleCommandFormProps { readonly nestFields?: boolean; } +interface PeopleCommandFormProps extends PeopleCommandSubmission { + readonly buttonLabel: string; +} + function parseField(field: PeopleCommandField, formData: FormData): unknown { if (field.type === "checkbox") return formData.get(field.name) === "on"; const raw = String(formData.get(field.name) ?? "") @@ -57,6 +60,78 @@ function parseField(field: PeopleCommandField, formData: FormData): unknown { const initialState: HousekeepingResult | null = null; +export async function submitPeopleCommandForm( + configuration: PeopleCommandSubmission, + _previous: HousekeepingResult | null, + formData: FormData, +): Promise> { + const fields = configuration.fields ?? []; + const dynamic = Object.fromEntries( + fields.map((field) => [field.name, parseField(field, formData)]), + ); + const reason = String(formData.get("reason") ?? "") + .normalize("NFC") + .trim() + .slice(0, 1000); + const commandInput = configuration.nestFields + ? { ...configuration.input, fields: dynamic } + : { + ...configuration.input, + ...dynamic, + ...(configuration.includeReasonInInput ? { reason } : {}), + }; + const { executeHousekeepingCommand } = await import( + "@/actions/housekeeping-command" + ); + return executeHousekeepingCommand({ + commandId: configuration.commandId, + input: commandInput, + ...(configuration.requiresReason ? { reason } : {}), + }); +} + +export function PeopleCommandResult({ + result, +}: { + readonly result: HousekeepingResult; +}) { + const generatedPassword = readGeneratedPassword(result); + const message = result.ok + ? result.completion?.status === "partial" + ? `Partially completed (${result.correlationId})` + : `Completed (${result.correlationId})` + : `Failed: ${result.error.messageKey} (${result.correlationId})`; + return ( +
+

{message}

+ {generatedPassword ? ( +

+ Generated temporary password:{" "} + + {generatedPassword} + +

+ ) : null} +
+ ); +} + +function readGeneratedPassword( + result: HousekeepingResult, +): string | null { + if (!result.ok || !isRecord(result.data)) return null; + const output = result.data.output; + if (!isRecord(output) || typeof output.newPassword !== "string") return null; + return output.newPassword.length > 0 ? output.newPassword : null; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + export function PeopleCommandForm({ commandId, buttonLabel, @@ -67,33 +142,14 @@ export function PeopleCommandForm({ nestFields = false, }: PeopleCommandFormProps) { const [result, submit, pending] = useActionState( - async ( - _previous: HousekeepingResult | null, - formData: FormData, - ) => { - const dynamic = Object.fromEntries( - fields.map((field) => [field.name, parseField(field, formData)]), - ); - const reason = String(formData.get("reason") ?? "") - .normalize("NFC") - .trim() - .slice(0, 1000); - const commandInput = nestFields - ? { ...input, fields: dynamic } - : { - ...input, - ...dynamic, - ...(includeReasonInInput ? { reason } : {}), - }; - const { executeHousekeepingCommand } = await import( - "@/actions/housekeeping-command" - ); - return executeHousekeepingCommand({ - commandId, - input: commandInput, - ...(requiresReason ? { reason } : {}), - }); - }, + submitPeopleCommandForm.bind(null, { + commandId, + input, + fields, + requiresReason, + includeReasonInInput, + nestFields, + }), initialState, ); @@ -168,13 +224,7 @@ export function PeopleCommandForm({ - {result ? ( -

- {result.ok - ? `Completed (${result.correlationId})` - : `Failed: ${result.error.messageKey} (${result.correlationId})`} -

- ) : null} + {result ? : null} ); } diff --git a/src/features/housekeeping/domains/people/pages/people-primary-pages.test.tsx b/src/features/housekeeping/domains/people/pages/people-primary-pages.test.tsx index 04672a5c..f6b72ae3 100644 --- a/src/features/housekeeping/domains/people/pages/people-primary-pages.test.tsx +++ b/src/features/housekeeping/domains/people/pages/people-primary-pages.test.tsx @@ -1,6 +1,7 @@ import { readFileSync } from "node:fs"; import { renderToStaticMarkup } from "react-dom/server"; import { describe, expect, it, vi } from "vitest"; +import { executeHousekeepingCommand } from "@/actions/housekeeping-command"; import { PERMS } from "@/lib/permission-slugs"; vi.mock("@/actions/housekeeping-command", () => ({ @@ -26,6 +27,10 @@ import { import { PeopleCommunityPage } from "./community"; import { PeopleMultiAccountsPage } from "./multi-accounts"; import { parsePeopleListInput } from "./page-state"; +import { + PeopleCommandResult, + submitPeopleCommandForm, +} from "./people-command-form"; import { PeopleStaffPage } from "./staff"; import { PeopleUserDetailPage } from "./user-detail"; import { PeopleUserEditPage } from "./user-edit"; @@ -215,7 +220,7 @@ const cases: readonly PageCase[] = [ page: { items: [ { - id: 5, + id: "5", userId: 7, username: "Alice", rankId: 4, @@ -342,17 +347,58 @@ describe("People URL list input", () => { }); }); describe("People actionable form contract", () => { - it("submits bounded command input and reason through the existing server action", () => { - const source = readFileSync( - "src/features/housekeeping/domains/people/pages/people-command-form.tsx", - "utf8", + it("submits bounded command input and reason through the actual form action adapter", async () => { + vi.mocked(executeHousekeepingCommand).mockResolvedValue( + ok({ before: null, after: { amount: 100 } }, "form-action"), ); - expect(source).toContain("executeHousekeepingCommand({"); - expect(source).toContain("commandId"); - expect(source).toContain("input: commandInput"); - expect(source).toContain("{ reason }"); - expect(source).toContain("action={submit}"); - expect(source).not.toContain(" { + const password = "one-time-secret"; + const html = renderToStaticMarkup( + , + ); + expect(html).toContain('aria-label="Generated temporary password"'); + expect(html.match(new RegExp(password, "gu"))).toHaveLength(1); }); it("provides a real Next loading boundary", () => { diff --git a/src/features/housekeeping/domains/people/queries/people-queries.test.ts b/src/features/housekeeping/domains/people/queries/people-queries.test.ts index a74d34a1..9b4fce76 100644 --- a/src/features/housekeeping/domains/people/queries/people-queries.test.ts +++ b/src/features/housekeeping/domains/people/queries/people-queries.test.ts @@ -427,7 +427,7 @@ describe("People community and staff queries", () => { loadApplications: async () => ({ rows: [ { - id: 11, + id: "9007199254740993", userId: 5, username: "Applicant", rankId: 3, @@ -456,7 +456,10 @@ describe("People community and staff queries", () => { expect(applications).toMatchObject({ ok: true, - data: { kind: "applications", page: { items: [{ id: 11 }] } }, + data: { + kind: "applications", + page: { items: [{ id: "9007199254740993" }] }, + }, }); expect(teams).toMatchObject({ ok: true, data: { kind: "teams" } }); expect(moderationTeam).toMatchObject({ diff --git a/src/features/housekeeping/domains/people/queries/staff.ts b/src/features/housekeeping/domains/people/queries/staff.ts index f579300e..43c4bdf1 100644 --- a/src/features/housekeeping/domains/people/queries/staff.ts +++ b/src/features/housekeeping/domains/people/queries/staff.ts @@ -169,7 +169,7 @@ const peopleStaffAdapters: PeopleStaffAdapters = { content: string; createdAt: Date | string | null; }>(rowsResult).map((row) => ({ - id: Number(row.id), + id: String(row.id), userId: Number(row.userId), username: row.username, rankId: Number(row.rankId), @@ -210,9 +210,9 @@ const peopleStaffAdapters: PeopleStaffAdapters = { badge: string | null; jobDescription: string | null; }>(rowsResult).map((row) => ({ - id: Number(row.id), + id: String(row.id), name: row.name, - rank: Number(row.id), + rank: String(row.id), hidden: Boolean(row.hidden), badge: row.badge, jobDescription: row.jobDescription, diff --git a/src/features/housekeeping/domains/people/services/mutations-audit-contract.test.ts b/src/features/housekeeping/domains/people/services/mutations-audit-contract.test.ts index c6fba165..a8765021 100644 --- a/src/features/housekeeping/domains/people/services/mutations-audit-contract.test.ts +++ b/src/features/housekeeping/domains/people/services/mutations-audit-contract.test.ts @@ -3,34 +3,25 @@ import { PERMS } from "@/lib/permission-slugs"; import type { AuditEntry } from "@/lib/services/audit"; import type { HousekeepingCapabilityContext } from "../../../foundation/contracts"; -const { alertUser, audit, resolveServerContext, selectLimit } = vi.hoisted( - () => ({ - alertUser: vi.fn(), - audit: vi.fn(), - resolveServerContext: vi.fn(), - selectLimit: vi.fn(), - }), -); +const { alertUser, audit, resolveServerContext } = vi.hoisted(() => ({ + alertUser: vi.fn(), + audit: vi.fn(), + resolveServerContext: vi.fn(), +})); vi.mock("@/features/housekeeping/foundation/server-capability-context", () => ({ getHousekeepingCapabilityContext: resolveServerContext, })); vi.mock("@/lib/auth", () => ({ invalidateLoginCache: vi.fn() })); vi.mock("@/lib/auth/password", () => ({ hashPassword: vi.fn() })); -vi.mock("@/lib/db", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - db: { - ...actual.db, - select: vi.fn(() => ({ - from: vi.fn(() => ({ - where: vi.fn(() => ({ limit: selectLimit })), - })), - })), - }, - }; -}); +vi.mock("@/lib/db", async (importOriginal) => ({ + ...(await importOriginal()), + db: { + select: vi.fn(() => { + throw new Error("alert must not query DB"); + }), + }, +})); vi.mock("@/lib/services/audit", async (importOriginal) => ({ ...(await importOriginal()), logAudit: audit, @@ -61,17 +52,6 @@ const invocation = { beforeEach(() => { vi.clearAllMocks(); resolveServerContext.mockResolvedValue(context()); - selectLimit.mockResolvedValue([ - { - id: 7, - username: "Alice", - rank: 2, - motto: "Ready", - credits: 100, - pixels: 50, - online: "1", - }, - ]); alertUser.mockResolvedValue(true); audit.mockResolvedValue(undefined); }); @@ -93,7 +73,7 @@ describe("People external audit contract", () => { ); }); - it("persists a correlated failure outcome when the external operation fails", async () => { + it("persists a correlated failure outcome when a pre-mutation external operation fails", async () => { alertUser.mockResolvedValue(false); const result = await peopleMutationService.execute( @@ -120,24 +100,51 @@ describe("People external audit contract", () => { ]); }); - it("throws typed completed-operation evidence when final outcome persistence fails", async () => { + it("returns serializable partial completion when final outcome persistence fails", async () => { audit.mockImplementation(async (entry: AuditEntry) => { if (entry.outcome === "success") throw new Error("outcome unavailable"); }); - await expect( - peopleMutationService.execute(invocation, "user.alert", { - userId: 7, - message: "Hello", - }), - ).rejects.toMatchObject({ - name: "AuditOutcomePersistenceError", - operationCompleted: true, - operationResult: { - before: expect.objectContaining({ id: 7 }), - after: expect.objectContaining({ alertDelivered: true }), + const result = await peopleMutationService.execute( + invocation, + "user.alert", + { userId: 7, message: "Hello" }, + ); + + expect(result).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "completed", + audit: "persisted", + }, + data: { + before: null, + after: { userId: 7, alertDelivered: true }, + }, + correlationId: "audit-contract", + }); + expect(() => JSON.stringify(result)).not.toThrow(); + expect(alertUser).toHaveBeenCalledTimes(1); + expect( + audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome), + ).toEqual(["intent", "success", "partial"]); + }); + + it("attempts legacy alert without a target lookup when the database is unavailable", async () => { + const result = await peopleMutationService.execute( + { ...invocation, legacy: true }, + "user.alert", + { userId: 900719925, message: "Direct RCON" }, + ); + + expect(result).toMatchObject({ + ok: true, + data: { + before: null, + after: { userId: 900719925, alertDelivered: true }, }, }); - expect(alertUser).toHaveBeenCalledTimes(1); + expect(alertUser).toHaveBeenCalledWith(900719925, "Direct RCON"); }); }); diff --git a/src/features/housekeeping/domains/people/services/mutations-production-workflows.test.ts b/src/features/housekeeping/domains/people/services/mutations-production-workflows.test.ts index b3383c43..79001aec 100644 --- a/src/features/housekeeping/domains/people/services/mutations-production-workflows.test.ts +++ b/src/features/housekeeping/domains/people/services/mutations-production-workflows.test.ts @@ -5,8 +5,11 @@ import type { HousekeepingCapabilityContext } from "../../../foundation/contract const mocks = vi.hoisted(() => ({ audit: vi.fn(), deleteWhere: vi.fn(), + hashPassword: vi.fn(), + invalidateLoginCache: vi.fn(), insertValues: vi.fn(), logStaffActivity: vi.fn(), + notify: vi.fn(), rcon: { alertUser: vi.fn(), disconnectUser: vi.fn(), @@ -14,7 +17,9 @@ const mocks = vi.hoisted(() => ({ giveCredits: vi.fn(), giveDuckets: vi.fn(), givePointsGotw: vi.fn(), + muteUser: vi.fn(), setTradeLock: vi.fn(), + unmuteUser: vi.fn(), updateWordFilter: vi.fn(), }, reloadSettings: vi.fn(), @@ -22,6 +27,7 @@ const mocks = vi.hoisted(() => ({ resolveServerContext: vi.fn(), selectQueue: [] as unknown[][], transaction: vi.fn(), + updateSet: vi.fn(), updateWhere: vi.fn(), })); @@ -56,7 +62,7 @@ function databaseFacade() { values: mocks.insertValues.mockImplementation(() => mutationPromise()), })), update: vi.fn(() => ({ - set: vi.fn(() => ({ + set: mocks.updateSet.mockImplementation(() => ({ where: mocks.updateWhere.mockResolvedValue([{ affectedRows: 1 }]), })), })), @@ -70,8 +76,12 @@ function databaseFacade() { vi.mock("@/features/housekeeping/foundation/server-capability-context", () => ({ getHousekeepingCapabilityContext: mocks.resolveServerContext, })); -vi.mock("@/lib/auth", () => ({ invalidateLoginCache: vi.fn() })); -vi.mock("@/lib/auth/password", () => ({ hashPassword: vi.fn() })); +vi.mock("@/lib/auth", () => ({ + invalidateLoginCache: mocks.invalidateLoginCache, +})); +vi.mock("@/lib/auth/password", () => ({ + hashPassword: mocks.hashPassword, +})); vi.mock("@/lib/db", async (importOriginal) => { const actual = await importOriginal(); const tx = databaseFacade(); @@ -112,7 +122,7 @@ vi.mock("@/lib/services/site-settings", () => ({ vi.mock("@/lib/services/staff-activity", () => ({ logStaffActivity: mocks.logStaffActivity, })); -vi.mock("@/lib/services/webhook", () => ({ notify: vi.fn() })); +vi.mock("@/lib/services/webhook", () => ({ notify: mocks.notify })); import { peopleMutationService } from "./mutations"; @@ -132,11 +142,25 @@ const invocation = { legacy: true, }; +function target(overrides: Record = {}) { + return { + id: 7, + username: "Alice", + rank: 2, + motto: "Original", + credits: 10, + pixels: 20, + online: "1", + ...overrides, + }; +} + beforeEach(() => { vi.clearAllMocks(); mocks.selectQueue.length = 0; mocks.resolveServerContext.mockResolvedValue(context()); mocks.audit.mockResolvedValue(undefined); + mocks.hashPassword.mockResolvedValue("hashed-password"); mocks.reloadSettings.mockResolvedValue(undefined); for (const call of Object.values(mocks.rcon)) call.mockResolvedValue(true); }); @@ -197,6 +221,37 @@ describe("People production workflow adapter", () => { ); }); + it("keeps absent trade settings null before and after a no-op settings update", async () => { + mocks.selectQueue.push( + [ + { + id: 7, + username: "Alice", + rank: 2, + motto: "", + credits: 0, + pixels: 0, + online: "0", + }, + ], + [], + [], + ); + + const result = await peopleMutationService.execute( + invocation, + "user.trade-lock", + { userId: 7, untilUnix: 200 }, + ); + + expect(result).toMatchObject({ + ok: true, + data: { + before: { canTrade: null, tradelockAmount: null }, + after: { canTrade: null, tradelockAmount: null }, + }, + }); + }); it("runs guild, application, team, and IP database workflows transactionally", async () => { mocks.selectQueue.push([{ id: 9, name: "Builders", userId: 7 }], []); await expect( @@ -246,6 +301,70 @@ describe("People production workflow adapter", () => { ).toBeGreaterThanOrEqual(4); }); + it("preserves BIGINT identifiers as canonical strings across application, team, IP, and wordfilter", async () => { + const id = "9007199254740993"; + const databaseId = 9007199254740993n; + + mocks.selectQueue.push([ + { id: databaseId, userId: 7, rankId: 4, content: "Ready" }, + ]); + const application = await peopleMutationService.execute( + invocation, + "application.decide", + { applicationId: id, decision: "dismiss" }, + ); + expect(application).toMatchObject({ + ok: true, + data: { before: { id }, after: null }, + }); + + mocks.selectQueue.push([ + { + id: databaseId, + rankName: "MOD", + badge: null, + jobDescription: null, + hiddenRank: false, + }, + ]); + const team = await peopleMutationService.execute( + invocation, + "team.change", + { action: "delete", teamId: id }, + ); + expect(team).toMatchObject({ + ok: true, + data: { before: { id }, after: null }, + }); + + mocks.selectQueue.push([ + { id: databaseId, ipAddress: "198.51.100.3", asn: null }, + ]); + const ip = await peopleMutationService.execute(invocation, "ip.action", { + action: "delete-blacklist", + id, + }); + expect(ip).toMatchObject({ + ok: true, + data: { before: { id }, after: null }, + }); + + mocks.selectQueue.push([{ id: databaseId, word: "spam" }]); + const wordfilter = await peopleMutationService.execute( + invocation, + "word-filter.update", + { action: "delete", id }, + ); + expect(wordfilter).toMatchObject({ + ok: true, + data: { before: { id }, after: null }, + }); + + for (const result of [application, team, ip, wordfilter]) { + expect(() => JSON.stringify(result)).not.toThrow(); + expect(JSON.stringify(result)).not.toContain("9007199254740992"); + } + }); it("keeps VPN secrets out of audit while reloading settings and logging legacy activity", async () => { const result = await peopleMutationService.execute( invocation, @@ -283,4 +402,420 @@ describe("People production workflow adapter", () => { expect(mocks.reloadWordFilter).toHaveBeenCalledTimes(1); expect(mocks.rcon.updateWordFilter).toHaveBeenCalledTimes(1); }); + + it("returns correlated partial completion when committed ban sync reports false", async () => { + mocks.selectQueue.push( + [ + { + id: 7, + username: "Alice", + rank: 2, + motto: "", + credits: 0, + pixels: 0, + online: "1", + }, + ], + [], + [ + { + id: 90, + banExpire: Math.floor(Date.now() / 1000) + 24 * 3600, + banReason: "Repeated abuse", + type: "account", + }, + ], + ); + mocks.rcon.disconnectUser.mockResolvedValue(false); + + const result = await peopleMutationService.execute( + { ...invocation, correlationId: "committed-ban-partial" }, + "user.ban", + { + userId: 7, + reason: "Repeated abuse", + duration: 24, + type: "account", + }, + ); + + expect(result).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "failed", + audit: "persisted", + }, + correlationId: "committed-ban-partial", + }); + expect(mocks.transaction).toHaveBeenCalledTimes(1); + expect( + mocks.audit.mock.calls.map((call) => ({ + outcome: (call[0] as AuditEntry).outcome, + correlationId: (call[0] as AuditEntry).correlationId, + })), + ).toEqual([ + { outcome: "intent", correlationId: "committed-ban-partial" }, + { outcome: "partial", correlationId: "committed-ban-partial" }, + ]); + }); + it("executes update, unban, and reset-password with transactional audit and one-time output", async () => { + mocks.selectQueue.push([target()]); + const updated = await peopleMutationService.execute( + invocation, + "user.update", + { userId: 7, fields: { motto: "Updated" } }, + ); + expect(updated).toMatchObject({ + ok: true, + data: { after: { motto: "Updated" } }, + }); + expect(mocks.updateSet).toHaveBeenCalledWith({ motto: "Updated" }); + expect(mocks.invalidateLoginCache).toHaveBeenCalledWith("Alice"); + + mocks.selectQueue.push( + [target()], + [ + { + id: 22, + banExpire: 0, + banReason: "Permanent active ban", + type: "account", + }, + ], + [], + ); + const unbanned = await peopleMutationService.execute( + invocation, + "user.unban", + { userId: 7 }, + ); + expect(unbanned).toMatchObject({ + ok: true, + data: { + before: { banned: true, banExpire: 0 }, + after: { banned: false }, + }, + }); + expect(mocks.deleteWhere).toHaveBeenCalled(); + expect(mocks.selectQueue).toHaveLength(0); + + mocks.selectQueue.push([target()]); + const reset = await peopleMutationService.execute( + invocation, + "user.reset-password", + { userId: 7, reason: "Owner verified identity" }, + ); + expect(reset).toMatchObject({ + ok: true, + data: { output: { newPassword: expect.any(String) } }, + }); + if (!reset.ok) throw new Error("Expected successful reset"); + const newPassword = String(reset.data.output?.newPassword); + expect(newPassword).toHaveLength(16); + expect(mocks.hashPassword).toHaveBeenCalledWith(newPassword); + expect(mocks.updateSet).toHaveBeenCalledWith({ + password: "hashed-password", + }); + const auditPayload = JSON.stringify( + mocks.audit.mock.calls.map((call) => call[0] as AuditEntry), + ); + expect(auditPayload).not.toContain(newPassword); + expect(auditPayload).not.toContain("hashed-password"); + }); + + it("executes disconnect, mute, unmute, and send-currency through audited RCON", async () => { + mocks.selectQueue.push([target()]); + await expect( + peopleMutationService.execute(invocation, "user.disconnect", { + userId: 7, + reason: "Stuck session", + }), + ).resolves.toMatchObject({ ok: true }); + expect(mocks.rcon.disconnectUser).toHaveBeenCalledWith(7); + + mocks.selectQueue.push([target()]); + await expect( + peopleMutationService.execute(invocation, "user.mute", { + userId: 7, + duration: 30, + reason: "Chat abuse", + }), + ).resolves.toMatchObject({ ok: true }); + expect(mocks.rcon.muteUser).toHaveBeenCalledWith(7, 30); + + mocks.selectQueue.push([target()]); + await expect( + peopleMutationService.execute(invocation, "user.unmute", { + userId: 7, + reason: "Appeal accepted", + }), + ).resolves.toMatchObject({ ok: true }); + expect(mocks.rcon.unmuteUser).toHaveBeenCalledWith(7); + + mocks.selectQueue.push([target()]); + await expect( + peopleMutationService.execute(invocation, "user.send-currency", { + userId: 7, + amount: 25, + reason: "Event award", + }), + ).resolves.toMatchObject({ ok: true }); + expect(mocks.rcon.giveCredits).toHaveBeenCalledWith(7, 25); + + const outcomes = mocks.audit.mock.calls.map( + (call) => (call[0] as AuditEntry).outcome, + ); + expect(outcomes).toEqual([ + "intent", + "success", + "intent", + "success", + "intent", + "success", + "intent", + "success", + ]); + }); + + it("executes bulk ban, currency, and badge with database, RCON, and legacy activity outcomes", async () => { + await expect( + peopleMutationService.execute(invocation, "users.bulk-ban", { + userIds: [7, 8], + duration: 3600, + reason: "Coordinated abuse", + }), + ).resolves.toMatchObject({ + ok: true, + data: { after: { completed: 2, total: 2 } }, + }); + expect(mocks.insertValues).toHaveBeenCalledTimes(2); + + await expect( + peopleMutationService.execute(invocation, "users.bulk-currency", { + userIds: [7], + type: "credits", + amount: 50, + }), + ).resolves.toMatchObject({ + ok: true, + data: { after: { completed: 1, total: 1 } }, + }); + expect(mocks.rcon.giveCredits).toHaveBeenCalledWith(7, 50); + + mocks.selectQueue.push([], [{ maxSlot: 2 }]); + await expect( + peopleMutationService.execute(invocation, "users.bulk-badge", { + userIds: [7], + badgeCode: "ADM", + }), + ).resolves.toMatchObject({ + ok: true, + data: { after: { completed: 1, total: 1 } }, + }); + expect(mocks.rcon.giveBadge).toHaveBeenCalledWith(7, "ADM"); + expect(mocks.logStaffActivity).toHaveBeenCalledWith( + expect.objectContaining({ action: "bulk_ban" }), + ); + expect(mocks.logStaffActivity).toHaveBeenCalledWith( + expect.objectContaining({ action: "bulk_give_currency" }), + ); + expect(mocks.logStaffActivity).toHaveBeenCalledWith( + expect.objectContaining({ action: "bulk_give_badge" }), + ); + }); + + it("reports bulk currency as partial after database commit when RCON reports false", async () => { + mocks.rcon.giveCredits.mockResolvedValue(false); + const result = await peopleMutationService.execute( + { ...invocation, correlationId: "bulk-currency-partial", legacy: false }, + "users.bulk-currency", + { userIds: [7], type: "credits", amount: 50 }, + ); + + expect(result).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "failed", + audit: "persisted", + }, + data: { + after: { + completed: 0, + total: 1, + failedIds: [{ userId: 7, reason: "Database error" }], + }, + }, + correlationId: "bulk-currency-partial", + }); + expect( + mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome), + ).toEqual(["intent", "partial"]); + }); + it("returns typed partial after committed update or reset when notification fails", async () => { + mocks.notify.mockRejectedValue(new Error("notification unavailable")); + mocks.selectQueue.push([target()]); + const updated = await peopleMutationService.execute( + { ...invocation, correlationId: "update-notify-partial", legacy: false }, + "user.update", + { userId: 7, fields: { motto: "Updated" } }, + ); + expect(updated).toMatchObject({ + ok: true, + 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"]); + + 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 }, + "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", + }); + expect( + mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome), + ).toEqual(["intent", "partial"]); + }); + + it("preserves legacy throw and false semantics while commands retain partial truth", async () => { + mocks.selectQueue.push( + [target()], + [], + [ + { + id: 91, + banExpire: Math.floor(Date.now() / 1000) + 3600, + banReason: "Repeated abuse", + type: "account", + }, + ], + ); + mocks.rcon.disconnectUser.mockRejectedValue(new Error("RCON unavailable")); + await expect( + peopleMutationService.execute(invocation, "user.ban", { + userId: 7, + reason: "Repeated abuse", + duration: 1, + type: "account", + }), + ).resolves.toMatchObject({ + ok: false, + error: { code: "DEPENDENCY_UNAVAILABLE" }, + }); + + vi.clearAllMocks(); + mocks.resolveServerContext.mockResolvedValue(context()); + mocks.audit.mockResolvedValue(undefined); + mocks.rcon.giveCredits.mockResolvedValue(false); + const legacyBulk = await peopleMutationService.execute( + invocation, + "users.bulk-currency", + { userIds: [7], type: "credits", amount: 50 }, + ); + expect(legacyBulk).toMatchObject({ + ok: true, + data: { + after: { completed: 1, total: 1, failedIds: [] }, + }, + }); + expect(legacyBulk).not.toHaveProperty("completion"); + }); + it("reports trade, VPN, and wordfilter post-commit sync failures as typed partial", async () => { + mocks.selectQueue.push([target()], [], []); + mocks.rcon.setTradeLock.mockResolvedValue(false); + const trade = await peopleMutationService.execute( + { ...invocation, correlationId: "trade-sync-partial", legacy: false }, + "user.trade-lock", + { userId: 7, untilUnix: 200 }, + ); + expect(trade).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "failed", + audit: "persisted", + }, + correlationId: "trade-sync-partial", + }); + expect( + mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome), + ).toEqual(["intent", "partial"]); + + vi.clearAllMocks(); + mocks.resolveServerContext.mockResolvedValue(context()); + mocks.audit.mockResolvedValue(undefined); + mocks.reloadSettings.mockRejectedValue(new Error("cache unavailable")); + const vpn = await peopleMutationService.execute( + { ...invocation, correlationId: "vpn-cache-partial", legacy: false }, + "vpn.configure", + { + enabled: true, + provider: "proxycheck", + apiKey: "new-secret", + blockMessage: "No VPN", + }, + ); + expect(vpn).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "failed", + audit: "persisted", + }, + correlationId: "vpn-cache-partial", + }); + + vi.clearAllMocks(); + mocks.resolveServerContext.mockResolvedValue(context()); + mocks.audit.mockImplementation(async (entry: AuditEntry) => { + if (entry.outcome === "partial") throw new Error("audit unavailable"); + }); + mocks.reloadWordFilter.mockResolvedValue(undefined); + mocks.rcon.updateWordFilter.mockResolvedValue(false); + mocks.selectQueue.push([]); + const wordfilter = await peopleMutationService.execute( + { + ...invocation, + correlationId: "wordfilter-sync-partial", + legacy: false, + }, + "word-filter.update", + { action: "delete", id: "99" }, + ); + expect(wordfilter).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "failed", + audit: "unavailable", + }, + correlationId: "wordfilter-sync-partial", + }); + expect( + mocks.audit.mock.calls.map((call) => (call[0] as AuditEntry).outcome), + ).toEqual(["intent", "partial"]); + }); }); diff --git a/src/features/housekeeping/domains/people/services/mutations-production.test.ts b/src/features/housekeeping/domains/people/services/mutations-production.test.ts index 7e43b7db..a113203f 100644 --- a/src/features/housekeeping/domains/people/services/mutations-production.test.ts +++ b/src/features/housekeeping/domains/people/services/mutations-production.test.ts @@ -99,14 +99,14 @@ describe("People production mutation boundary", () => { expect(result).toMatchObject({ ok: true, data: { - before: { id: 7, username: "Alice", online: true }, - after: { id: 7, alertDelivered: true }, + before: null, + after: { userId: 7, alertDelivered: true }, }, }); expect(audit).toHaveBeenCalledWith( expect.objectContaining({ action: "people.user.alert", - before: expect.objectContaining({ id: 7 }), + before: undefined, after: expect.objectContaining({ alertDelivered: true }), correlationId: "production-alert", outcome: "success", diff --git a/src/features/housekeeping/domains/people/services/mutations-reason-production.test.ts b/src/features/housekeeping/domains/people/services/mutations-reason-production.test.ts index 2a0da228..39cd8aed 100644 --- a/src/features/housekeeping/domains/people/services/mutations-reason-production.test.ts +++ b/src/features/housekeeping/domains/people/services/mutations-reason-production.test.ts @@ -10,6 +10,7 @@ const { selectLimit, transaction, txSelectLimit, + txSelectOrderBy, } = vi.hoisted(() => ({ audit: vi.fn(), disconnectUser: vi.fn(), @@ -18,6 +19,7 @@ const { selectLimit: vi.fn(), transaction: vi.fn(), txSelectLimit: vi.fn(), + txSelectOrderBy: vi.fn(), })); vi.mock("@/features/housekeeping/foundation/server-capability-context", () => ({ @@ -30,7 +32,7 @@ vi.mock("@/lib/db", async (importOriginal) => { const tx = { select: vi.fn(() => ({ from: vi.fn(() => ({ - where: vi.fn(() => ({ limit: txSelectLimit })), + where: vi.fn(() => ({ orderBy: txSelectOrderBy })), })), })), insert: vi.fn(() => ({ values: insertValues })), @@ -87,7 +89,24 @@ beforeEach(() => { online: "1", }, ]); - txSelectLimit.mockResolvedValue([]); + txSelectLimit + .mockResolvedValueOnce([ + { + id: 88, + banExpire: 0, + banReason: "Permanent active ban", + type: "account", + }, + ]) + .mockResolvedValue([ + { + id: 89, + banExpire: Math.floor(Date.now() / 1000) + 24 * 3600, + banReason: "Repeated harassment", + type: "account", + }, + ]); + txSelectOrderBy.mockReturnValue({ limit: txSelectLimit }); insertValues.mockResolvedValue([{}]); disconnectUser.mockResolvedValue(true); audit.mockResolvedValue(undefined); @@ -112,7 +131,12 @@ describe("People production reason audit", () => { expect.objectContaining({ action: "people.user.ban", outcome: "intent", - before: expect.objectContaining({ id: 7, banned: false }), + before: expect.objectContaining({ + id: 7, + banned: true, + banExpire: 0, + reason: "Permanent active ban", + }), after: expect.objectContaining({ id: 7, banned: true, @@ -121,6 +145,7 @@ describe("People production reason audit", () => { }), expect.anything(), ); + expect(txSelectOrderBy).toHaveBeenCalledTimes(2); expect(audit.mock.calls[0]?.[0]?.before).not.toHaveProperty("mail"); expect(audit.mock.calls[0]?.[0]?.after).not.toHaveProperty("mail"); }); diff --git a/src/features/housekeeping/domains/people/services/mutations-transactional-audit.test.ts b/src/features/housekeeping/domains/people/services/mutations-transactional-audit.test.ts index 2c9ec22d..ad6bb48c 100644 --- a/src/features/housekeeping/domains/people/services/mutations-transactional-audit.test.ts +++ b/src/features/housekeeping/domains/people/services/mutations-transactional-audit.test.ts @@ -109,7 +109,7 @@ describe("People transactional database audit", () => { expect(audit).toHaveBeenCalledWith( expect.objectContaining({ outcome: "success", - before: expect.objectContaining({ id: 9 }), + before: expect.objectContaining({ id: "9" }), after: undefined, }), expect.objectContaining({ insert: expect.any(Function) }), diff --git a/src/features/housekeeping/domains/people/services/mutations.ts b/src/features/housekeeping/domains/people/services/mutations.ts index 0c32cba4..19b6fc29 100644 --- a/src/features/housekeeping/domains/people/services/mutations.ts +++ b/src/features/housekeeping/domains/people/services/mutations.ts @@ -1,7 +1,7 @@ import "server-only"; import crypto from "node:crypto"; -import { and, eq, inArray, max, sql } from "drizzle-orm"; +import { and, desc, eq, gt, inArray, max, or, sql } from "drizzle-orm"; import type { ResultSetHeader } from "mysql2"; import { AuditOutcomePersistenceError } from "@/features/housekeeping/foundation/commands/audit-envelope"; import { invalidateLoginCache } from "@/lib/auth"; @@ -41,6 +41,7 @@ import { fail, type HousekeepingCapabilityContext, type HousekeepingErrorCode, + type HousekeepingPartialCompletion, type HousekeepingResult, ok, } from "../../../foundation/contracts"; @@ -72,6 +73,7 @@ export interface PeopleMutationSnapshot { readonly before: Readonly> | null; readonly after: Readonly> | null; readonly output?: Readonly>; + readonly completion?: HousekeepingPartialCompletion; } export interface PeopleMutationInvocation { @@ -164,16 +166,22 @@ export function createPeopleMutationService( legacy: invocation.legacy === true, }; try { - return ok( - await adapter.execute(operation, input, context), - invocation.correlationId, - ); + const snapshot = await adapter.execute(operation, input, context); + return ok(snapshot, invocation.correlationId, snapshot.completion); } catch (error) { if (error instanceof AuditOutcomePersistenceError) { - if (invocation.legacy) { - return ok(error.operationResult, invocation.correlationId); - } - throw error; + const operationSnapshot = + error.operationResult as PeopleMutationSnapshot; + const completion: HousekeepingPartialCompletion = { + status: "partial", + external: operationSnapshot.completion?.external ?? "completed", + audit: "unavailable", + }; + const snapshot = { + ...operationSnapshot, + completion, + }; + return ok(snapshot, invocation.correlationId, completion); } if (error instanceof PeopleMutationFailure) { return fail( @@ -221,6 +229,43 @@ function positiveInteger(value: unknown): number { return parsed; } +const MAX_UNSIGNED_BIGINT = 18_446_744_073_709_551_615n; +const MAX_AUDIT_TARGET_ID = 2_147_483_647n; + +function positiveBigIntIdentifier(value: unknown): bigint { + let parsed: bigint; + if (typeof value === "bigint") { + parsed = value; + } else if (typeof value === "string" && /^[1-9]\d{0,19}$/u.test(value)) { + parsed = BigInt(value); + } else if ( + typeof value === "number" && + Number.isSafeInteger(value) && + value > 0 + ) { + parsed = BigInt(value); + } else { + throw new PeopleMutationFailure( + "VALIDATION", + "errors.housekeeping.validation", + ); + } + if (parsed <= 0n || parsed > MAX_UNSIGNED_BIGINT) { + throw new PeopleMutationFailure( + "VALIDATION", + "errors.housekeeping.validation", + ); + } + return parsed; +} + +function serializeBigIntIdentifier(value: unknown): string { + return positiveBigIntIdentifier(value).toString(); +} + +function auditTargetId(value: bigint): number | undefined { + return value <= MAX_AUDIT_TARGET_ID ? Number(value) : undefined; +} function nonNegativeInteger(value: unknown): number { const parsed = Number(value); if (!Number.isSafeInteger(parsed) || parsed < 0) { @@ -356,10 +401,12 @@ async function finalizeExternalWithAudit( targetId: number | undefined, snapshot: PeopleMutationSnapshot, execute: () => Promise, + mutationCommitted = true, ): Promise { try { await execute(); } catch (error) { + let audit: HousekeepingPartialCompletion["audit"] = "persisted"; try { await logAudit( canonicalAuditEntry( @@ -368,13 +415,23 @@ async function finalizeExternalWithAudit( target, targetId, snapshot, - "failure", + mutationCommitted ? "partial" : "failure", ), ); } catch { - // Preserve the external failure as the authoritative result. + audit = "unavailable"; } - throw error; + if ( + !mutationCommitted || + (context.legacy && !(error instanceof PeopleMutationFailure)) + ) { + throw error; + } + return withPartialCompletion(snapshot, { + status: "partial", + external: "failed", + audit, + }); } try { await logAudit( @@ -387,7 +444,9 @@ async function finalizeExternalWithAudit( "success", ), ); - } catch (auditOutcomeError) { + return snapshot; + } catch { + let audit: HousekeepingPartialCompletion["audit"] = "persisted"; try { await logAudit( canonicalAuditEntry( @@ -400,13 +459,22 @@ async function finalizeExternalWithAudit( ), ); } catch { - // The typed completed-operation error remains primary. + audit = "unavailable"; } - throw new AuditOutcomePersistenceError(snapshot, auditOutcomeError); + return withPartialCompletion(snapshot, { + status: "partial", + external: "completed", + audit, + }); } - return snapshot; } +function withPartialCompletion( + snapshot: PeopleMutationSnapshot, + completion: HousekeepingPartialCompletion, +): PeopleMutationSnapshot { + return { ...snapshot, completion }; +} async function runExternalWithAudit( context: PeopleMutationContext, operation: PeopleMutationOperation, @@ -432,6 +500,7 @@ async function runExternalWithAudit( targetId, snapshot, execute, + false, ); } async function executeUserMutation( @@ -441,8 +510,24 @@ async function executeUserMutation( ): Promise { const data = record(input); const userId = positiveInteger(data.userId); - const guardHierarchy = - operation !== "user.alert" && operation !== "user.trade-lock"; + + if (operation === "user.alert") { + const message = normalizedText(data.message, 500); + const snapshot: PeopleMutationSnapshot = { + before: null, + after: { userId, alertDelivered: true }, + }; + return runExternalWithAudit( + context, + operation, + "User", + userId, + snapshot, + async () => requireRcon(await rcon.alertUser(userId, message)), + ); + } + + const guardHierarchy = operation !== "user.trade-lock"; const target = await loadTarget(userId, context, guardHierarchy); const before = targetSnapshot(target); @@ -494,19 +579,28 @@ async function executeUserMutation( "User", userId, snapshot, - "success", + "intent", ), tx, ); }); - invalidateLoginCache(target.username); - await notify({ - action: "user_edit", - actor: context.capability.actor.username, - target: target.username, - targetId: userId, - }); - return snapshot; + return finalizeExternalWithAudit( + context, + operation, + "User", + userId, + snapshot, + async () => { + invalidateLoginCache(target.username); + const notification = notify({ + action: "user_edit", + actor: context.capability.actor.username, + target: target.username, + targetId: userId, + }); + if (!context.legacy) await notification; + }, + ); } if (operation === "user.ban") { @@ -532,7 +626,13 @@ async function executeUserMutation( type: Ban.type, }) .from(Ban) - .where(eq(Ban.userId, userId)) + .where( + and( + eq(Ban.userId, userId), + or(eq(Ban.banExpire, 0), gt(Ban.banExpire, now)), + ), + ) + .orderBy(desc(Ban.timestamp), desc(Ban.id)) .limit(1); await tx.insert(Ban).values({ userId, @@ -544,6 +644,22 @@ async function executeUserMutation( ip: normalizedText(data.ip, 255, false), machineId: "", }); + const [observedAfterBan] = await tx + .select({ + id: Ban.id, + banExpire: Ban.banExpire, + banReason: Ban.banReason, + type: Ban.type, + }) + .from(Ban) + .where( + and( + eq(Ban.userId, userId), + or(eq(Ban.banExpire, 0), gt(Ban.banExpire, now)), + ), + ) + .orderBy(desc(Ban.timestamp), desc(Ban.id)) + .limit(1); snapshot = { before: { ...before, @@ -556,7 +672,17 @@ async function executeUserMutation( } : {}), }, - after: { ...before, banned: true, banExpire, type, reason }, + after: { + ...before, + banned: observedAfterBan !== undefined, + ...(observedAfterBan + ? { + banExpire: observedAfterBan.banExpire, + type: observedAfterBan.type, + reason: observedAfterBan.banReason, + } + : {}), + }, }; await logAudit( canonicalAuditEntry( @@ -577,7 +703,7 @@ async function executeUserMutation( userId, snapshot, async () => { - await rcon.disconnectUser(userId); + await requireRcon(await rcon.disconnectUser(userId)); await notify({ action: "ban", actor: context.capability.actor.username, @@ -589,6 +715,7 @@ async function executeUserMutation( } if (operation === "user.unban") { + const now = Math.floor(Date.now() / 1000); const snapshot = await db.transaction(async (tx) => { const [activeBan] = await tx .select({ @@ -598,9 +725,31 @@ async function executeUserMutation( type: Ban.type, }) .from(Ban) - .where(eq(Ban.userId, userId)) + .where( + and( + eq(Ban.userId, userId), + or(eq(Ban.banExpire, 0), gt(Ban.banExpire, now)), + ), + ) + .orderBy(desc(Ban.timestamp), desc(Ban.id)) .limit(1); await tx.delete(Ban).where(eq(Ban.userId, userId)); + const [observedAfterBan] = await tx + .select({ + id: Ban.id, + banExpire: Ban.banExpire, + banReason: Ban.banReason, + type: Ban.type, + }) + .from(Ban) + .where( + and( + eq(Ban.userId, userId), + or(eq(Ban.banExpire, 0), gt(Ban.banExpire, now)), + ), + ) + .orderBy(desc(Ban.timestamp), desc(Ban.id)) + .limit(1); const value: PeopleMutationSnapshot = { before: { ...before, @@ -613,7 +762,17 @@ async function executeUserMutation( } : {}), }, - after: { ...before, banned: false }, + after: { + ...before, + banned: observedAfterBan !== undefined, + ...(observedAfterBan + ? { + banExpire: observedAfterBan.banExpire, + type: observedAfterBan.type, + reason: observedAfterBan.banReason, + } + : {}), + }, }; await logAudit( canonicalAuditEntry( @@ -636,19 +795,6 @@ async function executeUserMutation( return snapshot; } - if (operation === "user.alert") { - const message = normalizedText(data.message, 500); - const snapshot = { before, after: { ...before, alertDelivered: true } }; - return runExternalWithAudit( - context, - operation, - "User", - userId, - snapshot, - async () => requireRcon(await rcon.alertUser(userId, message)), - ); - } - if (operation === "user.disconnect") { const snapshot = { before, after: { ...before, online: false } }; return runExternalWithAudit( @@ -718,19 +864,28 @@ async function executeUserMutation( "User", userId, snapshot, - "success", + "intent", ), tx, ); }); - invalidateLoginCache(target.username); - await notify({ - action: "user_edit", - actor: context.capability.actor.username, - target: target.username, - details: "Password reset", - }); - return snapshot; + return finalizeExternalWithAudit( + context, + operation, + "User", + userId, + snapshot, + async () => { + invalidateLoginCache(target.username); + const notification = notify({ + action: "user_edit", + actor: context.capability.actor.username, + target: target.username, + details: "Password reset", + }); + if (!context.legacy) await notification; + }, + ); } if (operation === "user.send-currency") { @@ -801,8 +956,10 @@ async function executeUserMutation( after: { ...before, tradeLockedUntil: untilUnix, - canTrade: locked ? "0" : "1", - tradelockAmount: (settings?.tradelockAmount ?? 0) + (locked ? 1 : 0), + canTrade: settings ? (locked ? "0" : "1") : null, + tradelockAmount: settings + ? settings.tradelockAmount + (locked ? 1 : 0) + : null, }, output: { userId, untilUnix }, }; @@ -825,15 +982,17 @@ async function executeUserMutation( userId, snapshot, async () => { - await rcon.setTradeLock(userId, locked); - await rcon.alertUser( - userId, - locked - ? "Trading has been disabled by staff." - : "Trading has been re-enabled by staff.", + await requireRcon(await rcon.setTradeLock(userId, locked)); + await requireRcon( + await rcon.alertUser( + userId, + locked + ? "Trading has been disabled by staff." + : "Trading has been re-enabled by staff.", + ), ); if (target.online === "1") { - await rcon.disconnectUser(userId, target.username); + await requireRcon(await rcon.disconnectUser(userId, target.username)); } await logStaffActivity({ staffId: context.capability.actor.id, @@ -856,6 +1015,7 @@ async function executeBulkMutation( const ids = userIds(data.userIds, context.legacy); const failures: Array<{ userId: number; reason: string }> = []; let completed = 0; + let externalFailed = false; let mutationReason: string | undefined; if (operation === "users.bulk-unban") { @@ -966,13 +1126,16 @@ 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)); - await rcon.giveCredits(userId, amount); + databaseCommitted = true; + const delivered = await rcon.giveCredits(userId, amount); + if (!context.legacy) await requireRcon(delivered); } else { const currencyType = type === "pixels" ? 0 : 101; await db @@ -981,11 +1144,18 @@ async function executeBulkMutation( .onDuplicateKeyUpdate({ set: { amount: sql`${UsersCurrency.amount} + ${amount}` }, }); - if (type === "pixels") await rcon.giveDuckets(userId, amount); - else await rcon.givePointsGotw(userId, 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" }); } } @@ -998,6 +1168,7 @@ async function executeBulkMutation( } else { const badgeCode = normalizedText(data.badgeCode, 20); for (const userId of ids) { + let databaseCommitted = false; try { const [existing] = await db .select({ id: UsersBadges.id }) @@ -1019,10 +1190,13 @@ async function executeBulkMutation( slotId: (aggregate?.maxSlot ?? 0) + 1, badgeCode, }); - await rcon.giveBadge(userId, badgeCode); + databaseCommitted = 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" }); } } @@ -1034,6 +1208,14 @@ async function executeBulkMutation( }); } + const completion: HousekeepingPartialCompletion | undefined = + failures.length > 0 + ? { + status: "partial", + external: externalFailed ? "failed" : "not-required", + audit: "persisted", + } + : undefined; const snapshot: PeopleMutationSnapshot = { before: { userIds: ids }, after: { @@ -1042,6 +1224,7 @@ async function executeBulkMutation( failedIds: failures, ...(mutationReason ? { reason: mutationReason } : {}), }, + ...(completion === undefined ? {} : { completion }), }; try { await logAudit( @@ -1137,7 +1320,7 @@ async function executeApplicationDecision( context: PeopleMutationContext, ): Promise { const data = record(input); - const applicationId = positiveInteger(data.applicationId); + const applicationId = positiveBigIntIdentifier(data.applicationId); if (data.decision !== "dismiss") { throw new PeopleMutationFailure( "VALIDATION", @@ -1153,7 +1336,7 @@ async function executeApplicationDecision( content: WebsiteStaffApplications.content, }) .from(WebsiteStaffApplications) - .where(eq(WebsiteStaffApplications.id, BigInt(applicationId))) + .where(eq(WebsiteStaffApplications.id, applicationId)) .limit(1); if (!application) { throw new PeopleMutationFailure( @@ -1163,10 +1346,10 @@ async function executeApplicationDecision( } await tx .delete(WebsiteStaffApplications) - .where(eq(WebsiteStaffApplications.id, BigInt(applicationId))); + .where(eq(WebsiteStaffApplications.id, applicationId)); const snapshot = { before: { - id: Number(application.id), + id: serializeBigIntIdentifier(application.id), userId: application.userId, rankId: application.rankId, content: application.content, @@ -1178,7 +1361,7 @@ async function executeApplicationDecision( context, "application.decide", "StaffApplication", - applicationId, + auditTargetId(applicationId), snapshot, "success", ), @@ -1194,7 +1377,7 @@ async function executeTeamChange( ): Promise { const data = record(input); if (data.action === "delete") { - const teamId = positiveInteger(data.teamId); + const teamId = positiveBigIntIdentifier(data.teamId); return db.transaction(async (tx) => { const [team] = await tx .select({ @@ -1205,7 +1388,7 @@ async function executeTeamChange( hiddenRank: WebsiteTeams.hiddenRank, }) .from(WebsiteTeams) - .where(eq(WebsiteTeams.id, BigInt(teamId))) + .where(eq(WebsiteTeams.id, teamId)) .limit(1); if (!team) { throw new PeopleMutationFailure( @@ -1213,9 +1396,9 @@ async function executeTeamChange( "errors.housekeeping.notFound", ); } - await tx.delete(WebsiteTeams).where(eq(WebsiteTeams.id, BigInt(teamId))); + await tx.delete(WebsiteTeams).where(eq(WebsiteTeams.id, teamId)); const snapshot: PeopleMutationSnapshot = { - before: { ...team, id: Number(team.id) }, + before: { ...team, id: serializeBigIntIdentifier(team.id) }, after: null, }; await logAudit( @@ -1223,7 +1406,7 @@ async function executeTeamChange( context, "team.change", "Team", - teamId, + auditTargetId(teamId), snapshot, "success", ), @@ -1249,11 +1432,11 @@ async function executeTeamChange( createdAt: now, updatedAt: now, }); - const teamId = Number(result.insertId); + const teamId = positiveBigIntIdentifier(result.insertId); const snapshot: PeopleMutationSnapshot = { before: null, after: { - teamId, + teamId: teamId.toString(), rankName, badge: badge || null, jobDescription: jobDescription || null, @@ -1266,7 +1449,7 @@ async function executeTeamChange( context, "team.change", "Team", - teamId, + auditTargetId(teamId), snapshot, "success", ), @@ -1309,17 +1492,17 @@ async function executeIpAction( : await tx .insert(WebsiteIpWhitelist) .values({ ipAddress, asn, whitelistAsn: asn !== null }); - const id = Number(result.insertId); + const id = positiveBigIntIdentifier(result.insertId); const snapshot: PeopleMutationSnapshot = { before: null, - after: { id, action, ipAddress, asn }, + after: { id: id.toString(), action, ipAddress, asn }, }; await logAudit( canonicalAuditEntry( context, "ip.action", "IpRule", - id, + auditTargetId(id), snapshot, "success", ), @@ -1328,7 +1511,7 @@ async function executeIpAction( return snapshot; }); } - const id = positiveInteger(data.id); + const id = positiveBigIntIdentifier(data.id); return db.transaction(async (tx) => { const [existing] = blacklist ? await tx @@ -1338,7 +1521,7 @@ async function executeIpAction( asn: WebsiteIpBlacklist.asn, }) .from(WebsiteIpBlacklist) - .where(eq(WebsiteIpBlacklist.id, BigInt(id))) + .where(eq(WebsiteIpBlacklist.id, id)) .limit(1) : await tx .select({ @@ -1347,21 +1530,17 @@ async function executeIpAction( asn: WebsiteIpWhitelist.asn, }) .from(WebsiteIpWhitelist) - .where(eq(WebsiteIpWhitelist.id, BigInt(id))) + .where(eq(WebsiteIpWhitelist.id, id)) .limit(1); if (blacklist) { - await tx - .delete(WebsiteIpBlacklist) - .where(eq(WebsiteIpBlacklist.id, BigInt(id))); + await tx.delete(WebsiteIpBlacklist).where(eq(WebsiteIpBlacklist.id, id)); } else { - await tx - .delete(WebsiteIpWhitelist) - .where(eq(WebsiteIpWhitelist.id, BigInt(id))); + await tx.delete(WebsiteIpWhitelist).where(eq(WebsiteIpWhitelist.id, id)); } const snapshot: PeopleMutationSnapshot = { before: existing ? { - id: Number(existing.id), + id: serializeBigIntIdentifier(existing.id), action, ipAddress: existing.ipAddress, asn: existing.asn, @@ -1374,7 +1553,7 @@ async function executeIpAction( context, "ip.action", "IpRule", - id, + auditTargetId(id), snapshot, "success", ), @@ -1472,20 +1651,20 @@ async function executeWordFilterUpdate( const data = record(input); if (data.action === "add") { const word = normalizedText(data.word, 255); - let id = 0; + let id = 0n; let snapshot!: PeopleMutationSnapshot; await db.transaction(async (tx) => { const [result] = (await tx .insert(WebsiteWordfilter) .values({ word })) as unknown as [ResultSetHeader]; - id = Number(result.insertId); - snapshot = { before: null, after: { id, word } }; + id = positiveBigIntIdentifier(result.insertId); + snapshot = { before: null, after: { id: id.toString(), word } }; await logAudit( canonicalAuditEntry( context, "word-filter.update", "WordFilter", - id, + auditTargetId(id), snapshot, "intent", ), @@ -1496,11 +1675,11 @@ async function executeWordFilterUpdate( context, "word-filter.update", "WordFilter", - id, + auditTargetId(id), snapshot, async () => { reloadWordFilter(); - await rcon.updateWordFilter(); + await requireRcon(await rcon.updateWordFilter()); }, ); } @@ -1510,20 +1689,18 @@ async function executeWordFilterUpdate( "errors.housekeeping.validation", ); } - const id = positiveInteger(data.id); + const id = positiveBigIntIdentifier(data.id); let snapshot!: PeopleMutationSnapshot; await db.transaction(async (tx) => { const [existing] = await tx .select({ id: WebsiteWordfilter.id, word: WebsiteWordfilter.word }) .from(WebsiteWordfilter) - .where(eq(WebsiteWordfilter.id, BigInt(id))) + .where(eq(WebsiteWordfilter.id, id)) .limit(1); - await tx - .delete(WebsiteWordfilter) - .where(eq(WebsiteWordfilter.id, BigInt(id))); + await tx.delete(WebsiteWordfilter).where(eq(WebsiteWordfilter.id, id)); snapshot = { before: existing - ? { id: Number(existing.id), word: existing.word } + ? { id: serializeBigIntIdentifier(existing.id), word: existing.word } : null, after: null, }; @@ -1532,7 +1709,7 @@ async function executeWordFilterUpdate( context, "word-filter.update", "WordFilter", - id, + auditTargetId(id), snapshot, "intent", ), @@ -1543,11 +1720,11 @@ async function executeWordFilterUpdate( context, "word-filter.update", "WordFilter", - id, + auditTargetId(id), snapshot, async () => { reloadWordFilter(); - await rcon.updateWordFilter(); + await requireRcon(await rcon.updateWordFilter()); }, ); } diff --git a/src/features/housekeeping/foundation/commands/dispatcher.test.ts b/src/features/housekeeping/foundation/commands/dispatcher.test.ts index 25ac6c61..e53240f2 100644 --- a/src/features/housekeeping/foundation/commands/dispatcher.test.ts +++ b/src/features/housekeeping/foundation/commands/dispatcher.test.ts @@ -780,7 +780,7 @@ describe("dispatchHousekeepingCommand", () => { expect(audit.entries.map((entry) => entry.outcome)).toEqual(["failure"]); }); - it("throws completed-operation evidence and persists partial when success audit rejects", async () => { + it("returns completed-operation evidence and persists partial when success audit rejects", async () => { const attempts: string[] = []; const persisted: string[] = []; register( @@ -806,10 +806,14 @@ describe("dispatchHousekeepingCommand", () => { }), ); - await expect(dispatch).rejects.toMatchObject({ - name: "AuditOutcomePersistenceError", - operationCompleted: true, - operationResult: { ok: true, data: { changed: true } }, + await expect(dispatch).resolves.toMatchObject({ + ok: true, + data: { changed: true }, + completion: { + status: "partial", + external: "not-required", + audit: "persisted", + }, }); expect(attempts).toEqual(["success", "partial"]); expect(persisted).toEqual(["partial"]); diff --git a/src/features/housekeeping/foundation/commands/dispatcher.ts b/src/features/housekeeping/foundation/commands/dispatcher.ts index 05e2a799..051ecb8d 100644 --- a/src/features/housekeeping/foundation/commands/dispatcher.ts +++ b/src/features/housekeeping/foundation/commands/dispatcher.ts @@ -5,15 +5,12 @@ import { satisfiesCapability } from "../capability-context"; import { fail, type HousekeepingCapabilityContext, + type HousekeepingPartialCompletion, type HousekeepingResult, mapUnknownError, } from "../contracts"; import { createCorrelationId } from "../correlation"; -import { - AuditOutcomePersistenceError, - writeIntent, - writeOutcome, -} from "./audit-envelope"; +import { writeIntent, writeOutcome } from "./audit-envelope"; import { confirmHousekeepingCommand } from "./confirmation"; import { getHousekeepingCommand, @@ -62,11 +59,25 @@ const housekeepingResultSchema = z.discriminatedUnion("ok", [ ok: z.literal(true), data: z.unknown(), 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(), }), z.strictObject({ 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(), }), ]); @@ -295,6 +306,13 @@ export async function dispatchHousekeepingCommand( correlatedResult, ); } + if (correlatedResult.completion?.status === "partial") { + return persistPartialCompletionOutcome( + dependencies.audit, + auditEntry, + correlatedResult, + ); + } return persistSuccessfulOutcome( dependencies.audit, auditEntry, @@ -455,17 +473,49 @@ async function persistReturnedOutcome( async function persistSuccessfulOutcome( writer: HousekeepingAuditWriter, entry: AuditEntry, - result: HousekeepingResult, + result: Extract, { ok: true }>, ): Promise> { try { await writeOutcome(writer, entry, "success"); return result; - } catch (auditOutcomeError) { + } catch { + let audit: HousekeepingPartialCompletion["audit"] = "persisted"; try { await writeOutcome(writer, entry, "partial"); } catch { - // The completed-operation error below remains the primary evidence. + audit = "unavailable"; } - throw new AuditOutcomePersistenceError(result, auditOutcomeError); + return withPartialCompletion(result, { + status: "partial", + external: result.completion?.external ?? "not-required", + audit, + }); } } + +async function persistPartialCompletionOutcome( + writer: HousekeepingAuditWriter, + entry: AuditEntry, + result: Extract, { ok: true }>, +): Promise> { + try { + await writeOutcome(writer, entry, "partial"); + return result; + } catch { + return withPartialCompletion(result, { + ...(result.completion ?? { + status: "partial", + external: "not-required", + audit: "unavailable", + }), + audit: "unavailable", + }); + } +} + +function withPartialCompletion( + result: Extract, { ok: true }>, + completion: HousekeepingPartialCompletion, +): HousekeepingResult { + return { ...result, completion }; +} diff --git a/src/features/housekeeping/foundation/contracts/index.ts b/src/features/housekeeping/foundation/contracts/index.ts index 2aeee309..ad0031dc 100644 --- a/src/features/housekeeping/foundation/contracts/index.ts +++ b/src/features/housekeeping/foundation/contracts/index.ts @@ -28,6 +28,7 @@ export { fail, type HousekeepingError, type HousekeepingErrorCode, + type HousekeepingPartialCompletion, type HousekeepingResult, mapUnknownError, ok, diff --git a/src/features/housekeeping/foundation/contracts/result.ts b/src/features/housekeeping/foundation/contracts/result.ts index 8487028e..1ccbc87d 100644 --- a/src/features/housekeeping/foundation/contracts/result.ts +++ b/src/features/housekeeping/foundation/contracts/result.ts @@ -17,12 +17,32 @@ export interface HousekeepingError { fieldErrors?: Readonly>; } +export interface HousekeepingPartialCompletion { + readonly status: "partial"; + readonly external: "not-required" | "completed" | "failed"; + readonly audit: "persisted" | "unavailable"; +} + export type HousekeepingResult = - | { ok: true; data: T; correlationId: string } + | { + ok: true; + data: T; + correlationId: string; + completion?: HousekeepingPartialCompletion; + } | { ok: false; error: HousekeepingError; correlationId: string }; -export function ok(data: T, correlationId: string): HousekeepingResult { - return { ok: true, data, correlationId }; +export function ok( + data: T, + correlationId: string, + completion?: HousekeepingPartialCompletion, +): HousekeepingResult { + return { + ok: true, + data, + correlationId, + ...(completion === undefined ? {} : { completion }), + }; } export function fail( diff --git a/src/features/housekeeping/foundation/foundation-source-contract.test.ts b/src/features/housekeeping/foundation/foundation-source-contract.test.ts index d703c6a0..57852ac9 100644 --- a/src/features/housekeeping/foundation/foundation-source-contract.test.ts +++ b/src/features/housekeeping/foundation/foundation-source-contract.test.ts @@ -39,7 +39,6 @@ const approvedRuntimeImports = new Map>([ [ "src/features/housekeeping/domains/people/pages/multi-accounts.tsx", new Set([ - "src/features/housekeeping/domains/people/pages/people-command-form", "src/features/housekeeping/domains/people/pages/page-state", "src/features/housekeeping/domains/people/queries/users", ]), diff --git a/src/lib/services/audit.test.ts b/src/lib/services/audit.test.ts index b8a7e453..6b36c68b 100644 --- a/src/lib/services/audit.test.ts +++ b/src/lib/services/audit.test.ts @@ -94,13 +94,17 @@ describe("logAudit", () => { action: "update", target: "user", before: { - profile: { authTicket: "private-ticket" }, + profile: { + authTicket: "private-ticket", + credentials: { newPassword: "one-time-password" }, + }, integrations: [{ api_key: "private-key" }], }, }); const before = JSON.parse(insertValues.mock.calls[0][0].before); expect(before.profile.authTicket).toBe("[Redacted]"); + expect(before.profile.credentials.newPassword).toBe("[Redacted]"); expect(before.integrations[0].api_key).toBe("[Redacted]"); });