fix(server): plan 3a re-review 2 follow-ups — absent-ORM boot branch pinned; airports boot hook never rejects; BOOT-SWEEP-001 asserts the seven routes; duplicate comment removed

server/src/nest/airports/airports.service.ts

@@ -20,6 +20,17 @@ import { CronRegistrarService } from '../scheduling/cron-registrar.service';
  * `runOnBoot` has no `isEnabled()` gate (unlike `register()`), so — parity
  * with the pre-existing behavior — the backfill still runs in every test
  * harness that boots this service with a MikroORM, exactly as it did before.
+ *
+ * task-6-rereview2.md M3: the pre-change code could never reject —
+ * `onApplicationBootstrap` awaited one `try`/`catch` around the whole thing.
+ * Putting the `try`/`catch` only INSIDE the `runOnBoot` callback left a gap:
+ * `runOnBoot` itself (its `RequestContext.create`/context machinery) has no
+ * catch of its own, so anything it throws before the callback runs would
+ * propagate out of `onApplicationBootstrap` and abort `app.init()` — a total
+ * boot refusal where the old code swallowed everything. The outer
+ * `try`/`catch` below restores that parity while the inner one stays for its
+ * own reason: `backfillFlightEndpoints` failing must not surface as a
+ * `runOnBoot`-level error.
  */
 @Injectable()
 export class AirportsService implements OnApplicationBootstrap {
@@ -29,13 +40,17 @@ export class AirportsService implements OnApplicationBootstrap {
   ) {}
 
   async onApplicationBootstrap(): Promise<void> {
-    await this.registrar.runOnBoot('airports-flight-endpoints-boot', async () => {
-      try {
-        await this.backfillFlightEndpoints();
-      } catch (err) {
-        console.error('[DB] Flight endpoint backfill failed:', err);
-      }
-    });
+    try {
+      await this.registrar.runOnBoot('airports-flight-endpoints-boot', async () => {
+        try {
+          await this.backfillFlightEndpoints();
+        } catch (err) {
+          console.error('[DB] Flight endpoint backfill failed:', err);
+        }
+      });
+    } catch (err) {
+      console.error('[DB] Flight endpoint backfill failed:', err);
+    }
   }
 
   search(query: string): Airport[] {

server/src/nest/plugins/plugin-runtime.service.ts

@@ -281,18 +281,6 @@ export class PluginRuntimeService implements OnApplicationBootstrap, OnModuleDes
       // the boot RULING (task-6-rereview.md §2) settled for CronRegistrarService.
       // runOnBoot: no ORM means skip loudly and never call activate — never abort
       // app.init() over a partially-wired test graph.
-      // task-6-rereview.md I1: assertActivatable (inside activate) reads
-      // AddonsService.isAddonEnabled — repository-backed — so this boot loop needs
-      // the same request-context wrap every other D6 choke point got, or an
-      // enabled plugin with a non-empty requiredAddons throws cannotUseGlobalContext
-      // here, silently: the .catch below only reconciles PluginDependencyError /
-      // DependencyCycleError, and the outer try/catch around this whole block eats
-      // everything else.
-      // `activate()` itself stays unwrapped (its HTTP callers already run inside a
-      // request); only this boot-time call site needs it. Fail closed the same way
-      // the boot RULING (task-6-rereview.md §2) settled for CronRegistrarService.
-      // runOnBoot: no ORM means skip loudly and never call activate — never abort
-      // app.init() over a partially-wired test graph.
       if (!this.orm) {
         if (order.length) {
           logError(`PluginRuntimeService.onApplicationBootstrap: no MikroORM available — plugin(s) not activated: ${order.join(', ')}`);

server/tests/e2e/airports.e2e.test.ts

@@ -31,6 +31,7 @@ vi.mock('../../src/nest/airports/airports.data', async (importActual) => {
 import { AirportsModule } from '../../src/nest/airports/airports.module';
 import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
 import { TestUnitOfWorkModule } from '../helpers/test-uow';
+import { createTestMikroOrmModule } from '../helpers/test-orm';
 
 const BER = {
   iata: 'BER', icao: 'EDDB', name: 'Berlin Brandenburg', city: 'Berlin',
@@ -42,7 +43,13 @@ describe('Airports e2e (real auth guard + temp SQLite)', () => {
   let app: Awaited<ReturnType<typeof build>>;
 
   async function build() {
-    const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), AirportsModule] }).compile();
+    const moduleRef = await Test.createTestingModule({
+      // task-6-rereview2.md M4: without a MikroORM in the graph,
+      // CronRegistrarService.runOnBoot skips the backfill and logs a genuine
+      // [ERROR] line on every run of this file. createTestMikroOrmModule(db)
+      // wires one so the boot backfill actually runs here, same as production.
+      imports: [await TestUnitOfWorkModule.forRoot(db), createTestMikroOrmModule(db), AirportsModule],
+    }).compile();
     const nest = moduleRef.createNestApplication();
     nest.use(cookieParser());
     nest.useGlobalFilters(new TrekExceptionFilter());

server/tests/integration/boot-sweeps-request-context.test.ts

@@ -39,6 +39,20 @@
  * `Promise.all(collected)` give that unawaited call a chance to run (and log)
  * before the assertion, so the regression still fails the test even though
  * its promise was never in `collected`.
+ *
+ * task-6-rereview2.md M2: the log-line assertion alone is timing-based, not
+ * structural, and in practice only `journey-thumbs-boot` makes it
+ * load-bearing — the other six sweeps are raw SQL that never reaches the EM,
+ * so a job silently routed back to a bare `void this.sweep()` (bypassing
+ * `runOnBoot` entirely) would fail here only for journey-thumbs; the per-job
+ * unit test is what actually catches the rest. This file drives all seven
+ * jobs' REAL `onApplicationBootstrap`, each of which routes through
+ * `runOnBoot` — but that routing itself was previously unasserted here. The
+ * second expectation below asserts it directly: every one of the seven boot
+ * names was passed to `runOnBoot`, so a job re-routed around the registrar
+ * fails HERE, not only in its own `*-00N` per-job suite. `journey-thumbs-boot`
+ * remains the one sweep whose body reaches a repository, so it is still what
+ * the log-line assertion below is actually exercising.
  */
 import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
 import type { INestApplication } from '@nestjs/common';
@@ -81,7 +95,7 @@ describe('Every onApplicationBootstrap boot sweep runs inside a request context'
     testDb.close();
   });
 
-  it('BOOT-SWEEP-001: the seven jobs\' REAL onApplicationBootstrap, driven through the real production-wired CronRegistrarService, never logs cannotUseGlobalContext / "global EntityManager"', async () => {
+  it('BOOT-SWEEP-001: the seven jobs\' REAL onApplicationBootstrap, driven through the real production-wired CronRegistrarService, each routes through runOnBoot (structurally asserted), and journey-thumbs — the one sweep that reaches a repository today — never logs cannotUseGlobalContext / "global EntityManager"', async () => {
     const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
     const registrar = app.get(CronRegistrarService);
     const isEnabledSpy = vi.spyOn(registrar, 'isEnabled').mockReturnValue(true);
@@ -119,6 +133,20 @@ describe('Every onApplicationBootstrap boot sweep runs inside a request context'
         .map((args) => args.map(String).join(' '))
         .filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
       expect(suspicious).toEqual([]);
+
+      // task-6-rereview2.md M2: structural, not timing-based — a job routed back
+      // to a bare `void this.sweep()` (bypassing runOnBoot entirely) fails HERE
+      // by name, regardless of whether its sweep body happens to touch a
+      // repository yet.
+      expect(runOnBootSpy.mock.calls.map((c) => c[0]).sort()).toEqual([
+        'airtrail-sync-boot',
+        'dawarich-sync-boot',
+        'docsync-boot',
+        'journey-thumbs-boot',
+        'place-photo-cache-boot',
+        'reminder-jobs-boot',
+        'trek-photo-cache-boot',
+      ]);
     } finally {
       errSpy.mockRestore();
       isEnabledSpy.mockRestore();

server/tests/unit/nest/airports/airports.service.test.ts

@@ -78,4 +78,23 @@ describe('AirportsService boot backfill', () => {
       errSpy.mockRestore();
     }
   });
+
+  it('AIRPORTS-SVC-004 (task-6-rereview2.md M3): a rejecting runOnBoot (the context machinery itself, not the backfill body) resolves onApplicationBootstrap instead of aborting app.init()', async () => {
+    const registrar = {
+      isEnabled: vi.fn(() => true),
+      register: vi.fn(() => true),
+      unregister: vi.fn(),
+      runOnBoot: vi.fn(async () => { throw new Error('RequestContext.create blew up'); }),
+    };
+    const svc = new AirportsService(new DatabaseService(testDb), registrar as unknown as CronRegistrarService);
+    const backfillSpy = vi.spyOn(svc, 'backfillFlightEndpoints');
+    const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    try {
+      await expect(svc.onApplicationBootstrap()).resolves.toBeUndefined();
+      expect(backfillSpy).not.toHaveBeenCalled();
+      expect(errSpy).toHaveBeenCalledWith('[DB] Flight endpoint backfill failed:', expect.any(Error));
+    } finally {
+      errSpy.mockRestore();
+    }
+  });
 });

server/tests/unit/plugins/plugin-runtime.boot-no-orm.test.ts

@@ -0,0 +1,81 @@
+/**
+ * task-6-rereview2.md I-A: `PluginRuntimeService.onApplicationBootstrap`'s
+ * absent-ORM branch (`plugin-runtime.service.ts:~296-300`) was dead
+ * suite-wide — every existing construction of the service that reaches
+ * `onApplicationBootstrap` (boot-registry-order.test.ts, plugin-host.ts) hands
+ * it a real MikroORM. That branch is the half of the boot RULING
+ * (task-6-rereview.md §2) that makes "skip loudly, never abort app.init()"
+ * defensible: dropping the `logError` call, or falling through to `activate`
+ * anyway, would ship green with no test failing. This file pins it directly.
+ */
+import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from 'vitest';
+
+const logMock = vi.hoisted(() => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logError: vi.fn(), logWarn: vi.fn(), logDebug: vi.fn() }));
+vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock);
+
+import { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service';
+import { DatabaseService } from '../../../src/nest/database/database.service';
+import { AuditService } from '../../../src/nest/audit/audit.service';
+import { PluginUserSettingsService } from '../../../src/nest/plugins/plugin-user-settings.service';
+import { createTestAddonsService } from '../../helpers/test-addons';
+import { createSnapshotTestDb } from '../../helpers/db-mock';
+import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
+import { AuditLog } from '../../../src/db/entities/AuditLog.entity';
+import { Users } from '../../../src/db/entities/Users.entity';
+
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+
+beforeAll(async () => {
+  // Global context disallowed — the production setting — so the WITH-orm
+  // sibling case genuinely proves activation runs through a request context,
+  // the same reasoning airports.service.test.ts and BOOT-REG-002 use.
+  t = await createTestOrm(testDb, { allowGlobalContext: false });
+});
+afterAll(async () => { await t.close(); testDb.close(); });
+beforeEach(() => vi.clearAllMocks());
+
+/** installedDepRows() reads the `plugins` table directly — no discovery needed. */
+function installEnabledPlugin(id: string): void {
+  testDb
+    .prepare(
+      `INSERT INTO plugins (id, name, status, enabled, version, api_version, permissions, granted_permissions, capabilities, config, dependencies)
+       VALUES (?, ?, 'inactive', 1, '1.0.0', 1, '[]', '[]', '{}', '{}', '{}')`,
+    )
+    .run(id, id);
+}
+
+async function buildRuntime(withOrm: boolean): Promise<PluginRuntimeService> {
+  const dbs = new DatabaseService(testDb);
+  const audit = new AuditService(t.repo(AuditLog), t.repo(Users));
+  const addons = await createTestAddonsService(testDb, dbs);
+  const userSettings = new PluginUserSettingsService(dbs);
+  return new PluginRuntimeService(dbs, audit, addons, userSettings, undefined, undefined, undefined, withOrm ? t.orm : undefined);
+}
+
+describe('PluginRuntimeService.onApplicationBootstrap — absent-ORM boot branch (task-6-rereview2.md I-A)', () => {
+  it('RT-BOOT-NOORM-001: without an ORM, boot logs the skip line naming the plugin id and never calls activate', async () => {
+    installEnabledPlugin('rtboot-noorm');
+    const rt = await buildRuntime(false);
+    const activateSpy = vi.spyOn(rt, 'activate');
+
+    await rt.onApplicationBootstrap();
+
+    expect(logMock.logError).toHaveBeenCalledTimes(1);
+    const [message] = logMock.logError.mock.calls[0] as [string];
+    expect(message).toMatch(/PluginRuntimeService\.onApplicationBootstrap: no MikroORM available/);
+    expect(message).toContain('rtboot-noorm');
+    expect(activateSpy).not.toHaveBeenCalled();
+  });
+
+  it('RT-BOOT-NOORM-002: WITH an ORM, the same arrangement activates the plugin — proves case 001 is not vacuous', async () => {
+    installEnabledPlugin('rtboot-withorm');
+    const rt = await buildRuntime(true);
+    const activateSpy = vi.spyOn(rt, 'activate').mockResolvedValue(undefined);
+
+    await rt.onApplicationBootstrap();
+
+    expect(activateSpy).toHaveBeenCalledWith('rtboot-withorm');
+    expect(logMock.logError).not.toHaveBeenCalled();
+  });
+});

server/tests/unit/plugins/request-context.test.ts

@@ -115,21 +115,26 @@ describe('PluginSupervisor request context (D6, C3)', () => {
     permissions.invalidatePermissionsCache();
     const { supervisor, sup, sent } = makeSupervisor(); // no resolveOrm — the bug's exact reproduction
     const canCreateAsSpy = vi.spyOn(guards, 'canCreateAs');
-
-    await expect(
-      supervisor.onMessage(sup, { k: 'req', id: 'r1', method: 'trips.create', params: { _inv: 'inv-1' } }),
-    ).rejects.toThrow(/no ORM available/i);
-    // The dispatch itself never ran — same fail-closed guarantee as before.
-    expect(canCreateAsSpy).not.toHaveBeenCalled();
-    // But unlike the previous wave, the child DOES get a response: a HOST_ERROR
-    // envelope, matching the rate-limiter refusal's shape, so its ctx.* promise
-    // settles instead of hanging forever.
-    expect(sent).toHaveLength(1);
-    const res = sent[0];
-    expect(res.ok).toBe(false);
-    if (res.ok === false) {
-      expect(res.error.code).toBe('HOST_ERROR');
-      expect(res.error.message).toMatch(/no ORM available/i);
+    try {
+      await expect(
+        supervisor.onMessage(sup, { k: 'req', id: 'r1', method: 'trips.create', params: { _inv: 'inv-1' } }),
+      ).rejects.toThrow(/no ORM available/i);
+      // The dispatch itself never ran — same fail-closed guarantee as before.
+      expect(canCreateAsSpy).not.toHaveBeenCalled();
+      // But unlike the previous wave, the child DOES get a response: a HOST_ERROR
+      // envelope, matching the rate-limiter refusal's shape, so its ctx.* promise
+      // settles instead of hanging forever.
+      expect(sent).toHaveLength(1);
+      const res = sent[0];
+      expect(res.ok).toBe(false);
+      if (res.ok === false) {
+        expect(res.error.code).toBe('HOST_ERROR');
+        expect(res.error.message).toMatch(/no ORM available/i);
+      }
+    } finally {
+      // task-6-rereview2.md M5: never restored before — a leak waiting for the
+      // first mockImplementation another test in this file adds to `guards`.
+      canCreateAsSpy.mockRestore();
     }
   });