feat(db): request-context and unit-of-work helpers for the ORM, with concurrency tests

server/src/nest/app.module.ts

@@ -26,6 +26,7 @@ import { TrekExceptionFilter } from './common/trek-exception.filter';
 import { ZodValidationPipe } from './common/zod-validation.pipe';
 import { ConfigModule } from './config/config.module';
 import { DatabaseModule } from './database/database.module';
+import { OrmModule } from './database/orm.module';
 import { DayNotesModule } from './day-notes/day-notes.module';
 import { DaysModule } from './days/days.module';
 import { DocSyncModule } from './doc-sync/doc-sync.module';
@@ -151,6 +152,9 @@ import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core';
     LlmParseModule,
     ManagedExtModule,
     MikroOrmModule.forRoot(mikroOrmConfig),
+    // Not part of DatabaseModule: 41 e2e suites compose that module without the
+    // ORM, and an EntityManager-dependent provider there fails their containers.
+    OrmModule,
   ],
   providers: [
     // Default-deny: a route is authenticated unless it carries @Public() or

server/src/nest/database/orm.module.ts

@@ -0,0 +1,18 @@
+import { Global, Module } from '@nestjs/common';
+import { UnitOfWork } from './unit-of-work';
+
+/**
+ * The ORM-side container module: the home of providers that depend on MikroORM
+ * itself rather than on the raw better-sqlite3 handle `DatabaseModule` owns.
+ *
+ * Deliberately separate from `DatabaseModule`, which 41 e2e suites compose
+ * standalone (`Test.createTestingModule({ imports: [DatabaseModule, ...] })`)
+ * with no ORM in the container: a provider there that injects the core
+ * `EntityManager` fails every one of them at `compile()`, because Nest
+ * instantiates providers eagerly. Nothing is imported here — the
+ * `EntityManager` token comes from `MikroOrmModule.forRoot`, whose
+ * `MikroOrmCoreModule` is itself `@Global()`.
+ */
+@Global()
+@Module({ providers: [UnitOfWork], exports: [UnitOfWork] })
+export class OrmModule {}

server/src/nest/database/request-context.ts

@@ -0,0 +1,16 @@
+import { RequestContext, type MikroORM } from '@mikro-orm/core';
+
+/**
+ * A request-scoped EntityManager for work that no HTTP request wraps.
+ *
+ * `@mikro-orm/nestjs` forks an EntityManager per HTTP request through its
+ * middleware; everything else — an MCP tool call, a plugin RPC dispatch, a
+ * WebSocket message, a cron job, the boot-time seeders, a backup restore — has
+ * no such wrapper, and with `allowGlobalContext` left at its safe default the
+ * ORM refuses to run a query there. This is the one helper those entrypoints
+ * call. It is deliberately a function and not a decorator so a call site can
+ * pass the ORM it was given rather than reach for a global.
+ */
+export function withRequestContext<T>(orm: MikroORM, fn: () => T): T {
+  return RequestContext.create(orm.em, fn);
+}

server/src/nest/database/unit-of-work.ts

@@ -0,0 +1,28 @@
+import { Injectable } from '@nestjs/common';
+import { EntityManager } from '@mikro-orm/core';
+
+/**
+ * The one way a service opens a transaction.
+ *
+ * Wraps `em.transactional()` with its default `NESTED` propagation: a call made
+ * while a transaction is open becomes a savepoint, which is what better-sqlite3's
+ * nested `transaction()` did and what the services were written against.
+ *
+ * Repositories injected by Nest (and the test helper's `t.repo`) resolve the
+ * transactional fork through MikroORM's TransactionContext, so nothing has to
+ * be passed down. A manually forked EntityManager does not: `fork()` defaults
+ * to `useContext: false`, so its `getContext()` returns itself, and a repository
+ * built on it would write outside the transaction, on a second connection the
+ * transaction is holding.
+ *
+ * Never awaits network I/O inside: the SQLite connection is one Kysely client
+ * with a mutex, and an open transaction holds it for every other request.
+ */
+@Injectable()
+export class UnitOfWork {
+  constructor(private readonly em: EntityManager) {}
+
+  transactional<T>(fn: () => Promise<T>): Promise<T> {
+    return this.em.transactional(() => fn());
+  }
+}

server/tests/helpers/test-orm.ts

@@ -6,7 +6,7 @@ import { createBoundSqliteDriver } from '../../src/db/orm-driver';
 
 export interface TestOrm {
   orm: MikroORM;
-  /** A fork of the global EntityManager; `clear()` empties its identity map. */
+  /** The global, context-resolving EntityManager; `clear()` empties its identity map. */
   em: EntityManager;
   repo<T extends object>(entity: EntityClass<T>): GetRepository<T, EntityRepository<T>>;
   clear(): void;
@@ -24,6 +24,15 @@ export interface TestOrm {
  * Production keeps the default (false), which is what `withRequestContext` in
  * `src/nest/database/request-context.ts` is for.
  *
+ * `em` and `repo()` hand out the ORM's global EntityManager rather than a fork
+ * on purpose: a forked EntityManager has `useContext: false`, so its
+ * `getContext()` returns itself and ignores `TransactionContext`. A `t.repo(X)`
+ * built on a fork and used inside `uow.transactional` would write outside the
+ * open transaction, on a second connection the transaction is holding, and
+ * deadlock on Kysely's connection mutex. The helper therefore hands out the
+ * same context-resolving global EM that Nest injects into services, which
+ * resolves the transactional fork the way production does.
+ *
  * `close()` on the underlying MikroORM connection ignores the `force` flag and
  * always tears down its Kysely client — what actually keeps the handle open
  * is `NonClosingSqliteDriver` in `../../src/db/orm-driver.ts`, which
@@ -42,12 +51,11 @@ export async function createTestOrm(
     allowGlobalContext: options.allowGlobalContext ?? true,
     discovery: { warnWhenNoEntities: false },
   });
-  const em = orm.em.fork();
   return {
     orm,
-    em,
-    repo: (entity) => em.getRepository(entity),
-    clear: () => em.clear(),
+    em: orm.em,
+    repo: (entity) => orm.em.getRepository(entity),
+    clear: () => orm.em.clear(),
     close: async () => {
       // The `force` flag is irrelevant here — MikroORM's connection.close()
       // always destroys its Kysely client either way. It's the driver's

server/tests/unit/nest/database/request-context.test.ts

@@ -0,0 +1,39 @@
+import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+import { createTestDb, resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { createUser } from '../../../helpers/factories';
+import { Users } from '../../../../src/db/entities/Users.entity';
+import { withRequestContext } from '../../../../src/nest/database/request-context';
+
+const testDb = createTestDb();
+let t: TestOrm;
+
+// Global context DISALLOWED here on purpose: this is the production setting.
+beforeAll(async () => { t = await createTestOrm(testDb, { allowGlobalContext: false }); });
+beforeEach(() => resetTestDb(testDb));
+afterAll(async () => { await t.close(); testDb.close(); });
+
+describe('withRequestContext', () => {
+  it('CTX-001: the global EntityManager refuses a query outside a context', async () => {
+    createUser(testDb);
+    await expect(t.orm.em.find(Users, {})).rejects.toThrow(/global EntityManager|global context/i);
+  });
+
+  it('CTX-002: inside the helper the same call succeeds on a request-scoped fork', async () => {
+    const { user } = createUser(testDb);
+    const found = await withRequestContext(t.orm, () => t.orm.em.findOne(Users, { id: user.id }));
+    expect(found?.id).toBe(user.id);
+  });
+
+  it('CTX-003: two contexts do not share an identity map', async () => {
+    const { user } = createUser(testDb);
+    const a = await withRequestContext(t.orm, () => t.orm.em.findOneOrFail(Users, { id: user.id }));
+    const b = await withRequestContext(t.orm, () => t.orm.em.findOneOrFail(Users, { id: user.id }));
+    // Compared as a boolean, not with `expect(a).not.toBe(b)`: on a failed
+    // `Object.is`, vitest's `toBe` still runs a deep `equals()` with
+    // `iterableEquality` to offer the "replace toBe with toEqual" hint, and
+    // that iterates these entities' uninitialized `Collection` properties,
+    // which throw. The assertion below is the same identity check without it.
+    expect(a === b).toBe(false);
+  });
+});

server/tests/unit/nest/database/unit-of-work.test.ts

@@ -0,0 +1,94 @@
+import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+import { createTestDb, resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { createTrip, createUser } from '../../../helpers/factories';
+import { Days } from '../../../../src/db/entities/Days.entity';
+import type { DaysRepository } from '../../../../src/db/repositories/Days.repository';
+import { UnitOfWork } from '../../../../src/nest/database/unit-of-work';
+
+const testDb = createTestDb();
+let t: TestOrm;
+let uow: UnitOfWork;
+let days: DaysRepository;
+
+/**
+ * `t.em` is the ORM's context-resolving global EntityManager and `t.repo()`
+ * builds on it, which is what makes this suite meaningful: the repository resolves the
+ * transactional fork out of MikroORM's TransactionContext, exactly as a
+ * Nest-injected repository does. A manually forked EntityManager would not
+ * (`fork()` defaults to `useContext: false`) — it would write outside the open
+ * transaction, on a second connection the transaction is holding, and deadlock
+ * on Kysely's connection mutex. See the docstring on `createTestOrm`.
+ */
+beforeAll(async () => {
+  t = await createTestOrm(testDb);
+  uow = new UnitOfWork(t.em);
+  days = t.repo(Days) as DaysRepository;
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
+
+const countDays = (tripId: number) =>
+  (testDb.prepare('SELECT COUNT(*) AS n FROM days WHERE trip_id = ?').get(tripId) as { n: number }).n;
+
+describe('UnitOfWork.transactional', () => {
+  it('UOW-001: a throw rolls every statement back', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    await expect(
+      uow.transactional(async () => {
+        await days.createDay({ trip_id: trip.id, day_number: 1, date: null, notes: null });
+        throw new Error('boom');
+      }),
+    ).rejects.toThrow('boom');
+    expect(countDays(trip.id)).toBe(0);
+  });
+
+  it('UOW-002: an inner transaction is a savepoint — its failure leaves the outer writes intact', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    await uow.transactional(async () => {
+      await days.createDay({ trip_id: trip.id, day_number: 1, date: null, notes: null });
+      await uow
+        .transactional(async () => {
+          await days.createDay({ trip_id: trip.id, day_number: 2, date: null, notes: null });
+          throw new Error('inner');
+        })
+        .catch(() => undefined);
+    });
+    expect(countDays(trip.id)).toBe(1);
+  });
+
+  it('UOW-003: concurrent transactions serialise on the one connection instead of throwing', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const order: string[] = [];
+    const tx = (label: string, dayNumber: number) =>
+      uow.transactional(async () => {
+        order.push(`${label}:in`);
+        await days.createDay({ trip_id: trip.id, day_number: dayNumber, date: null, notes: null });
+        await new Promise((r) => setTimeout(r, 20));
+        order.push(`${label}:out`);
+      });
+    await Promise.all([tx('a', 1), tx('b', 2)]);
+    expect(countDays(trip.id)).toBe(2);
+    // The second never enters before the first leaves.
+    expect(order.indexOf('b:in')).toBeGreaterThan(order.indexOf('a:out'));
+  });
+
+  it('UOW-004: a read outside the transaction waits for it and then sees the commit', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const tx = uow.transactional(async () => {
+      await days.createDay({ trip_id: trip.id, day_number: 1, date: null, notes: null });
+      await new Promise((r) => setTimeout(r, 20));
+    });
+    // A separate fork = a separate acquirer of the connection. It waits on the
+    // ConnectionMutex instead of deadlocking because nothing inside the
+    // transaction waits on it: the writer's own timer resolves regardless.
+    const reader = t.orm.em.fork().getRepository(Days) as DaysRepository;
+    const seen = await reader.listByTrip(trip.id);
+    await tx;
+    expect(seen).toHaveLength(1);
+  });
+});