From 0ec611eb98778499e863fd39270b7d63f738750a Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 10 Aug 2026 21:37:45 +0200 Subject: [PATCH] refactor(server)!: read the reminder gates per tick and drop the app-settings restart The trip/todo reminder enable gates (notify_trip_reminder, notify_todo_due) are now read inside each 9 AM tick instead of once at boot, so toggling them takes effect at the next run without a restart. Saving notification settings no longer re-arms anything: the auth-side startTripReminders() call and the shouldRestartScheduler result field are gone (no controller consumed it, so the PUT /api/auth/app-settings response shape is unchanged). The boot 'enabled/disabled' log lines are informational banners now. --- server/src/nest/auth/auth.service.ts | 12 ++---- server/src/scheduler.ts | 37 +++++++++++++------ server/tests/unit/nest/auth.service.test.ts | 11 ++---- server/tests/unit/nest/oidc.service.test.ts | 6 --- .../tests/unit/nest/passkey.service.test.ts | 6 --- 5 files changed, 31 insertions(+), 41 deletions(-) diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index a0dd80b81..bbbb356d1 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -19,7 +19,6 @@ import { EphemeralTokenService } from './ephemeral-token.service'; // 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. import { revokeUserSessions } from '../../mcp/sessionManager'; -import { startTripReminders } from '../../scheduler'; import { UserCleanupService } from './user-cleanup.service'; import { emitUserDeleted } from '../../plugin-user-lifecycle'; import { verifyJwtAndLoadUser } from './jwt-verify'; @@ -598,7 +597,6 @@ export class AuthService { success?: boolean; auditSummary?: Record; auditDebugDetails?: Record; - shouldRestartScheduler?: boolean; } { const body = rawBody as Record; const user = this.db.get<{ role: string }>('SELECT role FROM users WHERE id = ?', userId); @@ -665,13 +663,9 @@ export class AuthService { debugDetails[k] = k === 'smtp_pass' ? '***' : body[k]; } - const notifRelated = ['notification_channels', 'smtp_host']; - const shouldRestartScheduler = changedKeys.some(k => notifRelated.includes(k)); - if (shouldRestartScheduler) { - startTripReminders(); - } - - return { success: true, auditSummary: summary, auditDebugDetails: debugDetails, shouldRestartScheduler }; + // The reminder crons read their enable gates and channels per tick, so a + // notification-settings change takes effect at the next run — no restart. + return { success: true, auditSummary: summary, auditDebugDetails: debugDetails }; } // ------------------------------------------------------------------------- diff --git a/server/src/scheduler.ts b/server/src/scheduler.ts index 8b5d5847f..b697ab799 100644 --- a/server/src/scheduler.ts +++ b/server/src/scheduler.ts @@ -50,6 +50,9 @@ let reminderTask: ScheduledTask | null = null; function startTripReminders(): void { if (reminderTask) { reminderTask.stop(); reminderTask = null; } + // Boot banner only — the enable gate is read per tick below (airtrail-style), + // so toggling notify_trip_reminder takes effect at the next 9 AM run without + // a restart. The banner reflects the state at boot. try { const { db } = require('./db/database'); const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; @@ -58,19 +61,20 @@ function startTripReminders(): void { const activeChannels = channelsRaw === 'none' ? [] : channelsRaw.split(',').map((c: string) => c.trim()); if (!reminderEnabled) { logInfo('Trip reminders: disabled in settings'); - return; + } else { + const tripCount = (db.prepare('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL').get() as { c: number }).c; + logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`); } - - const tripCount = (db.prepare('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL').get() as { c: number }).c; - logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`); } catch { - return; + /* banner is best-effort */ } const tz = readEnv().app.tz || 'UTC'; reminderTask = cron.schedule('0 9 * * *', async () => { try { const { db } = require('./db/database'); + const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; + if (getSetting('notify_trip_reminder') === 'false') return; const { send } = require('./nest/notifications/notifications.bridge'); const trips = db.prepare(` @@ -104,18 +108,27 @@ let todoReminderTask: ScheduledTask | null = null; function startTodoReminders(): void { if (todoReminderTask) { todoReminderTask.stop(); todoReminderTask = null; } - const { db } = require('./db/database'); - const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; - const enabled = getSetting('notify_todo_due') !== 'false'; - if (!enabled) { - logInfo('Todo due reminders: disabled in settings'); - return; + // Boot banner only — the enable gate is read per tick below (airtrail-style), + // so toggling notify_todo_due takes effect at the next 9 AM run without a + // restart. The banner reflects the state at boot. + try { + const { db } = require('./db/database'); + const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; + if (getSetting('notify_todo_due') !== 'false') { + logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`); + } else { + logInfo('Todo due reminders: disabled in settings'); + } + } catch { + /* banner is best-effort */ } - logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`); const tz = readEnv().app.tz || 'UTC'; todoReminderTask = cron.schedule('0 9 * * *', async () => { try { + const { db } = require('./db/database'); + const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; + if (getSetting('notify_todo_due') === 'false') return; const { send } = require('./nest/notifications/notifications.bridge'); // Select unchecked todos with a due date inside the lead window diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 16f3fde43..09c527007 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -55,12 +55,6 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ })); vi.mock('../../../src/nest/auth/ephemeral-tokens', () => ({ createEphemeralToken: vi.fn() })); vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() })); -vi.mock('../../../src/scheduler', () => ({ - startTripReminders: vi.fn(), - buildCronExpression: vi.fn(), - loadSettings: vi.fn(() => ({ enabled: false })), - VALID_INTERVALS: ['daily', 'weekly', 'monthly'], -})); // --------------------------------------------------------------------------- // Imports (after mocks) @@ -736,12 +730,13 @@ describe('updateAppSettings', () => { expect(result).toEqual({ error: 'Cannot disable all login methods. At least one must remain enabled.', status: 400 }); }); - it('AUTH-DB-075: the smtp_pass masking sentinel is skipped, notification changes flag a scheduler restart', () => { + it('AUTH-DB-075: the smtp_pass masking sentinel is skipped on a notification-settings change', () => { + // The reminder crons read their gates per tick now, so a settings change no + // longer flags (or needs) a scheduler restart. const { user } = createAdmin(testDb); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('smtp_pass', 'stored')").run(); const result = svc.updateAppSettings(user.id, { smtp_pass: '••••••••', notification_channels: 'email' }); expect(result.success).toBe(true); - expect(result.shouldRestartScheduler).toBe(true); const { value } = testDb.prepare("SELECT value FROM app_settings WHERE key = 'smtp_pass'").get() as { value: string }; expect(value).toBe('stored'); // sentinel never overwrites the secret expect(result.auditDebugDetails).not.toHaveProperty('smtp_pass', 'stored'); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 9d66074d1..dfaf17b4c 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -63,12 +63,6 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ })); vi.mock('../../../src/nest/auth/ephemeral-tokens', () => ({ createEphemeralToken: vi.fn() })); vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() })); -vi.mock('../../../src/scheduler', () => ({ - startTripReminders: vi.fn(), - buildCronExpression: vi.fn(), - loadSettings: vi.fn(() => ({ enabled: false })), - VALID_INTERVALS: ['daily', 'weekly', 'monthly'], -})); const { getAppUrlMock } = vi.hoisted(() => ({ getAppUrlMock: vi.fn() })); vi.mock('../../../src/app-config', async (importOriginal) => { diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index 5877d8c0d..44b844be0 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -55,12 +55,6 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ })); vi.mock('../../../src/nest/auth/ephemeral-tokens', () => ({ createEphemeralToken: vi.fn() })); vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() })); -vi.mock('../../../src/scheduler', () => ({ - startTripReminders: vi.fn(), - buildCronExpression: vi.fn(), - loadSettings: vi.fn(() => ({ enabled: false })), - VALID_INTERVALS: ['daily', 'weekly', 'monthly'], -})); // The WebAuthn ceremony verdicts are the mock boundary — the service must // persist ONLY what the verifier vouches for, so the tests hand it verdicts.