[BUG] Scan-run cleanup caps history at 20 runs instead of keeping 20 as a minimum past RetainScanRunsDays #290

Summary

The cleanup pass deletes every unreferenced completed scan run that is either older than RetainScanRunsDays or not among the 20 most recently completed ones. That makes 20 a hard cap: with the default 30-day retention, a deployment that records dozens of runs a day keeps only the last 20 unreferenced runs, not 30 days of history as the README promises. The product decision is the opposite: the 20 most recent completed runs are a minimum that survives even when they are older than the retention period.

Area

• docs/areas/scan-engine.md (added in [Docs] Describe the current behavior by area #263) — Retention and cleanup, Open question 2
• docs/areas/web-ui.md (scan history shows the 20 most recent runs; informational)
• Security area: None

Current behavior

src/DockerUpdateGuard/Images/ScanCleanupBackgroundService.cs:

• :21 private const int MaxRetainedScanRuns = 20; — documented as "Maximum number of scan runs to retain".
• :186 var cutoff = cleanupStartedAtUtc.AddDays(-RetainScanRunsDays);
• :190-196 completedUnreferencedScanRuns: completed, CompletedAtUtc <= cleanupStartedAtUtc, and no update finding, vulnerability finding, container snapshot or image relationship refers to them.
• :198-202 retainedScanRunIds = the 20 most recently completed of those.
• :238-244 and :260-262 — the union of two sets is removed:

var oldScanRuns    = await completedUnreferencedScanRuns.Where(entity => entity.CompletedAtUtc < cutoff)...;
var excessScanRuns = await completedUnreferencedScanRuns.Where(entity => retainedScanRunIds.Contains(entity.Id) == false)...;
var removableScanRuns = oldScanRuns.Concat(excessScanRuns).DistinctBy(entity => entity.Id).ToList();

So a run survives only if it is both among the newest 20 and not older than the cutoff.

The existing test asserts the cap: src/Tests/DockerUpdateGuard.Tests/ScanCleanupBackgroundServiceTests.cs:28 ScanCleanupBackgroundServiceExecuteCoreAsyncRemovesUnreferencedScanRunsBeyondLatestTwentyAsync seeds 25 runs from the last 25 minutes with RetainScanRunsDays = 30 and expects only 20 to remain.

Scenario A (cap): default settings, 10 enabled observed images (60-min interval → ~240 ObservedImage runs/day). After a cleanup pass only 20 unreferenced runs remain, i.e. a few hours of history instead of 30 days. README :153 describes RetainScanRunsDays as "Retention period for completed scan history".

Scenario B (no minimum): an instance that was switched off for longer than RetainScanRunsDays and is started again: the first cleanup pass removes all old unreferenced runs, leaving an empty scan history although the last runs are the only evidence of what happened.

Expected behavior

Decision (product manager): the 20 most recent completed runs are kept as a minimum, even past RetainScanRunsDays — not as a cap.

• A completed scan run is removed by cleanup only when all of these hold:
  • it completed before the cutoff (now − RetainScanRunsDays);
  • it is not among the 20 most recently completed scan runs (ordered by CompletedAtUtc, counted over all completed runs);
  • no update finding, vulnerability finding, container snapshot or image relationship refers to it (unchanged).
• Completed runs within the retention period are always kept, regardless of their number.
• Running runs are never removed (unchanged); stale-run repair is unchanged.
• The cleanup log line still reports the number of removed scan runs.

Acceptance criteria

• 25 unreferenced runs all within retention → all 25 kept. Replace ScanCleanupBackgroundServiceExecuteCoreAsyncRemovesUnreferencedScanRunsBeyondLatestTwentyAsync with e.g. ScanCleanupBackgroundServiceExecuteCoreAsyncKeepsAllScanRunsWithinRetentionAsync.
• 25 unreferenced runs all older than the cutoff → exactly the 20 most recently completed are kept: ScanCleanupBackgroundServiceExecuteCoreAsyncKeepsLatestTwentyScanRunsPastRetentionAsync.
• Mixed: 5 runs within retention + 30 older → the 5 recent plus the 15 newest old runs are kept (20 in total), the other 15 removed: ScanCleanupBackgroundServiceExecuteCoreAsyncKeepsMinimumOfTwentyScanRunsAsync.
• Referenced runs older than the cutoff are still kept (existing behavior covered by a test).
• Running runs are still never removed (existing test stays green).
• Coverage gate passes; assertions have messages.
• README RetainScanRunsDays description states the minimum ("…; the 20 most recent completed runs are always kept").
• The area document docs/areas/scan-engine.md (Retention and cleanup) describes the new rule and its Known gaps entry for this issue is removed.

Implementation notes

• Change in ScanCleanupBackgroundService.ExecuteCoreAsync only: compute the IDs of the 20 most recently completed runs (CompletedAtUtc != null, ordered descending) and remove completedUnreferencedScanRuns.Where(CompletedAtUtc < cutoff && retainedIds.Contains(Id) == false); drop excessScanRuns. Rename the constant to MinimumRetainedScanRuns and fix its XML summary.
• Counting the minimum over all completed runs (not only unreferenced ones) guarantees that the 20 most recent completed runs exist after cleanup, because referenced runs are kept anyway.
• The pass uses DateTimeOffset.UtcNow (:180). The tests can keep relative timestamps; if the Tester needs a deterministic clock, inject TimeProvider (also used by the stale-run repair).
• Must not change: what references keep a run, the single-transaction removal, the other retention rules (tag candidates, findings, snapshots, resource samples), stale-run repair.
• No configuration key changes, no migration.

Related

• PR [Docs] Describe the current behavior by area #263 (area documents)
• F-027 (P3) — ScanCleanupBackgroundService loads all rows to be deleted into memory before RemoveRange #81 (cleanup loads all rows) — same method; avoid making the loading worse (prefer a server-side Where over materializing all runs).

Generated by Claude Code