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) + }) })