From 54f0345aac7f2a93883b34d2f62151bf66df81de Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sat, 5 Sep 2026 11:25:29 +0200 Subject: [PATCH] fix(housekeeping): release authority lock before activity --- .../people/services/moderation-mutations.ts | 38 ++++--- .../mutations-production-workflows.test.ts | 104 ++++++++++++++++++ .../domains/people/services/mutations.ts | 65 ++++++----- 3 files changed, 166 insertions(+), 41 deletions(-) diff --git a/src/features/housekeeping/domains/people/services/moderation-mutations.ts b/src/features/housekeeping/domains/people/services/moderation-mutations.ts index 3790b7ef..35263466 100644 --- a/src/features/housekeeping/domains/people/services/moderation-mutations.ts +++ b/src/features/housekeeping/domains/people/services/moderation-mutations.ts @@ -80,6 +80,7 @@ export interface PeopleModerationMutationDependencies { userId: number, context: PeopleModerationMutationContext, execute: () => Promise, + afterRelease?: () => Promise, ) => Promise; readonly transport: ModerationTransport; readonly executeModerationAction: ( @@ -406,22 +407,27 @@ export function createPeopleModerationMutationExecutor( banId, snapshot, () => - runWithLockedTargetAuthority(userId, context, async () => { - let delivered = true; - if (username !== null) { - delivered = await transport.disconnectUser(userId, username); - } - await logStaffActivity({ - staffId: context.capability.actor.id, - action: "user_ban", - description: `Banned user #${userId} (${type}, ${ - hours > 0 ? `${hours}h` : "permanent" - }): ${reason}`, - targetType: "user", - targetId: userId, - }); - await requireRcon(delivered); - }), + runWithLockedTargetAuthority( + userId, + context, + async () => { + let delivered = true; + if (username !== null) { + delivered = await transport.disconnectUser(userId, username); + } + await requireRcon(delivered); + }, + () => + logStaffActivity({ + staffId: context.capability.actor.id, + action: "user_ban", + description: `Banned user #${userId} (${type}, ${ + hours > 0 ? `${hours}h` : "permanent" + }): ${reason}`, + targetType: "user", + targetId: userId, + }), + ), ); } 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 5f0f9b6c..d4f05374 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 @@ -319,6 +319,110 @@ describe("People production workflow adapter", () => { ).toEqual(["intent", "partial"]); }); + it.each([ + [ + "user.trade-lock", + { userId: 7, untilUnix: 200 }, + [[target()], [], [], [target()]], + "setTradeLock", + ], + [ + "ban.create", + { userId: 7, hours: 24, type: "account", reason: "Abuse" }, + [[target()], [target()]], + "disconnectUser", + ], + ] as const)( + "releases the user lock before legacy activity for %s", + async (operation, input, selectedRows, transportMethod) => { + let transactionDepth = 0; + let dispatchDepth = -1; + let activityDepth = -1; + mocks.selectQueue.push(...selectedRows.map((rows) => [...rows])); + const transaction = async ( + callback: (tx: ReturnType) => Promise, + ) => { + transactionDepth += 1; + try { + return await callback(databaseFacade()); + } finally { + transactionDepth -= 1; + } + }; + mocks.transaction + .mockImplementationOnce(transaction) + .mockImplementationOnce(transaction); + mocks.rcon[transportMethod].mockImplementationOnce(async () => { + dispatchDepth = transactionDepth; + return true; + }); + mocks.logStaffActivity.mockImplementationOnce(async () => { + activityDepth = transactionDepth; + }); + + const result = await peopleMutationService.execute( + { ...invocation, legacy: false }, + operation, + input, + ); + + expect(result).toMatchObject({ ok: true }); + expect(dispatchDepth).toBe(1); + expect(activityDepth).toBe(0); + expect(mocks.logStaffActivity).toHaveBeenCalledTimes(1); + }, + ); + + it("preserves known completion when released activity also fails", async () => { + let transactionDepth = 0; + let dispatchCompleted = false; + let activityDepth = -1; + mocks.selectQueue.push([target()], [], [], [target()]); + mocks.transaction + .mockImplementationOnce(async (callback) => callback(databaseFacade())) + .mockImplementationOnce(async (callback) => { + transactionDepth += 1; + try { + const result = await callback(databaseFacade()); + if (dispatchCompleted) { + throw new Error("read-only authority commit failed"); + } + return result; + } finally { + transactionDepth -= 1; + } + }); + mocks.rcon.setTradeLock.mockImplementationOnce(async () => { + dispatchCompleted = true; + return true; + }); + mocks.logStaffActivity.mockImplementationOnce(async () => { + activityDepth = transactionDepth; + throw new Error("activity transport failed"); + }); + + const result = await peopleMutationService.execute( + { + ...invocation, + legacy: false, + correlationId: "released-activity-failed", + }, + "user.trade-lock", + { userId: 7, untilUnix: 200 }, + ); + + expect(activityDepth).toBe(0); + expect(result).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "completed", + audit: "persisted", + }, + }); + expect(mocks.rcon.setTradeLock).toHaveBeenCalledTimes(1); + }); + it("rejects forged, closed, and non-user CFH sanctions before mutation", async () => { for (const [ticket, input, code] of [ [ diff --git a/src/features/housekeeping/domains/people/services/mutations.ts b/src/features/housekeeping/domains/people/services/mutations.ts index 6fa0870e..bc9490e2 100644 --- a/src/features/housekeeping/domains/people/services/mutations.ts +++ b/src/features/housekeeping/domains/people/services/mutations.ts @@ -435,8 +435,10 @@ async function runWithLockedTargetAuthority( userId: number, context: PeopleMutationContext, execute: () => Promise, + afterRelease?: () => Promise, ): Promise { let externalCompleted = false; + let completedFailure: CompletedExternalEffectFailure | undefined; try { await db.transaction(async (tx) => { await loadTarget(userId, context, true, tx, true); @@ -444,9 +446,15 @@ async function runWithLockedTargetAuthority( externalCompleted = true; }); } catch (error) { - if (externalCompleted) throw new CompletedExternalEffectFailure(error); - throw error; + if (!externalCompleted) throw error; + completedFailure = new CompletedExternalEffectFailure(error); } + try { + await afterRelease?.(); + } catch (error) { + completedFailure ??= new CompletedExternalEffectFailure(error); + } + if (completedFailure) throw completedFailure; } async function assertBulkTargetHierarchy( @@ -1247,29 +1255,36 @@ async function executeUserMutation( userId, snapshot, () => - runWithLockedTargetAuthority(userId, context, async () => { - 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 requireRcon(await rcon.disconnectUser(userId, target.username)); - } - await logStaffActivity({ - staffId: context.capability.actor.id, - action: locked ? "trade_lock" : "trade_unlock", - description: locked - ? `Trade-locked ${target.username} (#${userId}) until ${untilUnix}` - : `Cleared trade lock for ${target.username} (#${userId})`, - targetType: "user", - targetId: userId, - }); - }), + runWithLockedTargetAuthority( + userId, + context, + async () => { + 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 requireRcon( + await rcon.disconnectUser(userId, target.username), + ); + } + }, + () => + logStaffActivity({ + staffId: context.capability.actor.id, + action: locked ? "trade_lock" : "trade_unlock", + description: locked + ? `Trade-locked ${target.username} (#${userId}) until ${untilUnix}` + : `Cleared trade lock for ${target.username} (#${userId})`, + targetType: "user", + targetId: userId, + }), + ), ); } async function executeBulkMutation(