feat(server): plan 3i groundwork — AuditLogRepository's reserved methods, the demo connection-swap spike, repository-fitness confirmations

server/src/db/repositories/AuditLog.repository.ts

@@ -22,10 +22,36 @@ const _auditLogRowKeys: AssertRowKeys<AuditLogRow, AuditLog> = true;
 /** The column set `insertEntry` writes; `id`/`created_at` are generated. */
 export type NewAuditLogRow = Omit<AuditLogRow, 'id' | 'created_at'>;
 
-// Reserved for Plan 3i (`admin.service.ts`'s paginated read of the same
-// table): `listPage(limit, offset): Promise<AuditLogPageRow[]>` (a `qb()`
-// join to `Users` for `username`/`email`) and `count(): Promise<number>`.
-// Not implemented here — see plan3a-sql-inventory.md §1.
+/**
+ * AD22's joined projection (`admin.service.ts#getAuditLog`): `a.id,
+ * a.created_at, a.user_id, u.username, u.email as user_email, a.action,
+ * a.resource, a.details, a.ip` from `audit_log a LEFT JOIN users u ON
+ * u.id = a.user_id`. Distinct from `AuditLogRow` (the bare-table shape
+ * `insertEntry` writes) — a deleted/never-existing user leaves `username`/
+ * `user_email` both null (the `LEFT JOIN`, not an `INNER JOIN`), the row
+ * itself is still returned. The `created_at` ISO-suffix normalization and
+ * the `details` JSON-parse-with-fallback stay in `AdminService` — this
+ * repository returns the raw joined row untouched, same boundary the
+ * class-level docs elsewhere in this program draw between repository and
+ * service.
+ */
+export interface AuditLogPageRow {
+  id: number;
+  created_at: string | null;
+  user_id: number | null;
+  username: string | null;
+  user_email: string | null;
+  action: string;
+  resource: string | null;
+  details: string | null;
+  ip: string | null;
+}
+
+/** Kysely shape for the `listPage` join — only the columns it actually selects. */
+interface AuditLogKyselyDB {
+  audit_log: AuditLogRow;
+  users: { id: number; username: string; email: string };
+}
 
 export class AuditLogRepository extends TrekRepository<AuditLog> {
   /**
@@ -65,4 +91,38 @@ export class AuditLogRepository extends TrekRepository<AuditLog> {
       ip: entry.ip,
     });
   }
+
+  /**
+   * AD22 (`admin.service.ts#getAuditLog`) — `SELECT a.id, a.created_at,
+   * a.user_id, u.username, u.email as user_email, a.action, a.resource,
+   * a.details, a.ip FROM audit_log a LEFT JOIN users u ON u.id = a.user_id
+   * ORDER BY a.id DESC LIMIT ? OFFSET ?`. Kysely, not the ORM's own
+   * QueryBuilder — the same `CollabMessagesRepository#joinedQuery` shape
+   * every other flat-projection join in this program uses: `LEFT JOIN`
+   * keeps a deleted-user's row (both `username`/`user_email` come back
+   * null rather than dropping the row), and the projection is nine flat
+   * scalars, not a hydrated `Users` entity.
+   */
+  async listPage(limit: number, offset: number): Promise<AuditLogPageRow[]> {
+    return await this.kysely<AuditLogKyselyDB>()
+      .selectFrom('audit_log as a')
+      .leftJoin('users as u', 'u.id', 'a.user_id')
+      .select(['a.id', 'a.created_at', 'a.user_id', 'u.username', 'u.email as user_email', 'a.action', 'a.resource', 'a.details', 'a.ip'])
+      .orderBy('a.id', 'desc')
+      .limit(limit)
+      .offset(offset)
+      .execute();
+  }
+
+  /**
+   * AD23 (`admin.service.ts#getAuditLog`) — `SELECT COUNT(*) as c FROM
+   * audit_log`. `TrekRepository#count` (no filter) already renders the
+   * equivalent `COUNT(*)` over the whole table; named explicitly here (a
+   * thin `super.count()` pass-through) so the reserved AD23 call site has
+   * its own documented anchor rather than admin's service code calling the
+   * inherited method by its generic name.
+   */
+  override async count(): Promise<number> {
+    return await super.count();
+  }
 }

