From 5cb42c8dfbecdbc80766e6e657ada39cec6887f5 Mon Sep 17 00:00:00 2001
From: openhands <openhands@all-hands.dev>
Date: Fri, 9 Oct 2026 17:57:28 +0200
Subject: [PATCH] fix: stop the commandocentrum audit call leaking an unhandled
 rejection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The full suite passed but exited non-zero, which fails CI: three unhandled
rejections came out of commandocentrum's fire-and-forget audit call.

The cause is a real defect, not a test artefact. `auditAction` wrapped
`logAudit(...)` in a try/catch to honour "auditing must never fail the command
it describes", but logAudit is async, so the catch can never see its rejection.
A failing audit insert therefore surfaced as an unhandled rejection instead of
being swallowed — in production that is a request taking down over a logging
failure. The catch is now on the promise itself.

The test now mocks the audit service explicitly instead of leaning on the fake
db lacking `insert`, and asserts both that an entry is logged and that a
rejecting audit still lets the command succeed.
---
 src/actions/commandocentrum.test.ts | 29 +++++++++++++++++++++++++++++
 src/actions/commandocentrum.ts      | 15 ++++++++++-----
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/src/actions/commandocentrum.test.ts b/src/actions/commandocentrum.test.ts
index 6198996e..fa3dbaf7 100644
--- a/src/actions/commandocentrum.test.ts
+++ b/src/actions/commandocentrum.test.ts
@@ -52,6 +52,13 @@ const mockSetMotto = vi.hoisted(() => vi.fn());
 const mockSetRank = vi.hoisted(() => vi.fn());
 const mockExecuteCommand = vi.hoisted(() => vi.fn());
 const mockSendGift = vi.hoisted(() => vi.fn());
+// The audit service is exercised separately; here it only has to be harmless.
+// Mocked explicitly because the fake db has no `insert`, which used to leak an
+// unhandled rejection out of the fire-and-forget audit call.
+vi.mock("@/lib/services/audit", () => ({
+	logAudit: vi.fn(async () => undefined),
+}));
+
 vi.mock("@/lib/services/rcon", () => ({
 	rcon: {
 		send: mockSend,
@@ -455,3 +462,25 @@ describe("access control", () => {
 		});
 	});
 });
+
+describe("auditing", () => {
+	it("logs an entry for a currency grant", async () => {
+		const { logAudit } = await import("@/lib/services/audit");
+		await giveCredits({ userId: 1, credits: 100 });
+		expect(logAudit).toHaveBeenCalledWith(
+			expect.objectContaining({ action: expect.any(String) }),
+		);
+	});
+
+	it("completes the command even when auditing rejects", async () => {
+		const { logAudit } = await import("@/lib/services/audit");
+		vi.mocked(logAudit).mockRejectedValueOnce(new Error("audit table missing"));
+		await expect(giveCredits({ userId: 1, credits: 100 })).resolves.toEqual({
+			ok: true,
+			data: {},
+		});
+		// Let the fire-and-forget promise settle; an unhandled rejection here is
+		// exactly the failure this guards against.
+		await new Promise((r) => setTimeout(r, 0));
+	});
+});
diff --git a/src/actions/commandocentrum.ts b/src/actions/commandocentrum.ts
index 92d243ab..fda8aea7 100644
--- a/src/actions/commandocentrum.ts
+++ b/src/actions/commandocentrum.ts
@@ -24,11 +24,16 @@ function auditAction(
 	targetId: number,
 	after: Record<string, unknown>,
 ): void {
-	try {
-		logAudit({ userId, action, target: "User", targetId, after });
-	} catch {
-		/* auditing must never fail the command it describes */
-	}
+	// logAudit is async, so a surrounding try/catch cannot see its rejection —
+	// it would surface as an unhandled rejection and, in production, take the
+	// request down over a failing audit insert. Swallow it on the promise
+	// instead, which is what "auditing must never fail the command it
+	// describes" actually requires.
+	void Promise.resolve()
+		.then(() => logAudit({ userId, action, target: "User", targetId, after }))
+		.catch(() => {
+			/* auditing must never fail the command it describes */
+		});
 }
 
 async function requireRconOk(ok: boolean): Promise<void> {