refactor(server): move the version check and demo reset into the admin domain

server/src/index.ts

@@ -101,8 +101,6 @@ const onListen = () => {
     // belongs at boot.
     airtrail: nestApp.get(AirtrailSyncService),
   });
-  scheduler.startVersionCheck();
-  scheduler.startDemoReset();
   scheduler.startIdempotencyCleanup();
   scheduler.startTrekPhotoCacheCleanup();
   scheduler.startPlacePhotoCacheCleanup();

server/src/nest/admin/admin.bridge.ts

@@ -1,58 +0,0 @@
-import { db } from '../../db/database';
-import { DatabaseService } from '../database/database.service';
-import { RealtimeService } from '../realtime/realtime.service';
-import { AddonsService } from '../addons/addons.service';
-import { SettingsService } from '../settings/settings.service';
-import { AuthService } from '../auth/auth.service';
-import { PasskeyService } from '../auth/passkey.service';
-import { UserCleanupService } from '../auth/user-cleanup.service';
-import { WebauthnConfigService } from '../auth/webauthn-config.service';
-import { TripMembershipService } from '../trip-membership/trip-membership.service';
-import { PackingService } from '../packing/packing.service';
-import { PermissionsService } from '../permissions/permissions.service';
-import { MailerService } from '../notifications/mailer/mailer.service';
-import { NotificationPreferencesService } from '../notifications/notification-preferences.service';
-import { NtfyService } from '../notifications/transports/ntfy.service';
-import { WebhookService } from '../notifications/transports/webhook.service';
-import { NotificationsService } from '../notifications/notifications.service';
-import { AdminService } from './admin.service';
-import { EphemeralTokenService } from '../auth/ephemeral-token.service';
-
-/**
- * Non-Nest entry point for the admin domain — for code running OUTSIDE the Nest
- * container. Exactly one consumer: the daily version-check cron in
- * scheduler.ts, which lazy-requires this instead of the retired
- * services/adminService. Exports the legacy name 1:1 so the repoint was an
- * import-path-only diff. Inside the container, inject AdminService instead.
- * Delete this file if the scheduler ever moves into Nest's @nestjs/schedule.
- *
- * The 5-minute version cache is module-scoped in admin.helpers.ts, so this
- * instance and the container singleton share it — the cron and
- * GET /api/admin/version-check hit GitHub once between them, as they did before
- * the fold.
- *
- * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto
- * the shared better-sqlite3 singleton.
- */
-const dbs = new DatabaseService(db);
-const realtime = new RealtimeService();
-const permissions = new PermissionsService(dbs);
-const webauthn = new WebauthnConfigService(dbs);
-const userCleanup = new UserCleanupService(dbs);
-const mailer = new MailerService(dbs);
-const notifPrefs = new NotificationPreferencesService(dbs, mailer);
-const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, mailer, new EphemeralTokenService());
-const admin = new AdminService(
-  dbs,
-  new AddonsService(dbs),
-  new PasskeyService(dbs, auth, webauthn),
-  auth,
-  permissions,
-  new NotificationsService(dbs, realtime, mailer, new WebhookService(dbs), new NtfyService(dbs), notifPrefs),
-  userCleanup,
-  new RealtimeService(),
-);
-
-export function checkAndNotifyVersion(): Promise<void> {
-  return admin.checkAndNotifyVersion();
-}

server/src/nest/admin/admin.helpers.ts

@@ -6,11 +6,13 @@ import fs from 'fs';
  * atlas-geo precedent).
  *
  * The version cache is module-scoped **on purpose** (the permissions-cache
- * precedent, permissions.bridge.ts): scheduler.ts reaches checkAndNotifyVersion
- * through admin.bridge's instance while GET /api/admin/version-check runs on the
- * container singleton, and the legacy module shared one 5-minute cache between
- * them. Instance state would double the GitHub traffic and let the scheduler
- * notify on a version the UI has not seen yet.
+ * precedent, permissions.bridge.ts): the daily version-check cron
+ * (VersionCheckJob) and GET /api/admin/version-check both run on the container
+ * singleton now, but the module scope keeps the one-5-minute-cache guarantee
+ * independent of how many AdminService instances exist (the legacy module and
+ * the retired admin bridge relied on exactly that). Instance state would double
+ * the GitHub traffic and let the cron notify on a version the UI has not seen
+ * yet.
  */
 
 /** bcrypt cost factor for user passwords — kept in sync with authService. */

