From 0ca00149eb6006cb1522930295aa39da84fab599 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe Date: Sat, 22 Aug 2026 09:50:37 +0200 Subject: [PATCH] fix(scheduler): harden daily start reconciliation Preserve legacy one-time schedules across internal account state saves and clear them only for explicit renderer daily-rule updates. Enforce monotone handled and pending calendar receipts across backward local-date changes, invalidate late in-flight start results on shutdown, and align queue eligibility with enabled non-cancelled packages. Replace the in-memory restart assertion with real settings persistence and add RED/GREEN regression coverage for every reviewed behavior. --- src/main/app-controller.ts | 4 +- src/main/daily-start-scheduler.ts | 46 +++++++++-- src/main/main.ts | 4 +- tests/app-controller.test.ts | 28 ++++++- tests/daily-start-scheduler.test.ts | 124 +++++++++++++++++++++++++--- 5 files changed, 182 insertions(+), 24 deletions(-) diff --git a/src/main/app-controller.ts b/src/main/app-controller.ts index 54f774b..342fc95 100644 --- a/src/main/app-controller.ts +++ b/src/main/app-controller.ts @@ -81,7 +81,7 @@ import { normalizeStatisticsLedger, saveStatisticsLedger } from "./statistics-le import { NotificationOutbox } from "./notification-outbox"; import { sendNotification } from "./notify"; import { DownloadHealthMonitor } from "./download-health-monitor"; -import { prepareDailyStartSettingsPatch, shouldDeferAutoResumeToDailyStart } from "./daily-start-scheduler"; +import { shouldDeferAutoResumeToDailyStart } from "./daily-start-scheduler"; function sanitizeSettingsPatch(partial: Partial): Partial { const entries = Object.entries(partial || {}).filter(([, value]) => value !== undefined); @@ -521,7 +521,7 @@ export class AppController { } public updateSettings(partial: Partial): AppSettings { - const sanitizedPatch = prepareDailyStartSettingsPatch(sanitizeSettingsPatch(partial)) as Partial; + const sanitizedPatch = sanitizeSettingsPatch(partial); const previousSettings = this.settings; let nextSettings = normalizeSettings({ ...previousSettings, diff --git a/src/main/daily-start-scheduler.ts b/src/main/daily-start-scheduler.ts index 3597889..0886314 100644 --- a/src/main/daily-start-scheduler.ts +++ b/src/main/daily-start-scheduler.ts @@ -5,7 +5,8 @@ interface DailyStartSnapshot { session: { running: boolean; paused: boolean; - items: Record; + items: Record; + packages: Record; }; canStart: boolean; } @@ -73,6 +74,12 @@ export function nextDailyStartEpochMs(settings: DailyStartSettings, nowEpochMs = const now = new Date(nowEpochMs); const today = formatLocalDate(now); let candidate = settings.dailyStartFirstLocalDate > today ? settings.dailyStartFirstLocalDate : today; + const pending = isValidLocalDate(settings.dailyStartPendingLocalDate) + ? settings.dailyStartPendingLocalDate + : ""; + if (pending && candidate < pending) { + candidate = pending; + } const handled = isValidLocalDate(settings.dailyStartLastHandledLocalDate) ? settings.dailyStartLastHandledLocalDate : ""; @@ -105,6 +112,7 @@ export function shouldDeferAutoResumeToDailyStart( export class DailyStartScheduler { private reconcileInFlight: Promise | null = null; private reconcileTimer: ReturnType | null = null; + private lifecycleGeneration = 0; public constructor( private readonly controller: DailyStartController, @@ -115,7 +123,7 @@ export class DailyStartScheduler { if (this.reconcileInFlight) { return this.reconcileInFlight; } - const operation = this.reconcileOnce(); + const operation = this.reconcileOnce(this.lifecycleGeneration); this.reconcileInFlight = operation; void operation.finally(() => { if (this.reconcileInFlight === operation) { @@ -135,6 +143,7 @@ export class DailyStartScheduler { } public end(): void { + this.lifecycleGeneration += 1; if (this.reconcileTimer !== null) { clearInterval(this.reconcileTimer); this.reconcileTimer = null; @@ -150,7 +159,7 @@ export class DailyStartScheduler { return outcome; } - private async reconcileOnce(): Promise { + private async reconcileOnce(generation: number): Promise { const nowEpochMs = this.now(); let settings = this.controller.getSnapshot().settings; if (!settings.dailyStartEnabled || !isValidLocalDate(settings.dailyStartFirstLocalDate)) { @@ -158,17 +167,27 @@ export class DailyStartScheduler { } const today = formatLocalDate(new Date(nowEpochMs)); + const handledDate = isValidLocalDate(settings.dailyStartLastHandledLocalDate) + ? settings.dailyStartLastHandledLocalDate + : ""; + const pendingDate = isValidLocalDate(settings.dailyStartPendingLocalDate) + ? settings.dailyStartPendingLocalDate + : ""; + if ((handledDate && today <= handledDate) || (pendingDate && today < pendingDate)) { + return null; + } let missed: DailyStartOutcome | null = null; if (isValidLocalDate(settings.dailyStartPendingLocalDate) && settings.dailyStartPendingLocalDate < today) { - const pendingDate = settings.dailyStartPendingLocalDate; + const missedDate = settings.dailyStartPendingLocalDate; + const lastHandledLocalDate = handledDate > missedDate ? handledDate : missedDate; this.controller.updateSettings({ - dailyStartLastHandledLocalDate: pendingDate, + dailyStartLastHandledLocalDate: lastHandledLocalDate, dailyStartPendingLocalDate: "", dailyStartLastOutcome: "missed" }); settings = { ...settings, - dailyStartLastHandledLocalDate: pendingDate, + dailyStartLastHandledLocalDate: lastHandledLocalDate, dailyStartPendingLocalDate: "", dailyStartLastOutcome: "missed" }; @@ -194,8 +213,13 @@ export class DailyStartScheduler { if (snapshot.session.running || snapshot.session.paused) { return this.finish(today, "already_active"); } - const hasQueuedItems = Object.values(snapshot.session.items) - .some((item) => item.status === "queued" || item.status === "reconnect_wait"); + const hasQueuedItems = Object.values(snapshot.session.items).some((item) => { + if (item.status !== "queued" && item.status !== "reconnect_wait") { + return false; + } + const pkg = snapshot.session.packages[item.packageId]; + return Boolean(pkg && pkg.enabled && !pkg.cancelled); + }); if (!hasQueuedItems) { return this.finish(today, "empty_queue"); } @@ -207,9 +231,15 @@ export class DailyStartScheduler { try { await this.controller.start(); } catch (error) { + if (generation !== this.lifecycleGeneration) { + return null; + } this.controller.updateSettings({ dailyStartLastOutcome: "start_failed" }); throw error; } + if (generation !== this.lifecycleGeneration) { + return null; + } return this.finish(today, "started"); } } diff --git a/src/main/main.ts b/src/main/main.ts index 4c2edc3..bbb16f0 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -24,7 +24,7 @@ import { assertTrustedIpcSender, type TrustedIpcOptions } from "./ipc-security"; import { validateRealDebridLoginRequest } from "../shared/preload-api"; import { migrateProductUserDataDirectory } from "./storage"; import { validateCollectorContainerInspectionRequest, validateCollectorInspectionRequest } from "../shared/collector"; -import { DailyStartScheduler, hasDailyStartRulePatch } from "./daily-start-scheduler"; +import { DailyStartScheduler, hasDailyStartRulePatch, prepareDailyStartSettingsPatch } from "./daily-start-scheduler"; function validateString(value: unknown, name: string): string { if (typeof value !== "string") { @@ -464,7 +464,7 @@ function registerIpcHandlers(): void { }); handleTrusted(IPC_CHANNELS.UPDATE_SETTINGS, async (_event: IpcMainInvokeEvent, partial: RendererSettingsUpdate) => { const validated = validateRendererSettingsUpdate(partial ?? {}, controller.getSettings()); - const result = controller.updateSettings(validated as Partial); + const result = controller.updateSettings(prepareDailyStartSettingsPatch(validated) as Partial); updateClipboardWatcher(); updateTray(); armScheduledStart(result.scheduledStartEpochMs || 0, { startOnPast: true }); diff --git a/tests/app-controller.test.ts b/tests/app-controller.test.ts index 140cb57..ec1fa5f 100644 --- a/tests/app-controller.test.ts +++ b/tests/app-controller.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { AppController } from "../src/main/app-controller"; import { defaultSettings } from "../src/main/constants"; import { configureCredentialProtector } from "../src/main/credential-protection"; +import { prepareDailyStartSettingsPatch } from "../src/main/daily-start-scheduler"; import { createStoragePaths } from "../src/main/storage"; import type { AppSettings } from "../src/shared/types"; @@ -55,11 +56,11 @@ describe("AppController daily start settings", () => { scheduledStartEpochMs: 1_800_000_000_000 }); - const updated = controller.updateSettings({ + const updated = controller.updateSettings(prepareDailyStartSettingsPatch({ dailyStartEnabled: true, dailyStartMinuteOfDay: 18 * 60 + 45, dailyStartFirstLocalDate: "2026-08-23" - }); + })); expect(updated.scheduledStartEpochMs).toBe(0); expect(updated).toMatchObject({ @@ -68,4 +69,27 @@ describe("AppController daily start settings", () => { dailyStartFirstLocalDate: "2026-08-23" }); }); + + it("preserves a legacy one-time schedule when an account mutation saves a complete settings state", async () => { + configureCredentialProtector({ + isEncryptionAvailable: () => false, + encryptString: (value) => Buffer.from(value, "utf8"), + decryptString: (value) => Buffer.from(value).toString("utf8") + }); + const scheduledStartEpochMs = 1_800_000_000_000; + const controller = createController({ + ...defaultSettings(), + ddownloadLogin: "account@example.test", + ddownloadPassword: "secret", + scheduledStartEpochMs + }); + + await controller.executeAccountCommand({ + action: "delete", + kind: "ddownload-login", + accountId: "svc-ddownload" + }); + + expect(controller.getSettings().scheduledStartEpochMs).toBe(scheduledStartEpochMs); + }); }); diff --git a/tests/daily-start-scheduler.test.ts b/tests/daily-start-scheduler.test.ts index eab39d9..77b60ad 100644 --- a/tests/daily-start-scheduler.test.ts +++ b/tests/daily-start-scheduler.test.ts @@ -1,6 +1,12 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { afterAll, describe, expect, it, vi } from "vitest"; -import type { DailyStartSettings } from "../src/shared/types"; +import type { AppSettings, DailyStartSettings } from "../src/shared/types"; import { DailyStartScheduler, nextDailyStartEpochMs, prepareDailyStartSettingsPatch, shouldDeferAutoResumeToDailyStart } from "../src/main/daily-start-scheduler"; +import { defaultSettings } from "../src/main/constants"; +import { configureCredentialProtector } from "../src/main/credential-protection"; +import { createStoragePaths, loadSettings, saveSettings } from "../src/main/storage"; const originalTimezone = process.env.TZ; process.env.TZ = "Europe/Berlin"; @@ -30,13 +36,19 @@ class FakeDailyStartController { public running = false; public paused = false; public canStart = true; - public items: Record = { queued: { status: "queued" } }; + public items: Record = { queued: { status: "queued", packageId: "package" } }; + public packages: Record = { + package: { enabled: true, cancelled: false } + }; public events: string[] = []; public start = vi.fn(async () => { this.events.push("start"); }); - public constructor(value: DailyStartSettings) { + public constructor( + value: DailyStartSettings, + private readonly persist?: (value: DailyStartSettings) => void + ) { this.settings = value; } @@ -46,7 +58,8 @@ class FakeDailyStartController { session: { running: this.running, paused: this.paused, - items: this.items + items: this.items, + packages: this.packages }, canStart: this.canStart }; @@ -60,6 +73,7 @@ class FakeDailyStartController { if (partial.dailyStartLastHandledLocalDate) { this.events.push(`handled:${partial.dailyStartLastHandledLocalDate}`); } + this.persist?.(this.settings); return this.settings; } } @@ -125,6 +139,39 @@ describe("daily start scheduler", () => { expect(controller.start).toHaveBeenCalledTimes(1); }); + it("does not redispatch when the local calendar moves behind a handled or pending day", async () => { + const now = new Date(2026, 7, 22, 11, 0).getTime(); + const handledController = new FakeDailyStartController(settings({ + dailyStartLastHandledLocalDate: "2026-08-23" + })); + const pendingController = new FakeDailyStartController(settings({ + dailyStartPendingLocalDate: "2026-08-23" + })); + + expect(await new DailyStartScheduler(handledController, () => now).reconcile()).toBeNull(); + expect(await new DailyStartScheduler(pendingController, () => now).reconcile()).toBeNull(); + + expect(handledController.start).not.toHaveBeenCalled(); + expect(pendingController.start).not.toHaveBeenCalled(); + expect(handledController.settings.dailyStartLastHandledLocalDate).toBe("2026-08-23"); + expect(pendingController.settings.dailyStartPendingLocalDate).toBe("2026-08-23"); + expect(nextDailyStartEpochMs(handledController.settings, now)).toBe(new Date(2026, 7, 24, 10, 0).getTime()); + expect(nextDailyStartEpochMs(pendingController.settings, now)).toBe(new Date(2026, 7, 23, 10, 0).getTime()); + }); + + it("does not regress last handled when an older pending receipt expires", async () => { + const now = new Date(2026, 7, 23, 9, 0).getTime(); + const controller = new FakeDailyStartController(settings({ + dailyStartLastHandledLocalDate: "2026-08-22", + dailyStartPendingLocalDate: "2026-08-21" + })); + + expect(await new DailyStartScheduler(controller, () => now).reconcile()).toBe("missed"); + expect(controller.start).not.toHaveBeenCalled(); + expect(controller.settings.dailyStartLastHandledLocalDate).toBe("2026-08-22"); + expect(controller.settings.dailyStartPendingLocalDate).toBe(""); + }); + it("treats repeated suspend and resume reconciles as one daily dispatch", async () => { const now = new Date(2026, 7, 22, 10, 5).getTime(); const controller = new FakeDailyStartController(settings()); @@ -139,14 +186,38 @@ describe("daily start scheduler", () => { it("recovers a persisted pending receipt after restart and then deduplicates it", async () => { const now = new Date(2026, 7, 22, 10, 5).getTime(); - const controller = new FakeDailyStartController(settings({ dailyStartPendingLocalDate: "2026-08-22" })); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "mdd-daily-restart-")); + const paths = createStoragePaths(dir); + configureCredentialProtector({ + isEncryptionAvailable: () => false, + encryptString: (value) => Buffer.from(value, "utf8"), + decryptString: (value) => Buffer.from(value).toString("utf8") + }); + saveSettings(paths, { + ...defaultSettings(), + ...settings({ dailyStartPendingLocalDate: "2026-08-22" }) + }); - await new DailyStartScheduler(controller, () => now).reconcile(); - await new DailyStartScheduler(controller, () => now).reconcile(); + try { + const restartedController = new FakeDailyStartController( + loadSettings(paths), + (value) => saveSettings(paths, value as AppSettings) + ); + await new DailyStartScheduler(restartedController, () => now).reconcile(); - expect(controller.start).toHaveBeenCalledTimes(1); - expect(controller.settings.dailyStartLastHandledLocalDate).toBe("2026-08-22"); - expect(controller.settings.dailyStartPendingLocalDate).toBe(""); + expect(restartedController.start).toHaveBeenCalledTimes(1); + expect(restartedController.settings.dailyStartLastHandledLocalDate).toBe("2026-08-22"); + expect(restartedController.settings.dailyStartPendingLocalDate).toBe(""); + + const secondRestartController = new FakeDailyStartController(loadSettings(paths)); + await new DailyStartScheduler(secondRestartController, () => now).reconcile(); + + expect(secondRestartController.start).not.toHaveBeenCalled(); + expect(secondRestartController.settings.dailyStartLastHandledLocalDate).toBe("2026-08-22"); + expect(secondRestartController.settings.dailyStartPendingLocalDate).toBe(""); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it("marks an empty queue handled without calling start", async () => { @@ -160,6 +231,17 @@ describe("daily start scheduler", () => { expect(controller.settings.dailyStartLastOutcome).toBe("empty_queue"); }); + it("treats queued items in disabled packages as an empty startable queue", async () => { + const now = new Date(2026, 7, 22, 10, 5).getTime(); + const controller = new FakeDailyStartController(settings()); + controller.packages.package.enabled = false; + + expect(await new DailyStartScheduler(controller, () => now).reconcile()).toBe("empty_queue"); + expect(controller.start).not.toHaveBeenCalled(); + expect(controller.settings.dailyStartLastHandledLocalDate).toBe("2026-08-22"); + expect(controller.settings.dailyStartLastOutcome).toBe("empty_queue"); + }); + it("keeps a missing-account occurrence pending and retries after account recovery", async () => { const now = new Date(2026, 7, 22, 10, 5).getTime(); const controller = new FakeDailyStartController(settings()); @@ -218,6 +300,28 @@ describe("daily start scheduler", () => { expect(controller.start).toHaveBeenCalledTimes(2); }); + it("does not consume a pending day when shutdown invalidates an in-flight start", async () => { + const now = new Date(2026, 7, 22, 10, 5).getTime(); + const controller = new FakeDailyStartController(settings()); + let resolveStart!: () => void; + controller.start.mockImplementationOnce(() => new Promise((resolve) => { + resolveStart = resolve; + })); + const scheduler = new DailyStartScheduler(controller, () => now); + + const reconcile = scheduler.reconcile(); + expect(controller.start).toHaveBeenCalledTimes(1); + expect(controller.settings.dailyStartPendingLocalDate).toBe("2026-08-22"); + + scheduler.end(); + resolveStart(); + + expect(await reconcile).toBeNull(); + expect(controller.settings.dailyStartPendingLocalDate).toBe("2026-08-22"); + expect(controller.settings.dailyStartLastHandledLocalDate).toBe(""); + expect(controller.settings.dailyStartLastOutcome).toBe(""); + }); + it("constructs each DST target from local calendar fields instead of adding 24 hours", () => { const beforeDst = settings({ dailyStartMinuteOfDay: 2 * 60 + 30,