server/tests/integration/demo-reset-connection-swap.spike.test.ts

@@ -0,0 +1,170 @@
+/**
+ * Plan 3i Task 0 — R3's spike.
+ *
+ * `demo-reset.ts#resetDemoUser` runs inside a cron tick that
+ * `CronRegistrarService.register()` already wraps in a `withRequestContext`
+ * fork (`cron-registrar.service.ts:87-92`, `wrappedTick`). INSIDE that one
+ * fork, the function reads credentials, calls `closeDb()` then
+ * `await reinitialize()` (which closes the process's one better-sqlite3
+ * handle, opens a new one, and — via `orm.ts#attachOrm`'s
+ * `registerReinitializeHook` — closes and reconnects the ORM's own
+ * Kysely-bound `Connection`, then re-runs `runSchemaBootstrap`), and only
+ * THEN writes the preserved credentials back through a freshly re-required
+ * `db` reference.
+ *
+ * The open question (plan3i-inputs.md correction #2): does a REPOSITORY
+ * call issued through the SAME `EntityManager` fork the tick started with —
+ * i.e. still inside the one `withRequestContext(orm, async () => { ... })`
+ * call — still resolve correctly against the driver's connection after that
+ * connection object has been closed and rebound to a new raw handle
+ * mid-callback? This file measures it against a real `buildApp()` boot, not
+ * a mock, exactly as `demo-seed-request-context.test.ts` (its closest
+ * neighbour) measures `runDemoSeed`'s own, simpler wrap.
+ *
+ * Mechanism (for the report, not asserted directly here — this is the
+ * reasoning the measurement below either confirms or refutes): MikroORM
+ * forks an `EntityManager` per `RequestContext.create` call, but a fork does
+ * NOT fork the `Connection`/`Driver` — those are singletons owned by the ORM
+ * instance's `Configuration`, shared by every fork. `orm-driver.ts`'s
+ * `BoundSqliteConnection.createKyselyDialect()` reads `getRawConnection()`
+ * (the CURRENT `db/database.ts` handle) on every `connect()`, and
+ * `attachOrm`'s hook calls `connection.close(true)` then
+ * `connection.connect()` on that ONE shared `Connection` object — it never
+ * replaces the object itself, only rebinds its internal Kysely client. So
+ * any EM fork — pre-swap or fresh — that resolves a query calls
+ * `this.driver.getConnection()` fresh each time, landing on the SAME,
+ * now-rebound `Connection` object. The theoretical expectation is "safe";
+ * this file is what turns that into a measured answer with real evidence
+ * rather than an assumption.
+ *
+ * Test-mode nuance: `NODE_ENV=test` makes `db/database.ts#initDb()` reopen a
+ * PRISTINE copy of the migrated schema snapshot on every call (a fresh
+ * `better-sqlite3` instance deserialised from a fixed buffer), not the same
+ * on-disk file demo-reset.ts restores in production — so a row inserted
+ * before the swap is genuinely gone after `reinitialize()` resolves. That is
+ * expected data loss (the same thing a real demo-reset baseline restore
+ * does) and is NOT what this file is measuring; what it measures is whether
+ * the post-swap call reaches the new, live connection at all (succeeds,
+ * even returning null/creating a fresh row) versus throwing, hanging, or
+ * silently targeting a stale/closed handle.
+ */
+import { afterEach, describe, expect, it } from 'vitest';
+import type { INestApplication } from '@nestjs/common';
+import { MikroORM } from '@mikro-orm/core';
+import { getRepositoryToken } from '@mikro-orm/nestjs';
+import { buildApp } from '../../src/bootstrap';
+import { withRequestContext } from '../../src/nest/database/request-context';
+import { closeDb, reinitialize } from '../../src/db/database';
+import { Users } from '../../src/db/entities/Users.entity';
+import type { UsersRepository } from '../../src/db/repositories/Users.repository';
+
+type Outcome = { ok: true; value: unknown } | { ok: false; error: string };
+
+async function capture<T>(fn: () => Promise<T>): Promise<Outcome> {
+  try {
+    return { ok: true, value: await fn() };
+  } catch (e) {
+    return { ok: false, error: e instanceof Error ? e.message : String(e) };
+  }
+}
+
+describe('Plan 3i Task 0 spike: demo-reset connection swap', () => {
+  let app: INestApplication | undefined;
+
+  afterEach(async () => {
+    await app?.close();
+    app = undefined;
+  });
+
+  it('SPIKE-001: a repository read AND a repository write, issued through the pre-swap EntityManager fork after closeDb()+reinitialize(), inside the SAME withRequestContext call demo-reset.job.ts\'s wrappedTick uses', async () => {
+    app = await buildApp();
+    const orm = app.get(MikroORM);
+    const users = app.get<UsersRepository>(getRepositoryToken(Users));
+
+    let preSwapId: number | undefined;
+    let readAfterSwap: Outcome | undefined;
+    let writeAfterSwap: Outcome | undefined;
+
+    // Exactly the shape of cron-registrar.service.ts's wrappedTick: one
+    // withRequestContext call wrapping the whole tick, read -> swap -> write.
+    await withRequestContext(orm, async () => {
+      // Phase 1 — pre-close, proving the fork works before the swap at all
+      // (mirrors demo-reset.ts's DMR1/DMR2 credential reads).
+      const inserted = await users.insertUser({
+        username: 'spike-pre-swap',
+        email: 'spike-pre-swap@example.test',
+        password_hash: 'x',
+        role: 'user',
+        first_seen_version: '0.0.0-spike',
+      });
+      preSwapId = inserted.id;
+      const before = await users.findById(preSwapId);
+      expect(before?.email).toBe('spike-pre-swap@example.test');
+
+      // The swap — demo-reset.ts's own `closeDb(); await reinitialize();`,
+      // called from inside the still-open fork, exactly where resetDemoUser
+      // calls it between its pre-close reads and its post-reopen writes.
+      closeDb();
+      await reinitialize();
+
+      // Phase 2 — SAME fork, AFTER the swap. A read first (mirrors nothing
+      // demo-reset.ts does post-swap, but isolates read-only behaviour),
+      // then a write (mirrors DMR5/DMR6, the credential/API-key restore).
+      readAfterSwap = await capture(() => users.findById(preSwapId as number));
+      writeAfterSwap = await capture(() =>
+        users.insertUser({
+          username: 'spike-post-swap',
+          email: 'spike-post-swap@example.test',
+          password_hash: 'x',
+          role: 'user',
+          first_seen_version: '0.0.0-spike',
+        }),
+      );
+    });
+
+    console.log('[SPIKE-001] readAfterSwap:', JSON.stringify(readAfterSwap));
+    console.log('[SPIKE-001] writeAfterSwap:', JSON.stringify(writeAfterSwap));
+
+    // The pre-swap fork's post-swap READ must not throw (test-mode reopens a
+    // pristine snapshot, so the pre-swap row is gone — a defined `null` is
+    // the CORRECT answer here, not a failure).
+    expect(readAfterSwap?.ok).toBe(true);
+    if (readAfterSwap?.ok) expect(readAfterSwap.value).toBeNull();
+
+    // The pre-swap fork's post-swap WRITE must not throw either.
+    expect(writeAfterSwap?.ok).toBe(true);
+
+    // The decisive check: did that write actually land on the NEW, live
+    // connection, or silently no-op / target a stale handle? Verify through
+    // a completely FRESH request context (a new fork), reading the row back
+    // by a value only the post-swap write could have created.
+    let writeVisibleOnNewConnection = false;
+    await withRequestContext(orm, async () => {
+      const id = await users.findIdByEmailOrUsernameCI('spike-post-swap@example.test', 'spike-post-swap');
+      writeVisibleOnNewConnection = id !== null;
+    });
+    console.log('[SPIKE-001] writeVisibleOnNewConnection:', writeVisibleOnNewConnection);
+    expect(writeVisibleOnNewConnection).toBe(true);
+  });
+
+  it('SPIKE-002: a FRESH withRequestContext opened AFTER reinitialize() resolves works correctly for a post-swap write (R3\'s fallback-shape requirement)', async () => {
+    app = await buildApp();
+    const orm = app.get(MikroORM);
+    const users = app.get<UsersRepository>(getRepositoryToken(Users));
+
+    closeDb();
+    await reinitialize();
+
+    await withRequestContext(orm, async () => {
+      const inserted = await users.insertUser({
+        username: 'spike-fresh-fork',
+        email: 'spike-fresh-fork@example.test',
+        password_hash: 'x',
+        role: 'user',
+        first_seen_version: '0.0.0-spike',
+      });
+      const row = await users.findById(inserted.id);
+      expect(row?.email).toBe('spike-fresh-fork@example.test');
+    });
+  });
+});

