diff --git a/package-lock.json b/package-lock.json index e25eb6070..836dc3a30 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7384,13 +7384,6 @@ "undici-types": ">=7.24.0 <7.24.7" } }, - "node_modules/@types/node-cron": { - "version": "3.0.11", - "resolved": "https://registry.npmjs.org/@types/node-cron/-/node-cron-3.0.11.tgz", - "integrity": "sha512-0ikrnug3/IyneSHqCBeslAhlK2aBfYek1fGo4bP4QnZPmiqSGRK+Oy7ZMisLWkesffJvQ1cqAcBnJC+8+nxIAg==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/nodemailer": { "version": "8.0.1", "resolved": "https://registry.npmjs.org/@types/nodemailer/-/nodemailer-8.0.1.tgz", @@ -15231,15 +15224,6 @@ "node": ">=10" } }, - "node_modules/node-cron": { - "version": "4.4.1", - "resolved": "https://registry.npmjs.org/node-cron/-/node-cron-4.4.1.tgz", - "integrity": "sha512-1e4Gku1qeKx1ywNnMoA0o5hiZ8iA4klH84TIszTHZAME68xVIjLkiBo5KrtowbcOJn8MQqJG/yWHjC/dFdMWyQ==", - "license": "ISC", - "engines": { - "node": ">=20" - } - }, "node_modules/node-int64": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/node-int64/-/node-int64-0.4.0.tgz", @@ -21187,7 +21171,6 @@ "json5": "^2.2.3", "jsonwebtoken": "^9.0.2", "nestjs-zod": "^5.5.0", - "node-cron": "^4.2.1", "nodemailer": "^9.0.1", "otplib": "^12.0.1", "pdf-parse": "^2.4.5", @@ -21220,7 +21203,6 @@ "@types/jsonwebtoken": "^9.0.10", "@types/multer": "^2.1.0", "@types/node": "^25.5.0", - "@types/node-cron": "^3.0.11", "@types/nodemailer": "^8.0.1", "@types/qrcode": "^1.5.5", "@types/semver": "^7.7.1", diff --git a/server/package.json b/server/package.json index 94db19826..b1e3a6ff4 100644 --- a/server/package.json +++ b/server/package.json @@ -50,7 +50,6 @@ "json5": "^2.2.3", "jsonwebtoken": "^9.0.2", "nestjs-zod": "^5.5.0", - "node-cron": "^4.2.1", "nodemailer": "^9.0.1", "otplib": "^12.0.1", "pdf-parse": "^2.4.5", @@ -93,7 +92,6 @@ "@types/jsonwebtoken": "^9.0.10", "@types/multer": "^2.1.0", "@types/node": "^25.5.0", - "@types/node-cron": "^3.0.11", "@types/nodemailer": "^8.0.1", "@types/qrcode": "^1.5.5", "@types/semver": "^7.7.1", diff --git a/server/src/nest/plugins/host/plugin-jobs.ts b/server/src/nest/plugins/host/plugin-jobs.ts index 942408fe8..ec5ed839a 100644 --- a/server/src/nest/plugins/host/plugin-jobs.ts +++ b/server/src/nest/plugins/host/plugin-jobs.ts @@ -1,11 +1,11 @@ -import cron, { type ScheduledTask } from 'node-cron'; +import { CronJob, validateCronExpression } from 'cron'; import { JOBS_PERMISSION } from '../protocol/envelope'; /** * Host-side scheduler for a plugin's declared background jobs (#plugins). * * A plugin declares `jobs` (id + cron `schedule`) in its code; the child reports - * them at load. This wires each to node-cron in the HOST and fires + * them at load. This wires each to a `cron` CronJob in the HOST and fires * `invoke(id, 'invoke.job', { jobId })` on the tick. Kept deliberately small and * pure so it is unit-testable in isolation of the supervisor. * @@ -16,6 +16,10 @@ import { JOBS_PERMISSION } from '../protocol/envelope'; * egress. (3) An invalid cron is skipped (never runs). (4) The tick callback and * `stop()` are wrapped so a throwing job or a stray task can never break the host * or leak across a deactivation. + * + * The library is `cron` (the one @nestjs/schedule wraps), migrated from + * node-cron when the core scheduler retired; tests/unit/plugins/ + * plugin-jobs.cron-compat.test.ts pins that the accepted grammar did not narrow. */ export interface ScheduledJob { @@ -27,25 +31,29 @@ export interface ScheduledJob { export { JOBS_PERMISSION as JOBS_RUN_PERMISSION } from '../protocol/envelope'; /** - * Schedule every valid-cron job (returns the node-cron tasks to keep for teardown). + * Schedule every valid-cron job (returns the CronJobs to keep for teardown). * Empty when the plugin lacks `jobs:run`, so a plugin without the grant runs nothing. */ export function scheduleJobs( granted: ReadonlySet, jobs: readonly ScheduledJob[], run: (jobId: string) => void, -): ScheduledTask[] { +): CronJob[] { if (!granted.has(JOBS_PERMISSION)) return []; - const tasks: ScheduledTask[] = []; + const tasks: CronJob[] = []; for (const job of jobs) { - if (!job.schedule || !cron.validate(job.schedule)) continue; // invalid cron -> never runs + if (!job.schedule || !validateCronExpression(job.schedule).valid) continue; // invalid cron -> never runs tasks.push( - cron.schedule(job.schedule, () => { - try { - run(job.id); - } catch { - /* a job must never break the host loop */ - } + CronJob.from({ + cronTime: job.schedule, + start: true, + onTick: () => { + try { + run(job.id); + } catch { + /* a job must never break the host loop */ + } + }, }), ); } @@ -53,7 +61,7 @@ export function scheduleJobs( } /** Stop + drop the given tasks (called on deactivation/kill so nothing leaks). */ -export function stopJobs(tasks: ScheduledTask[] | undefined): void { +export function stopJobs(tasks: CronJob[] | undefined): void { for (const t of tasks ?? []) { try { t.stop(); diff --git a/server/src/nest/plugins/supervisor/plugin-supervisor.ts b/server/src/nest/plugins/supervisor/plugin-supervisor.ts index 27c142497..5577b7fb1 100644 --- a/server/src/nest/plugins/supervisor/plugin-supervisor.ts +++ b/server/src/nest/plugins/supervisor/plugin-supervisor.ts @@ -53,7 +53,7 @@ interface Supervised { lastRss: number; // last reported resident set size (bytes) routes: PluginRouteInfo[]; jobs: ScheduledJob[]; // declared background jobs (id + cron schedule) - jobTasks?: ReturnType; // live node-cron tasks (only when jobs:run granted) + jobTasks?: ReturnType; // live cron jobs (only when jobs:run granted) hooks: string[]; // provider hooks the plugin implements (e.g. 'placeDetailProvider') events: string[]; // core events the plugin subscribes to (names or '*') exports: string[]; // functions the plugin exposes to dependents (ctx.plugins.call) @@ -638,7 +638,7 @@ export class PluginSupervisor { sup.child = null; // In-flight host→child invokes can never complete now. this.rejectPending(sup, 'plugin exited'); - // The dead child's node-cron tasks keep ticking (node-cron holds them, not the + // The dead child's cron jobs keep ticking (the host holds them, not the // child). Stop them here — otherwise every crash-restart cycle leaks a task-set // AND re-schedules a fresh one, so the job fires N+1 times per tick after N // crashes (duplicate egress/db side effects). kill() already does this for the diff --git a/server/tests/unit/auto-backup.test.ts b/server/tests/unit/auto-backup.test.ts index 95116655f..89b1e1bb2 100644 --- a/server/tests/unit/auto-backup.test.ts +++ b/server/tests/unit/auto-backup.test.ts @@ -229,6 +229,56 @@ describe('auto-backup scheduling (AutoBackupJob.start)', () => { expect(registrar.unregister).not.toHaveBeenCalled(); expect(logMock.logInfo).not.toHaveBeenCalled(); }); + + it('onApplicationBootstrap arms the cron when the registrar is enabled', () => { + fsMock.existsSync.mockImplementation((p: PathLike) => String(p).endsWith('backup-settings.json')); + fsMock.readFileSync.mockReturnValue(JSON.stringify({ enabled: true, interval: 'hourly' })); + const { job, registered } = makeJob(); + job.onApplicationBootstrap(); + expect(registered.at(-1)?.name).toBe('auto-backup'); + expect(registered.at(-1)?.expr).toBe('0 * * * *'); + }); + + it('keep_days 0 banners "retention: forever" and skips retention after a run', async () => { + fsMock.existsSync.mockImplementation((p: PathLike) => String(p).endsWith('backup-settings.json')); + fsMock.readFileSync.mockReturnValue(JSON.stringify({ enabled: true, interval: 'daily', keep_days: 0 })); + stubArchiver(); + const { job, registered } = makeJob(); + job.start(); + expect(logMock.logInfo).toHaveBeenCalledWith(expect.stringContaining('retention: forever')); + + await (registered.at(-1)?.onTick as () => Promise)(); + expect(logMock.logInfo).toHaveBeenCalledWith(expect.stringContaining('Auto-Backup created')); + expect(fsMock.readdirSync).not.toHaveBeenCalled(); // cleanupOldBackups never ran + }); + + it('a non-Error rejection from createBackup is stringified into the failure log', async () => { + const broken = { createBackup: vi.fn().mockRejectedValue('plain string') } as unknown as BackupService; + const registrar = { isEnabled: vi.fn(() => true), register: vi.fn(() => true), unregister: vi.fn() }; + const job = new AutoBackupJob(broken, registrar as unknown as CronRegistrarService); + await job.runBackup(); + expect(logMock.logError).toHaveBeenCalledWith('Auto-Backup: plain string'); + }); + + it('getAutoSettings returns the loaded settings with a resolved timezone', () => { + const prevTz = process.env.TZ; + try { + process.env.TZ = 'Europe/Zurich'; + const { job } = makeJob(); + expect(job.getAutoSettings()).toEqual({ + settings: { enabled: false, interval: 'daily', keep_days: 7, hour: 2, day_of_week: 0, day_of_month: 1 }, + timezone: 'Europe/Zurich', + }); + + delete process.env.TZ; + // Falls back to the host zone (or UTC when the host reports none). + expect(typeof job.getAutoSettings().timezone).toBe('string'); + expect(job.getAutoSettings().timezone.length).toBeGreaterThan(0); + } finally { + if (prevTz === undefined) delete process.env.TZ; + else process.env.TZ = prevTz; + } + }); }); // Moved from backup.impl.test.ts when updateAutoSettings moved onto the job diff --git a/server/tests/unit/idempotency-cleanup.test.ts b/server/tests/unit/idempotency-cleanup.test.ts index a3331c58c..2005738ef 100644 --- a/server/tests/unit/idempotency-cleanup.test.ts +++ b/server/tests/unit/idempotency-cleanup.test.ts @@ -76,6 +76,11 @@ describe('IdempotencyCleanupJob', () => { on.job.onApplicationBootstrap(); expect(on.registrar.register).toHaveBeenCalledWith('idempotency-cleanup', '0 3 * * *', expect.any(Function)); + // The registered callback IS the tick — drive it once over an empty table + // (nothing to purge → no log, no throw). + const onTick = on.registrar.register.mock.calls[0][2] as () => void; + expect(() => onTick()).not.toThrow(); + const off = makeJob(false); off.job.onApplicationBootstrap(); expect(off.registrar.register).not.toHaveBeenCalled(); @@ -103,5 +108,10 @@ describe('IdempotencyCleanupJob', () => { const broken = { prepare: () => { throw new Error('db gone'); } } as unknown as DatabaseService; const job = new IdempotencyCleanupJob(broken, { isEnabled: () => true } as unknown as CronRegistrarService); expect(() => job.tick()).not.toThrow(); + + // Non-Error throws are stringified rather than crashing the catch itself. + const brokenString = { prepare: () => { throw 'db string'; } } as unknown as DatabaseService; + const job2 = new IdempotencyCleanupJob(brokenString, { isEnabled: () => true } as unknown as CronRegistrarService); + expect(() => job2.tick()).not.toThrow(); }); }); diff --git a/server/tests/unit/nest/auto-backup-settings.test.ts b/server/tests/unit/nest/auto-backup-settings.test.ts index c3ace2465..bbf18ad82 100644 --- a/server/tests/unit/nest/auto-backup-settings.test.ts +++ b/server/tests/unit/nest/auto-backup-settings.test.ts @@ -263,4 +263,9 @@ describe('cleanupOldBackups', () => { readdirSyncMock.mockImplementation(() => { throw new Error('ENOENT'); }); expect(() => cleanupOldBackups(7, NOW)).not.toThrow(); }); + + it('swallows non-Error throws without throwing (string rejection path)', () => { + readdirSyncMock.mockImplementation(() => { throw 'nope'; }); + expect(() => cleanupOldBackups(7, NOW)).not.toThrow(); + }); }); diff --git a/server/tests/unit/plugins/plugin-jobs.cron-compat.test.ts b/server/tests/unit/plugins/plugin-jobs.cron-compat.test.ts new file mode 100644 index 000000000..07928464d --- /dev/null +++ b/server/tests/unit/plugins/plugin-jobs.cron-compat.test.ts @@ -0,0 +1,43 @@ +/** + * Grammar-compat pin for the plugin job scheduler's node-cron → cron migration. + * + * A validation *tightening* would silently disable a published plugin's job + * (an invalid cron is skipped, never scheduled — deliberately, so a bad + * schedule can't take the host down). This runs the REAL `cron` validator + * (unmocked) over the manifest shapes plugins realistically declare and pins + * that everything node-cron accepted still schedules, and garbage still + * doesn't. + */ +import { describe, it, expect } from 'vitest'; +import { validateCronExpression } from 'cron'; + +const ACCEPTED = [ + '0 0 * * *', // nightly + '*/5 * * * *', // step minutes + '0 9 * * MON', // day-of-week name + '0 9 * * sun', // lowercase name + '0 9 * * 7', // 7-as-Sunday + '0 0 1 JAN *', // month name + '30 */2 * * 0-5', // range + step combined + '1-5 * * * *', // minute range + '*/30 * * * * *', // 6-field with seconds first + '0 0 29 2 *', // leap-day +]; + +const REJECTED = [ + 'not-a-cron', + '', + '* * *', // too few fields + '61 * * * *', // minute out of range + '0 24 * * *', // hour out of range +]; + +describe('plugin job cron grammar (real validator)', () => { + it.each(ACCEPTED)('PJOB-COMPAT accepts %j', (expr) => { + expect(validateCronExpression(expr).valid).toBe(true); + }); + + it.each(REJECTED)('PJOB-COMPAT rejects %j', (expr) => { + expect(validateCronExpression(expr).valid).toBe(false); + }); +}); diff --git a/server/tests/unit/plugins/plugin-jobs.test.ts b/server/tests/unit/plugins/plugin-jobs.test.ts index 7e4642d7c..d5f473a37 100644 --- a/server/tests/unit/plugins/plugin-jobs.test.ts +++ b/server/tests/unit/plugins/plugin-jobs.test.ts @@ -6,11 +6,11 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; const h = vi.hoisted(() => ({ tasks: [] as Array<{ expr: string; fn: () => void; stopped: boolean; stop(): void }> })); -vi.mock('node-cron', () => ({ - default: { - validate: (expr: string) => typeof expr === 'string' && expr.trim() !== '' && expr !== 'not-a-cron', - schedule: (expr: string, fn: () => void) => { - const t = { expr, fn, stopped: false, stop() { this.stopped = true; } }; +vi.mock('cron', () => ({ + validateCronExpression: (expr: string) => ({ valid: typeof expr === 'string' && expr.trim() !== '' && expr !== 'not-a-cron' }), + CronJob: { + from: ({ cronTime, onTick }: { cronTime: string; onTick: () => void }) => { + const t = { expr: cronTime, fn: onTick, stopped: false, stop() { this.stopped = true; } }; h.tasks.push(t); return t; },