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.
This commit is contained in:
@@ -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<AppSettings>): Partial<AppSettings> {
|
||||
const entries = Object.entries(partial || {}).filter(([, value]) => value !== undefined);
|
||||
@@ -521,7 +521,7 @@ export class AppController {
|
||||
}
|
||||
|
||||
public updateSettings(partial: Partial<AppSettings>): AppSettings {
|
||||
const sanitizedPatch = prepareDailyStartSettingsPatch(sanitizeSettingsPatch(partial)) as Partial<AppSettings>;
|
||||
const sanitizedPatch = sanitizeSettingsPatch(partial);
|
||||
const previousSettings = this.settings;
|
||||
let nextSettings = normalizeSettings({
|
||||
...previousSettings,
|
||||
|
||||
@@ -5,7 +5,8 @@ interface DailyStartSnapshot {
|
||||
session: {
|
||||
running: boolean;
|
||||
paused: boolean;
|
||||
items: Record<string, { status: string }>;
|
||||
items: Record<string, { status: string; packageId: string }>;
|
||||
packages: Record<string, { enabled: boolean; cancelled: boolean }>;
|
||||
};
|
||||
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<DailyStartOutcome | null> | null = null;
|
||||
private reconcileTimer: ReturnType<typeof setInterval> | 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<DailyStartOutcome | null> {
|
||||
private async reconcileOnce(generation: number): Promise<DailyStartOutcome | null> {
|
||||
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");
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -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<AppSettings>);
|
||||
const result = controller.updateSettings(prepareDailyStartSettingsPatch(validated) as Partial<AppSettings>);
|
||||
updateClipboardWatcher();
|
||||
updateTray();
|
||||
armScheduledStart(result.scheduledStartEpochMs || 0, { startOnPast: true });
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, { status: string }> = { queued: { status: "queued" } };
|
||||
public items: Record<string, { status: string; packageId: string }> = { queued: { status: "queued", packageId: "package" } };
|
||||
public packages: Record<string, { enabled: boolean; cancelled: boolean }> = {
|
||||
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<void>((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,
|
||||
|
||||
Reference in New Issue
Block a user