server/tests/unit/db/repositories/AuditLog.repository.test.ts

@@ -75,4 +75,63 @@ describe('AuditLogRepository', () => {
     const auditRow = testDb.prepare('SELECT action FROM audit_log').get() as { action: string };
     expect(auditRow.action).toBe('trip.create');
   });
+
+  // ── Plan 3i Task 0 — listPage/count (AD22/AD23) ────────────────────────────
+
+  /** Byte-for-byte AD22 (`admin.service.ts#getAuditLog`). */
+  const LEGACY_PAGE_SQL = `
+    SELECT a.id, a.created_at, a.user_id, u.username, u.email as user_email, a.action, a.resource, a.details, a.ip
+    FROM audit_log a
+    LEFT JOIN users u ON u.id = a.user_id
+    ORDER BY a.id DESC
+    LIMIT ? OFFSET ?
+  `;
+
+  it('AUDITREPO-007: listPage matches the legacy joined SELECT exactly for a known user, a deleted (FK SET NULL) user, and a null user_id row', async () => {
+    const { user: alice } = createUser(testDb, { username: 'alice', email: 'alice@test.example.com' });
+    const { user: bob } = createUser(testDb, { username: 'bob', email: 'bob@test.example.com' });
+
+    await auditLog.insertEntry({ user_id: alice.id, action: 'trip.create', resource: 'trip', details: '{"title":"Rome"}', ip: '1.1.1.1' });
+    await auditLog.insertEntry({ user_id: bob.id, action: 'trip.update', resource: 'trip', details: null, ip: '2.2.2.2' });
+    await auditLog.insertEntry({ user_id: null, action: 'user.login', resource: null, details: null, ip: null });
+
+    // bob is deleted after the fact; `audit_log.user_id REFERENCES users(id)
+    // ON DELETE SET NULL` fires — this row's own user_id becomes NULL, the
+    // same shape a NULL-user_id row has, reached a different way.
+    testDb.prepare('DELETE FROM users WHERE id = ?').run(bob.id);
+
+    const legacy = testDb.prepare(LEGACY_PAGE_SQL).all(10, 0);
+    const result = await auditLog.listPage(10, 0);
+    expect(result).toEqual(legacy);
+    // The deleted user's row and the originally-null row both keep their
+    // place — a LEFT JOIN never drops the row.
+    expect(result).toHaveLength(3);
+    expect(result.filter((r) => r.username === null && r.user_email === null)).toHaveLength(2);
+    // The known user's row still carries its username/email.
+    const aliceRow = result.find((r) => r.user_id === alice.id);
+    expect(aliceRow?.username).toBe('alice');
+    expect(aliceRow?.user_email).toBe('alice@test.example.com');
+  });
+
+  it('AUDITREPO-008: listPage honors LIMIT/OFFSET and ORDER BY a.id DESC, matching the legacy statement', async () => {
+    const { user } = createUser(testDb);
+    for (let i = 0; i < 5; i++) {
+      await auditLog.insertEntry({ user_id: user.id, action: `action.${i}`, resource: null, details: null, ip: null });
+    }
+    const legacyPage = testDb.prepare(LEGACY_PAGE_SQL).all(2, 1);
+    const result = await auditLog.listPage(2, 1);
+    expect(result).toEqual(legacyPage);
+    expect(result).toHaveLength(2);
+  });
+
+  it('AUDITREPO-009: count matches the legacy COUNT(*) over the whole table', async () => {
+    const { user } = createUser(testDb);
+    await auditLog.insertEntry({ user_id: user.id, action: 'a', resource: null, details: null, ip: null });
+    await auditLog.insertEntry({ user_id: null, action: 'b', resource: null, details: null, ip: null });
+
+    const legacyCount = (testDb.prepare('SELECT COUNT(*) as c FROM audit_log').get() as { c: number }).c;
+    const result = await auditLog.count();
+    expect(result).toBe(legacyCount);
+    expect(result).toBe(2);
+  });
 });