server/src/nest/admin/admin.module.ts

@@ -3,6 +3,9 @@ import { TokensModule } from '../tokens/tokens.module';
 import { OauthModule } from '../oauth/oauth.module';
 import { AdminController } from './admin.controller';
 import { AdminService } from './admin.service';
+import { VersionCheckJob } from './version-check.job';
+import { DemoResetJob } from './demo-reset.job';
+import { SchedulingModule } from '../scheduling/scheduling.module';
 import { PluginsRuntimeModule } from '../plugins/plugins-runtime.module';
 import { SettingsModule } from '../settings/settings.module';
 import { AuditModule } from '../audit/audit.module';
@@ -20,8 +23,8 @@ import { PackingModule } from '../packing/packing.module';
 import { PermissionsModule } from '../permissions/permissions.module';
 
 @Module({
-  imports: [PluginsRuntimeModule, SettingsModule, AuditModule, AddonsModule, AuthModule, NotificationsModule, PackingModule, PermissionsModule, TokensModule, OauthModule],
+  imports: [PluginsRuntimeModule, SettingsModule, AuditModule, AddonsModule, AuthModule, NotificationsModule, PackingModule, PermissionsModule, TokensModule, OauthModule, SchedulingModule],
   controllers: [AdminController],
-  providers: [AdminService],
+  providers: [AdminService, VersionCheckJob, DemoResetJob],
 })
 export class AdminModule {}

server/src/nest/admin/admin.service.ts

@@ -10,7 +10,7 @@ import { updateJwtSecret } from '../../config';
 // the whole tools fan-out (and via the domain bridges, the Nest services) into
 // every consumer of this module — a nest→mcp→nest module cycle. The
 // invalidateMcpSessions barrel import below predates the fold and is deliberately
-// separate: it is only reached from the controller, never from admin.bridge.
+// separate: it is only reached from the controller, never from the cron path.
 import { revokeUserSessions, revokeUserSessionsForClient } from '../../mcp/sessionManager';
 import { invalidateMcpSessions } from '../../mcp';
 import { emitUserDeleted } from '../../plugin-user-lifecycle';

server/src/nest/admin/demo-reset.job.ts

@@ -0,0 +1,35 @@
+import { Injectable, type OnApplicationBootstrap } from '@nestjs/common';
+import { logInfo, logError } from '../audit/audit-log.logger';
+import { RuntimeEnvService } from '../app-config/runtime-env.service';
+import { CronRegistrarService } from '../scheduling/cron-registrar.service';
+import { resetDemoUser } from '../../demo/demo-reset';
+
+/**
+ * Demo mode: hourly reset of demo user data (moved from src/scheduler.ts).
+ * Gated on DEMO_MODE at bootstrap — parity: toggling it always required a
+ * restart — and scheduled in the server-local zone (the old cron.schedule call
+ * passed no timezone). demo-reset is a static import: its module top is
+ * side-effect-free, and the dangerous closeDb/reinitialize sequence stays
+ * inside resetDemoUser, called at tick time.
+ */
+@Injectable()
+export class DemoResetJob implements OnApplicationBootstrap {
+  constructor(
+    private readonly runtimeEnv: RuntimeEnvService,
+    private readonly registrar: CronRegistrarService,
+  ) {}
+
+  onApplicationBootstrap(): void {
+    if (!this.registrar.isEnabled() || !this.runtimeEnv.isDemoMode()) return;
+    this.registrar.register('demo-reset', '0 * * * *', () => this.tick(), { timezone: 'none' });
+    logInfo('Demo hourly reset scheduled');
+  }
+
+  tick(): void {
+    try {
+      resetDemoUser();
+    } catch (err: unknown) {
+      logError(`Demo reset: ${err instanceof Error ? err.message : err}`);
+    }
+  }
+}

