From 81dd0ce74207323ac003ce6d03b20ff64b422448 Mon Sep 17 00:00:00 2001 From: debont80 Date: Wed, 1 Jul 2026 09:19:00 -0400 Subject: [PATCH] =?UTF-8?q?fix(main):=20R9=20=E2=80=94=20guard=20the=20yt-?= =?UTF-8?q?dlp=20update=20throttle=20against=20a=20backward=20clock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shouldAutoCheckYtdlp compared (now - lastCheck) >= interval; a backward system- clock correction leaves lastCheck in the future so the difference is negative and the daily check never comes due again. Treat a future lastCheck as skewed and run the check. The renderer scheduled-item promoter is inherently fire-once (promotion flips the item out of 'saved'), so it can't double-fire on a backward jump — a forward jump firing early is documented and accepted as minor. Unit-tested. typecheck + 263 tests + eslint + prettier green. Co-Authored-By: Claude Opus 4.8 --- CODE-AUDIT.md | 7 ++++++- src/main/ytdlpPolicy.ts | 5 +++++ src/renderer/src/store/downloads.ts | 4 ++++ test/ytdlpPolicy.test.ts | 7 +++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md index 6b88129..46c5555 100644 --- a/CODE-AUDIT.md +++ b/CODE-AUDIT.md @@ -1595,8 +1595,13 @@ cosmetics. `R` IDs. should never fire in practice. (refusing/at-rest enforcement remains an option if ever wanted.)* - [x] **R8 — Pretty-printed JSON for large stores.** `JSON.stringify(value, null, 2)` inflates size/write time for the potentially-20k-item media store (ties R3). **Fix:** compact JSON for the large stores. -- [ ] **R9 — Clock-skew sensitivity.** `shouldAutoCheckYtdlp` and the scheduled-download promoter compare +- [x] **R9 — Clock-skew sensitivity.** `shouldAutoCheckYtdlp` and the scheduled-download promoter compare `Date.now()`; a backward system-clock correction can skip or double-fire a check/schedule. Minor; note for awareness. + *Fixed the concrete case: `shouldAutoCheckYtdlp` now treats a `lastCheck` in the future (clock ran + backward → negative interval) as due, so the daily update check can't be skipped forever (unit-tested). + The scheduled-download promoter is inherently fire-once — each promotion flips the item out of `saved`, + so a backward jump only delays it and can't double-fire; a forward jump firing early is documented and + accepted as minor (a monotonic scheduler would be overkill for a park-until-time feature).* --- diff --git a/src/main/ytdlpPolicy.ts b/src/main/ytdlpPolicy.ts index a2d54f5..b178dbc 100644 --- a/src/main/ytdlpPolicy.ts +++ b/src/main/ytdlpPolicy.ts @@ -21,5 +21,10 @@ export function shouldAutoCheckYtdlp( ): boolean { if (!enabled) return false if (!lastCheck) return true + // A backward system-clock correction can leave `lastCheck` in the "future" + // relative to `now`, making `now - lastCheck` negative so the daily check would + // never come due again. Treat a future timestamp as skewed/bogus and run the + // check rather than waiting out a negative interval (R9). + if (lastCheck > now) return true return now - lastCheck >= intervalMs } diff --git a/src/renderer/src/store/downloads.ts b/src/renderer/src/store/downloads.ts index decd534..cb81ea6 100644 --- a/src/renderer/src/store/downloads.ts +++ b/src/renderer/src/store/downloads.ts @@ -272,6 +272,10 @@ export const useDownloads = create((set, get) => { }, promoteDueScheduled: () => { + // Wall-clock comparison: each scheduled item fires once (the promotion flips it + // out of 'saved'), so a backward clock jump only delays it and can't double-fire; + // a forward jump can fire it early. Accepted as minor per R9 — a monotonic + // scheduler would be overkill for a park-until-time feature. const now = Date.now() set((s) => ({ items: s.items.map((i) => diff --git a/test/ytdlpPolicy.test.ts b/test/ytdlpPolicy.test.ts index 1f5101e..149b181 100644 --- a/test/ytdlpPolicy.test.ts +++ b/test/ytdlpPolicy.test.ts @@ -31,4 +31,11 @@ describe('shouldAutoCheckYtdlp', () => { expect(shouldAutoCheckYtdlp(true, NOW - 30 * 60_000, NOW, hour)).toBe(false) expect(shouldAutoCheckYtdlp(true, NOW - hour, NOW, hour)).toBe(true) }) + + it('checks when the clock ran backward (lastCheck is in the future, R9)', () => { + // A backward clock correction leaves lastCheck > now → (now - lastCheck) is + // negative; without the guard the check would be skipped forever. + expect(shouldAutoCheckYtdlp(true, NOW + AUTO_UPDATE_INTERVAL_MS, NOW)).toBe(true) + expect(shouldAutoCheckYtdlp(true, NOW + 1, NOW)).toBe(true) + }) })