fix: resolve issues #50, #51, #52, #53

.gitignore

@@ -7,6 +7,13 @@ dist/
 coverage/
 last-processed-ledger.json
 
+# Test artefacts
+**/__snapshots__/
+*.snap
+.vitest-cache/
+test-results/
+
+# Editor / OS
 # Test snapshots, vitest cache, and coverage artefacts
 **/__snapshots__/
 .vitest-cache/
@@ -21,6 +28,11 @@ vitest.d.ts
 .idea/
 *.swp
 *.swo
+Thumbs.db
+
+# Build artefacts
+*.tsbuildinfo
+
 
 # OS artefacts
 Thumbs.db

drizzle/0012_moderation_events.sql

@@ -0,0 +1,11 @@
+-- Moderation events table — immutable audit log of admin actions on resources (#50).
+CREATE TABLE IF NOT EXISTS "moderation_events" (
+  "id" text PRIMARY KEY NOT NULL,
+  "resource_id" text NOT NULL REFERENCES "resources"("id"),
+  "action" text NOT NULL,
+  "reason" text,
+  "actor" text NOT NULL,
+  "created_at" timestamp DEFAULT now() NOT NULL
+);
+--> statement-breakpoint
+CREATE INDEX IF NOT EXISTS "idx_moderation_events_resource_id" ON "moderation_events" ("resource_id");

src/db/schema.ts

@@ -251,6 +251,10 @@ export const ledgerCheckpoints = pgTable("ledger_checkpoints", {
   updatedAt: timestamp("updated_at").defaultNow().notNull(),
 });
 
+// Moderation events — immutable audit log of admin actions on resources (#50).
+// Written whenever an admin delists a resource so the action is reviewable later.
+export const moderationEvents = pgTable(
+  "moderation_events",
 // Idempotency records shared across replicas (#71).
 export const idempotencyRecords = pgTable("idempotency_records", {
   key: text("key").primaryKey(),
@@ -272,6 +276,21 @@ export const webhookOutbox = pgTable(
     id: text("id")
       .primaryKey()
       .$defaultFn(() => createId()),
+    // The resource that was acted upon.
+    resourceId: text("resource_id")
+      .notNull()
+      .references(() => resources.id),
+    // The moderation action taken (e.g. "admin_delist").
+    action: text("action").notNull(),
+    // Optional free-text reason supplied by the admin.
+    reason: text("reason"),
+    // Identifier of the actor (admin key digest or "admin" when unset).
+    actor: text("actor").notNull(),
+    createdAt: timestamp("created_at").defaultNow().notNull(),
+  },
+  (table) => ({
+    // Fast lookup of all moderation events for a given resource.
+    resourceIdIdx: index("idx_moderation_events_resource_id").on(table.resourceId),
     publisherId: text("publisher_id")
       .notNull()
       .references(() => publishers.id),

src/index.ts

@@ -194,6 +194,13 @@ process.on("unhandledRejection", (reason) => {
 });
 
 process.on("uncaughtException", (err) => {
-  rootLogger.error({ event: "uncaught_exception", err }, "uncaught exception");
+  rootLogger.error({ event: "uncaught_exception", err }, "uncaught exception — flushing and exiting");
   captureServerException(err);
+  // Flush Sentry with a short timeout so the event is delivered before we exit,
+  // then always terminate with code 1 so the platform can restart the service.
+  // We do not await inside a sync handler, so we use .then/.catch to ensure
+  // exit is always called even if flush rejects.
+  void import("@sentry/node").then(({ flush }) =>
+    flush(2000).finally(() => process.exit(1)),
+  ).catch(() => process.exit(1));
 });

src/index.uncaught.test.ts

@@ -0,0 +1,61 @@
+/**
+ * #53 — uncaughtException handler must flush Sentry and call process.exit(1).
+ */
+import { describe, it, expect, vi, afterEach } from "vitest";
+
+// Intercept process.exit so the test process doesn't actually terminate.
+const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {}) as any);
+
+const flushMock = vi.fn().mockResolvedValue(true);
+
+vi.mock("@sentry/node", () => ({
+  init: vi.fn(),
+  captureException: vi.fn(),
+  flush: flushMock,
+}));
+
+vi.mock("./config.js", () => ({ config: { SENTRY_DSN: undefined } }));
+vi.mock("./lib/sentry.js", () => ({
+  initSentry: vi.fn(),
+  captureServerException: vi.fn(),
+}));
+vi.mock("./lib/logger.js", () => ({
+  rootLogger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
+}));
+vi.mock("./app.js", () => ({ createApp: () => ({ listen: vi.fn(() => ({ close: vi.fn() })) }) }));
+vi.mock("./db/client.js", () => ({
+  pgClient: Object.assign(vi.fn(), { connect: vi.fn(), end: vi.fn() }),
+  startPoolMetrics: vi.fn(),
+  stopPoolMetrics: vi.fn(),
+}));
+vi.mock("./lib/lifecycle.js", () => ({ beginShutdown: vi.fn(), whenDrained: vi.fn(), inFlightCount: vi.fn(() => 0) }));
+vi.mock("./lib/rateLimit/index.js", () => ({ closeRateLimitStore: vi.fn() }));
+vi.mock("./workers/retryPendingWorker.js", () => ({
+  startRetryPendingWorker: vi.fn(),
+  stopRetryPendingWorker: vi.fn(),
+}));
+vi.mock("./workers/eventListener.js", () => ({
+  startEventListener: vi.fn(),
+  stopEventListener: vi.fn(),
+}));
+vi.mock("./tracing.js", () => ({}));
+
+describe("#53 uncaughtException handler", () => {
+  afterEach(() => {
+    exitSpy.mockClear();
+    flushMock.mockClear();
+  });
+
+  it("calls process.exit(1) after an uncaught exception", async () => {
+    // Import index.ts to register the handler. We need to let async settle.
+    await import("./index.js");
+
+    const err = new Error("boom — uncaught");
+    process.emit("uncaughtException", err, "uncaughtException");
+
+    // Allow the flush promise to resolve.
+    await new Promise((r) => setTimeout(r, 50));
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+  });
+});

src/routes/admin.test.ts

@@ -13,33 +13,9 @@ vi.mock('../config.js', () => ({
 }));
 
 vi.mock('../db/client.js', () => {
-  const select = vi.fn();
-  const from = vi.fn();
-  const where = vi.fn();
-  const orderBy = vi.fn();
-  const limit = vi.fn();
-  const offset = vi.fn();
-  const chain: any = {};
-  chain.select = select;
-  chain.from = from;
-  chain.where = where;
-  chain.orderBy = orderBy;
-  chain.limit = limit;
-  chain.offset = offset;
-  select.mockReturnValue(chain);
-  from.mockReturnValue(chain);
-  where.mockReturnValue(chain);
-  orderBy.mockReturnValue(chain);
-  limit.mockReturnValue(chain);
-  offset.mockReturnValue(chain);
   return {
     db: {
-      select,
-      from,
-      where,
-      orderBy,
-      limit,
-      offset,
+      select: vi.fn(),
     },
   };
 });
@@ -49,7 +25,7 @@ vi.mock('../services/resourceService.js', () => ({
 }));
 
 vi.mock('../lib/logger.js', () => ({
-  logger: {
+  getLogger: () => ({
     info: vi.fn(),
     warn: vi.fn(),
     error: vi.fn(),
@@ -73,6 +49,29 @@ import { default as adminRouter } from './admin.js';
 import * as dbClient from '../db/client.js';
 import * as resourceService from '../services/resourceService.js';
 
+const ORIGINAL_ENV = { ...process.env };
+
+// Build the Express app once; admin.ts reads process.env at request time,
+// not at import time, so we don't need to re-import between tests.
+let app: ReturnType<typeof express>;
+
+async function getApp() {
+  if (!app) {
+    const { default: adminRouter } = await import('./admin.js');
+    app = express();
+    app.use(express.json());
+    // Mount without a prefix — the router registers /admin/stats, etc. itself,
+    // matching how app.ts does it with app.use(adminRouter).
+    app.use(adminRouter);
+    // Map AppError codes to HTTP status codes (mirror errorHandler.ts).
+    app.use((err: any, _req: any, res: any, _next: any) => {
+      const codes: Record<string, number> = {
+        UNAUTHORIZED: 401, FORBIDDEN: 403, NOT_FOUND: 404, VALIDATION_ERROR: 400,
+      };
+      const status = codes[err?.code] ?? (typeof err?.status === 'number' ? err.status : 500);
+      res.status(status).json({ error: err?.message ?? 'Internal Server Error' });
+    });
+  }
 function buildApp() {
   const app = express();
   app.use(express.json());
@@ -94,12 +93,16 @@ describe('admin routes', () => {
 
   describe('auth check', () => {
     it('returns 401 when X-Admin-Key header is missing', async () => {
+      process.env.ADMIN_API_KEY = 'secret-key';
+      const res = await request(await getApp()).get('/admin/stats');
       mockConfig.ADMIN_API_KEY = 'secret-key';
       const res = await request(buildApp()).get('/admin/stats');
       expect(res.status).toBe(401);
     });
 
     it('returns 401 when X-Admin-Key is wrong', async () => {
+      process.env.ADMIN_API_KEY = 'secret-key';
+      const res = await request(await getApp())
       mockConfig.ADMIN_API_KEY = 'secret-key';
       const res = await request(buildApp())
         .get('/admin/stats')
@@ -108,6 +111,8 @@ describe('admin routes', () => {
     });
 
     it('returns 401 when ADMIN_API_KEY is unset', async () => {
+      delete process.env.ADMIN_API_KEY;
+      const res = await request(await getApp())
       mockConfig.ADMIN_API_KEY = undefined;
       const res = await request(buildApp())
         .get('/admin/stats')
@@ -120,14 +125,13 @@ describe('admin routes', () => {
     it('returns counts from the db plus uptime and timestamp', async () => {
       mockConfig.ADMIN_API_KEY = 'secret-key';
 
-      const db = dbClient.db as any;
-      // Each select chain resolves to a count row.
-      db.select.mockImplementation(() => {
-        const chain: any = {};
-        chain.from = vi.fn().mockResolvedValue([{ count: 7 }]);
-        return chain;
-      });
+      // The stats route calls db.select({count}).from(table) three times in parallel.
+      // Each Promise.all element resolves to [[{count: N}]].
+      (dbClient.db.select as any).mockImplementation(() => ({
+        from: vi.fn().mockResolvedValue([{ count: 7 }]),
+      }));
 
+      const res = await request(await getApp())
       const res = await request(buildApp())
         .get('/admin/stats')
         .set('X-Admin-Key', 'secret-key');
@@ -137,6 +141,10 @@ describe('admin routes', () => {
       expect(typeof res.body.uptime).toBe('number');
       expect(res.body).toHaveProperty('timestamp');
       expect(typeof res.body.timestamp).toBe('string');
+      // The route returns top-level resource / publisher / payment counts.
+      expect(res.body.resources).toBeDefined();
+      expect(res.body.publishers).toBeDefined();
+      expect(res.body.payments).toBeDefined();
     });
   });
 
@@ -145,21 +153,40 @@ describe('admin routes', () => {
       mockConfig.ADMIN_API_KEY = 'secret-key';
       (resourceService.adminDelistResource as any).mockResolvedValue({
         id: 'res-1',
-        delisted: true,
       });
 
+      const res = await request(await getApp())
       const res = await request(buildApp())
         .post('/admin/delist/res-1')
         .set('X-Admin-Key', 'secret-key');
 
       expect(res.status).toBe(200);
-      expect(resourceService.adminDelistResource).toHaveBeenCalledWith('res-1');
+      expect(resourceService.adminDelistResource).toHaveBeenCalledWith(
+        'res-1',
+        expect.objectContaining({ actor: expect.stringMatching(/^admin:/) }),
+      );
+    });
+
+    it('passes reason from request body to adminDelistResource', async () => {
+      process.env.ADMIN_API_KEY = 'secret-key';
+      (resourceService.adminDelistResource as any).mockResolvedValue({ id: 'res-2' });
+
+      await request(await getApp())
+        .post('/admin/delist/res-2')
+        .set('X-Admin-Key', 'secret-key')
+        .send({ reason: 'copyright violation' });
+
+      expect(resourceService.adminDelistResource).toHaveBeenCalledWith(
+        'res-2',
+        expect.objectContaining({ reason: 'copyright violation' }),
+      );
     });
 
     it('returns 404 when adminDelistResource resolves null', async () => {
       mockConfig.ADMIN_API_KEY = 'secret-key';
       (resourceService.adminDelistResource as any).mockResolvedValue(null);
 
+      const res = await request(await getApp())
       const res = await request(buildApp())
         .post('/admin/delist/missing')
         .set('X-Admin-Key', 'secret-key');
@@ -172,16 +199,19 @@ describe('admin routes', () => {
     it('returns paginated entries', async () => {
       mockConfig.ADMIN_API_KEY = 'secret-key';
 
-      const db = dbClient.db as any;
       const entries = [
-        { id: 'a1', action: 'delist', createdAt: new Date().toISOString() },
-        { id: 'a2', action: 'delist', createdAt: new Date().toISOString() },
+        { id: 'a1', resourceId: 'r1', payer: 'G1', amount: '1.00', paidAt: new Date().toISOString() },
+        { id: 'a2', resourceId: 'r2', payer: 'G2', amount: '2.00', paidAt: new Date().toISOString() },
       ];
+      // The audit query: db.select({...}).from(payments).where(...).orderBy(...).limit(...).offset(...)
       const offsetMock = vi.fn().mockResolvedValue(entries);
       const limitMock = vi.fn().mockReturnValue({ offset: offsetMock });
       const orderByMock = vi.fn().mockReturnValue({ limit: limitMock });
       const whereMock = vi.fn().mockReturnValue({ orderBy: orderByMock });
       const fromMock = vi.fn().mockReturnValue({ where: whereMock });
+      (dbClient.db.select as any).mockReturnValue({ from: fromMock });
+
+      const res = await request(await getApp())
       db.select.mockReturnValue({ from: fromMock });
 
       const res = await request(buildApp())
@@ -197,6 +227,15 @@ describe('admin routes', () => {
     it('clamps limit to [1,200] and page to >=1', async () => {
       mockConfig.ADMIN_API_KEY = 'secret-key';
 
+      const offsetMock = vi.fn().mockResolvedValue([]);
+      const limitMock = vi.fn().mockReturnValue({ offset: offsetMock });
+      const orderByMock = vi.fn().mockReturnValue({ limit: limitMock });
+      const whereMock = vi.fn().mockReturnValue({ orderBy: orderByMock });
+      const fromMock = vi.fn().mockReturnValue({ where: whereMock });
+      (dbClient.db.select as any).mockReturnValue({ from: fromMock });
+
+      // limit too high, page too low (page 0 becomes 1, offset 0)
+      await request(await getApp())
       const db = dbClient.db as any;
 
       function setupChain() {
@@ -219,6 +258,17 @@ describe('admin routes', () => {
       expect(chain1.limitMock).toHaveBeenLastCalledWith(200);
       expect(chain1.offsetMock).toHaveBeenLastCalledWith(0);
 
+      expect(limitMock).toHaveBeenLastCalledWith(200);
+      expect(offsetMock).toHaveBeenLastCalledWith(0);
+
+      // limit too low (0 is falsy so it falls back to the default of 50), page valid (page 3, offset = (3-1)*50 = 100)
+      await request(await getApp())
+        .get('/admin/audit?limit=0&page=3')
+        .set('X-Admin-Key', 'secret-key');
+
+      // limit=0 is falsy → defaults to 50 (the || 50 fallback in admin.ts)
+      expect(limitMock).toHaveBeenLastCalledWith(50);
+      expect(offsetMock).toHaveBeenLastCalledWith(100);
       // explicit low limit (1), page 3 → offset = 2
       const chain2 = setupChain();
       await request(app)

src/routes/admin.ts

@@ -49,11 +49,16 @@ router.get("/admin/stats", adminAuth, async (_req, res) => {
 /** POST /admin/delist/:id */
 router.post("/admin/delist/:id", adminAuth, async (req, res) => {
   const id = req.params.id as string;
-  const resource = await adminDelistResource(id);
+  const reason = typeof req.body?.reason === "string" ? req.body.reason.trim() || undefined : undefined;
+  // Use a digest of the admin key as the actor identifier so the audit log
+  // is attributable without storing the raw key.
+  const rawKey = req.headers["x-admin-key"] as string;
+  const actor = `admin:${createHash("sha256").update(rawKey).digest("hex").slice(0, 12)}`;
+  const resource = await adminDelistResource(id, { reason, actor });
   if (!resource) {
     throw new AppError("NOT_FOUND", "Resource not found");
   }
-  getLogger().warn({ resourceId: id, event: "admin_delist" }, "resource delisted by admin");
+  getLogger().warn({ resourceId: id, event: "admin_delist", actor, reason }, "resource delisted by admin");
   res.json({ success: true, id, delistedAt: new Date().toISOString() });
 });