server/src/nest/admin/version-check.job.ts

@@ -0,0 +1,32 @@
+import { Injectable, type OnApplicationBootstrap } from '@nestjs/common';
+import { logError } from '../audit/audit-log.logger';
+import { AdminService } from './admin.service';
+import { CronRegistrarService } from '../scheduling/cron-registrar.service';
+
+/**
+ * Daily version check (moved from src/scheduler.ts — it was the only consumer
+ * of the old admin bridge, which died with the move). checkAndNotifyVersion
+ * shares the module-scoped 5-minute cache in admin.helpers.ts with
+ * GET /api/admin/version-check, so the cron and the route hit GitHub once
+ * between them. No boot log — parity with the old starter.
+ */
+@Injectable()
+export class VersionCheckJob implements OnApplicationBootstrap {
+  constructor(
+    private readonly admin: AdminService,
+    private readonly registrar: CronRegistrarService,
+  ) {}
+
+  onApplicationBootstrap(): void {
+    if (!this.registrar.isEnabled()) return;
+    this.registrar.register('version-check', '0 9 * * *', () => this.tick());
+  }
+
+  async tick(): Promise<void> {
+    try {
+      await this.admin.checkAndNotifyVersion();
+    } catch (err: unknown) {
+      logError(`Version check: ${err instanceof Error ? err.message : err}`);
+    }
+  }
+}

server/src/scheduler.ts

@@ -26,41 +26,6 @@ export function setSchedulerDeps(next: SchedulerDeps): void {
   deps = next;
 }
 
-// Demo mode: hourly reset of demo user data
-let demoTask: ScheduledTask | null = null;
-
-function startDemoReset(): void {
-  if (demoTask) { demoTask.stop(); demoTask = null; }
-  if (!readEnv().demo.enabled) return;
-
-  demoTask = cron.schedule('0 * * * *', () => {
-    try {
-      const { resetDemoUser } = require('./demo/demo-reset');
-      resetDemoUser();
-    } catch (err: unknown) {
-      logError(`Demo reset: ${err instanceof Error ? err.message : err}`);
-    }
-  });
-  logInfo('Demo hourly reset scheduled');
-}
-
-// Version check: daily at 9 AM — notify admins if a new TREK release is available
-let versionCheckTask: ScheduledTask | null = null;
-
-function startVersionCheck(): void {
-  if (versionCheckTask) { versionCheckTask.stop(); versionCheckTask = null; }
-
-  const tz = readEnv().app.tz || 'UTC';
-  versionCheckTask = cron.schedule('0 9 * * *', async () => {
-    try {
-      const { checkAndNotifyVersion } = require('./nest/admin/admin.bridge');
-      await checkAndNotifyVersion();
-    } catch (err: unknown) {
-      logError(`Version check: ${err instanceof Error ? err.message : err}`);
-    }
-  }, { timezone: tz });
-}
-
 // Idempotency key cleanup: nightly at 3 AM — delete keys past their TTL.
 // The TTL must exceed any realistic offline window: the TREK client replays
 // queued mutations with their X-Idempotency-Key when it reconnects, so a key
@@ -200,12 +165,10 @@ function startAirTrailSync(): void {
 }
 
 function stop(): void {
-  if (demoTask) { demoTask.stop(); demoTask = null; }
-  if (versionCheckTask) { versionCheckTask.stop(); versionCheckTask = null; }
   if (idempotencyCleanupTask) { idempotencyCleanupTask.stop(); idempotencyCleanupTask = null; }
   if (trekPhotoCacheTask) { trekPhotoCacheTask.stop(); trekPhotoCacheTask = null; }
   if (placePhotoCacheTask) { placePhotoCacheTask.stop(); placePhotoCacheTask = null; }
   if (airtrailSyncTask) { airtrailSyncTask.stop(); airtrailSyncTask = null; }
 }
 
