From c98885ce202b28bb9c7a50c803c55ff28eacba78 Mon Sep 17 00:00:00 2001
From: jon <iam.rans0m00@gmail.com>
Date: Sun, 1 Mar 2026 16:46:16 -0600
Subject: [PATCH] fix: address second code review findings for rollover storage

- C3: Make retain_policy global-only by hiding from per-camera settings
  UI and removing from CameraConfig TypeScript type
- C4: Add missing units field to StorageBreakdown TypeScript type to
  match API response
- I3: Add 24-hour grace period for deleted camera recordings in rollover
  mode to prevent data loss if camera is temporarily removed from config
- I6: Deduplicate model_validator warning by using module-level flag to
  prevent N+1 duplicate warnings during config propagation
- S5: Add debug log when skipping time-based expiry in rollover mode and
  clean up redundant if/not-if to if/else
---
 frigate/config/camera/record.py                  | 11 ++++++++++-
 frigate/record/cleanup.py                        | 16 +++++++++++++---
 .../config-form/section-configs/record.ts        |  1 +
 web/src/types/frigateConfig.ts                   |  2 +-
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/frigate/config/camera/record.py b/frigate/config/camera/record.py
index b1a921120..f38f3f5f8 100644
--- a/frigate/config/camera/record.py
+++ b/frigate/config/camera/record.py
@@ -11,6 +11,10 @@ from ..base import FrigateBaseModel
 
 logger = logging.getLogger(__name__)
 
+# Module-level flag to prevent the rollover warning from firing N+1 times
+# (once per camera + once global) during config propagation.
+_rollover_warning_emitted = False
+
 __all__ = [
     "RecordConfig",
     "RecordExportConfig",
@@ -157,8 +161,13 @@ class RecordConfig(FrigateBaseModel):
 
     @model_validator(mode="after")
     def warn_rollover_with_days(self) -> "RecordConfig":
-        if self.retain_policy == RetainPolicyEnum.continuous_rollover:
+        global _rollover_warning_emitted
+        if (
+            not _rollover_warning_emitted
+            and self.retain_policy == RetainPolicyEnum.continuous_rollover
+        ):
             if self.continuous.days > 0 or self.motion.days > 0:
+                _rollover_warning_emitted = True
                 logger.warning(
                     "retain_policy is 'continuous_rollover' - continuous.days and "
                     "motion.days are ignored. Recordings will fill available disk "
diff --git a/frigate/record/cleanup.py b/frigate/record/cleanup.py
index 0d59f66ae..bcced9bb6 100644
--- a/frigate/record/cleanup.py
+++ b/frigate/record/cleanup.py
@@ -289,7 +289,11 @@ class RecordingCleanup(threading.Thread):
         # Handle deleted cameras
         logger.debug("Start deleted cameras.")
         if is_rollover:
-            # In rollover mode, delete recordings from removed cameras immediately
+            # In rollover mode, keep a 24-hour grace period for removed cameras
+            # so footage isn't lost if a camera is temporarily removed from config
+            rollover_expire_before = (
+                datetime.datetime.now() - datetime.timedelta(hours=24)
+            ).timestamp()
             no_camera_recordings: Recordings = (
                 Recordings.select(
                     Recordings.id,
@@ -297,6 +301,7 @@ class RecordingCleanup(threading.Thread):
                 )
                 .where(
                     Recordings.camera.not_in(list(self.config.cameras.keys())),
+                    Recordings.end_time < rollover_expire_before,
                 )
                 .namedtuples()
                 .iterator()
@@ -347,8 +352,13 @@ class RecordingCleanup(threading.Thread):
             # Always expire review segments (alerts/detections) regardless of policy
             maybe_empty_dirs |= self.expire_review_segments(config, now)
 
-            # Skip continuous/motion time-based expiry in rollover mode
-            if not is_rollover:
+            # Skip continuous/motion time-based expiry in rollover mode;
+            # StorageMaintainer handles space reclamation by deleting oldest recordings.
+            if is_rollover:
+                logger.debug(
+                    f"Skipping time-based expiry for {camera} (continuous_rollover mode)."
+                )
+            else:
                 continuous_expire_date = (
                     now - datetime.timedelta(days=config.record.continuous.days)
                 ).timestamp()
diff --git a/web/src/components/config-form/section-configs/record.ts b/web/src/components/config-form/section-configs/record.ts
index 086e686d9..6542b85fd 100644
--- a/web/src/components/config-form/section-configs/record.ts
+++ b/web/src/components/config-form/section-configs/record.ts
@@ -44,6 +44,7 @@ const record: SectionConfigOverrides = {
   },
   camera: {
     restartRequired: [],
+    hiddenFields: ["retain_policy"],
   },
 };
 
diff --git a/web/src/types/frigateConfig.ts b/web/src/types/frigateConfig.ts
index 6e8dfcddd..b366f5389 100644
--- a/web/src/types/frigateConfig.ts
+++ b/web/src/types/frigateConfig.ts
@@ -191,7 +191,6 @@ export interface CameraConfig {
   record: {
     enabled: boolean;
     enabled_in_config: boolean;
-    retain_policy: "time" | "continuous_rollover";
     alerts: {
       post_capture: number;
       pre_capture: number;
@@ -625,4 +624,5 @@ export type StorageBreakdown = {
   overwritable: number;
   event_retention: number;
   protected: number;
+  units: string;
 };