-export { stop, startDemoReset, startVersionCheck, startIdempotencyCleanup, purgeExpiredIdempotencyKeys, startTrekPhotoCacheCleanup, startPlacePhotoCacheCleanup, startAirTrailSync };
+export { stop, startIdempotencyCleanup, purgeExpiredIdempotencyKeys, startTrekPhotoCacheCleanup, startPlacePhotoCacheCleanup, startAirTrailSync };

server/tests/unit/nest/admin-jobs.test.ts

@@ -0,0 +1,87 @@
+/**
+ * The admin domain's cron providers — VersionCheckJob and DemoResetJob
+ * (moved from src/scheduler.ts). Registration, gating, and tick error
+ * containment; the version-check end-to-end path is VCJOB-001 in
+ * admin.service.test.ts.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const logMock = vi.hoisted(() => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logError: vi.fn(), logWarn: vi.fn(), logDebug: vi.fn() }));
+const { resetDemoUserMock } = vi.hoisted(() => ({ resetDemoUserMock: vi.fn() }));
+
+vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock);
+vi.mock('../../../src/demo/demo-reset', () => ({ resetDemoUser: resetDemoUserMock, saveBaseline: vi.fn(), hasBaseline: vi.fn() }));
+
+import { VersionCheckJob } from '../../../src/nest/admin/version-check.job';
+import { DemoResetJob } from '../../../src/nest/admin/demo-reset.job';
+import type { AdminService } from '../../../src/nest/admin/admin.service';
+import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
+import type { CronRegistrarService } from '../../../src/nest/scheduling/cron-registrar.service';
+
+function registrarStub(enabled = true) {
+  return {
+    isEnabled: vi.fn(() => enabled),
+    register: vi.fn(() => enabled),
+    unregister: vi.fn(),
+  };
+}
+
+beforeEach(() => vi.clearAllMocks());
+
+describe('VersionCheckJob', () => {
+  it('AJOB-001 — registers the daily 9 AM cron, with no boot log (parity)', () => {
+    const registrar = registrarStub();
+    new VersionCheckJob({} as AdminService, registrar as unknown as CronRegistrarService).onApplicationBootstrap();
+    expect(registrar.register).toHaveBeenCalledWith('version-check', '0 9 * * *', expect.any(Function));
+    expect(logMock.logInfo).not.toHaveBeenCalled();
+  });
+
+  it('AJOB-002 — does not register under the test gate', () => {
+    const registrar = registrarStub(false);
+    new VersionCheckJob({} as AdminService, registrar as unknown as CronRegistrarService).onApplicationBootstrap();
+    expect(registrar.register).not.toHaveBeenCalled();
+  });
+
+  it('AJOB-003 — a throwing check is contained to the Version check log line', async () => {
+    const admin = { checkAndNotifyVersion: vi.fn().mockRejectedValue(new Error('github down')) } as unknown as AdminService;
+    const job = new VersionCheckJob(admin, registrarStub() as unknown as CronRegistrarService);
+    await expect(job.tick()).resolves.toBeUndefined();
+    expect(logMock.logError).toHaveBeenCalledWith('Version check: github down');
+  });
+});
+
+describe('DemoResetJob', () => {
+  function make(demo: boolean, enabled = true) {
+    const registrar = registrarStub(enabled);
+    const runtimeEnv = { isDemoMode: () => demo } as RuntimeEnvService;
+    return { job: new DemoResetJob(runtimeEnv, registrar as unknown as CronRegistrarService), registrar };
+  }
+
+  it('AJOB-004 — registers the hourly server-local cron and logs the banner when demo mode is on', () => {
+    const { job, registrar } = make(true);
+    job.onApplicationBootstrap();
+    expect(registrar.register).toHaveBeenCalledWith('demo-reset', '0 * * * *', expect.any(Function), { timezone: 'none' });
+    expect(logMock.logInfo).toHaveBeenCalledWith('Demo hourly reset scheduled');
+  });
+
+  it('AJOB-005 — stays silent when demo mode is off or the gate is closed', () => {
+    const off = make(false);
+    off.job.onApplicationBootstrap();
+    expect(off.registrar.register).not.toHaveBeenCalled();
+
+    const gated = make(true, false);
+    gated.job.onApplicationBootstrap();
+    expect(gated.registrar.register).not.toHaveBeenCalled();
+    expect(logMock.logInfo).not.toHaveBeenCalled();
+  });
+
+  it('AJOB-006 — the tick runs resetDemoUser and contains a throw to the Demo reset log line', () => {
+    const { job } = make(true);
+    job.tick();
+    expect(resetDemoUserMock).toHaveBeenCalledTimes(1);
+
+    resetDemoUserMock.mockImplementation(() => { throw new Error('baseline gone'); });
+    expect(() => job.tick()).not.toThrow();
+    expect(logMock.logError).toHaveBeenCalledWith('Demo reset: baseline gone');
+  });
+});

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

@@ -6,7 +6,8 @@
  * their functions to tests/unit/nest/packing.service.test.ts.
  * Constructs the service directly over a real in-memory SQLite DB (repo
  * convention — no TestingModule). Focuses on validation/error branches that the
- * integration tests don't exercise. ADMIN-BR-001 pins the admin.bridge export.
+ * integration tests don't exercise. VCJOB-001 pins the version-check cron path
+ * (it replaced ADMIN-BR-001 when the old admin bridge died with the cron move).
  */
 import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest';
 
@@ -71,7 +72,8 @@ import { PackingService } from '../../../src/nest/packing/packing.service';
 import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
 import { NotificationsService } from '../../../src/nest/notifications/notifications.service';
 import { AdminService } from '../../../src/nest/admin/admin.service';
-import { checkAndNotifyVersion as bridgeCheckAndNotifyVersion } from '../../../src/nest/admin/admin.bridge';
+import { VersionCheckJob } from '../../../src/nest/admin/version-check.job';
+import type { CronRegistrarService } from '../../../src/nest/scheduling/cron-registrar.service';
 import { __clearVersionCacheForTests } from '../../../src/nest/admin/admin.helpers';
 import { makeNotificationsService, makeNotificationPreferencesService } from '../../helpers/notifications';
 import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
@@ -477,10 +479,12 @@ describe('updateAddon', () => {
   });
 });
 
-// ── admin.bridge ──────────────────────────────────────────────────────────────
+// ── version-check cron ────────────────────────────────────────────────────────
 
-describe('admin.bridge', () => {
-  it('ADMIN-BR-001 — checkAndNotifyVersion delegates to AdminService over the shared db Proxy', async () => {
+describe('version-check job', () => {
+  const registrarStub = { isEnabled: () => true, register: vi.fn(() => true), unregister: vi.fn() } as unknown as CronRegistrarService;
+
+  it('VCJOB-001 — the cron tick notifies and shares the module-scoped version cache with the route', async () => {
     createAdmin(testDb);
     __clearVersionCacheForTests();
     const fetchMock = vi.fn().mockResolvedValue({
@@ -490,15 +494,15 @@ describe('admin.bridge', () => {
     });
     vi.stubGlobal('fetch', fetchMock);
 
-    await bridgeCheckAndNotifyVersion();
+    await new VersionCheckJob(svc, registrarStub).tick();
 
     const notified = testDb
       .prepare('SELECT value FROM app_settings WHERE key = ?')
       .get('last_notified_version') as { value: string } | undefined;
     expect(notified?.value).toBe('99.9.9');
 
-    // The version cache is module-scoped in admin.helpers, so the bridge instance
-    // and the container singleton share it — the second read must not re-fetch.
+    // The version cache is module-scoped in admin.helpers, so the cron and
+    // GET /api/admin/version-check hit GitHub once between them.
     expect(await svc.checkVersion()).toMatchObject({ latest: '99.9.9' });
     expect(fetchMock).toHaveBeenCalledTimes(1);
     vi.unstubAllGlobals();