fix(cleanup): give runner-downloads an ownership, age, and liveness predicate (#10584)

* fix(cleanup): give runner-downloads an ownership, age, and liveness predicate

`runs_service::cleanup_runner_downloads` was an unconditional
`fs::remove_dir_all` of `<artifact-root>/runner`. Its only checks were
path-containment ones — `--run-id` requires `--runner`, each filter must be
a single normal path component, the root must be a real directory — which
prove the deletion stays inside the cache and prove nothing about whether
the bytes are dead. `runner-downloads` was also swept by a bare `homeboy
cleanup --apply`, so artifacts an operator pulled seconds earlier were
deleted by an unrelated sweep.

That tree has exactly one writer: the default output path of
`download_remote_artifact`, `<artifact-root>/runner/<runner-id>/<run-id>/
<file>`. Every caller of it is a fetch someone asked for (`runs artifact
get`, `runs artifacts --pull`, `lab apply`, evidence mirroring, the HTTP
artifact endpoint), and `runs artifact get` hands that path back as the
location of the operator's file.

The predicate now requires, per cache directory:

- ownership by the canonical `<runner-id>/<run-id>` name shape, with no
  symlink at either level. The database is deliberately not joined: bytes
  here are written before, and usually without, any local artifacts row, so
  row absence is the normal state of a download that is succeeding.
- a fixed 24h floor (`cleanup::RUNNER_MIN_AGE_HOURS`) over the *newest* byte
  anywhere in the subtree, so one fresh pull re-arms the whole directory.
  Not operator-overridable.
- a non-terminal-run veto read from the observation store in the retain
  direction only. A missing row never authorizes removal.
- fail closed on every uncertainty: unreadable or future-dated mtime,
  unwalkable subtree, out-of-root path, unopenable store, or a truncated
  running-run scan all retain.

Removal is per cache directory rather than whole-root, so a stale cache and
a fresh one under the same runner are decided independently; the cache root
is never removed and an emptied `<runner-id>` directory is pruned only by a
non-recursive `remove_dir`. Sizes stay advisory and move the verdict in
neither direction. `--runner`/`--run-id` narrow which candidates are
considered and never waive a check.

The category is also withheld from the bare sweep. The predicate fixes the
acute data-loss case but cannot fix the remaining one: the writer emits the
same name shape for an operator's deliberate pull and for an internal
auto-fetch, so ownership of *intent* cannot be proven by name. Until the
writer tags its output, `--include runner-downloads` is the honest contract.
`cleanup retained-storage` still accounts for the bytes, now split into what
a sweep would reclaim and what it is holding on to, and still names the
reclaim command.

The specialist resolves its inspection budget through
`resolve_cleanup_policy` and echoes the policy, matching the other delete
paths unified in #10562. It carried no `default_value_t` retention literal,
so it was not drifting — it was simply not participating.

Fixes #10564

* fix(cleanup): derive Debug on CleanupFilters

Workspace Tests Compile failed with E0277: assert macros in the new
tests format CleanupFilters, which had no Debug impl. The CLI Reference
Docs gate failed for the same reason -- it runs cargo test -p homeboy-cli
to regenerate, so it could not compile either. The regenerated doc tree
is byte-identical to the checked-in one; the docs were never stale.

---------

Co-authored-by: chubes-bot <266378653+homeboy-ci[bot]@users.noreply.github.com>

Author: chubes4

crates/homeboy-cli/src/commands/cleanup.rs

@@ -35,6 +35,8 @@ pub struct CleanupArgs {
     pub apply: bool,
 
     /// Include only these cleanup categories. Comma-separated or repeatable.
+    /// `runner-downloads` is opt-in only: it holds artifacts an operator asked
+    /// Homeboy to fetch, so a bare sweep never includes it.
     #[arg(long, value_enum, value_delimiter = ',')]
     pub include: Vec<CleanupCategoryArg>,
 
@@ -535,25 +537,52 @@ fn artifact_root_records(
         });
     }
 
+    // The whole cache used to be reported as reclaimable, because the category
+    // deleted all of it unconditionally. It now reports the two halves its
+    // predicate actually produces (#10564): what a sweep would reclaim, and
+    // what it is holding on to.
     let downloads = runs_service::cleanup_runner_downloads(RunnerDownloadCleanupOptions {
         apply: false,
         runner: None,
         run_id: None,
+        limit: policy.scan_limit(),
     })?;
-    if downloads.size_bytes > 0 || downloads.file_count > 0 || downloads.directory_count > 0 {
+    let downloads_root = downloads.root.display().to_string();
+    if downloads.planned_count > 0 {
         records.push(RetainedStorageRecord {
             category: "runner_downloads".to_string(),
             reason: format!(
-                "cached runner artifact downloads ({} file(s), {} directory(ies))",
-                downloads.file_count, downloads.directory_count
+                "{} cached runner download(s) past the fixed {}s age floor with no non-terminal owning run ({} file(s), {} directory(ies))",
+                downloads.planned_count,
+                downloads.min_age_seconds,
+                downloads.file_count,
+                downloads.directory_count
             ),
             owner: "homeboy".to_string(),
             run_id: None,
             liveness: LIVENESS_RECLAIMABLE.to_string(),
+            age: age_bucket(downloads.min_age_seconds),
+            age_seconds: Some(downloads.min_age_seconds),
+            size_bytes: downloads.planned_size_bytes,
+            reference: downloads_root.clone(),
+        });
+    }
+    if downloads.skipped_count > 0 {
+        records.push(RetainedStorageRecord {
+            category: "runner_downloads".to_string(),
+            reason: format!(
+                "{} cached runner download(s) retained: younger than the age floor, claimed by a non-terminal run, or not the canonical <runner>/<run> shape; bytes not measured",
+                downloads.skipped_count
+            ),
+            owner: "homeboy".to_string(),
+            run_id: None,
+            liveness: "lifecycle_pinned".to_string(),
             age: "unknown".to_string(),
             age_seconds: None,
-            size_bytes: downloads.size_bytes,
-            reference: downloads.root.display().to_string(),
+            // Advisory-signal rule: retained entries are deliberately not
+            // measured, so a zero here is "not measured", never "empty".
+            size_bytes: 0,
+            reference: format!("{downloads_root} (retained runner downloads)"),
         });
     }
 
@@ -1009,15 +1038,16 @@ fn cleanup_inventory(args: CleanupArgs) -> homeboy::core::Result<Value> {
             apply,
             runner: None,
             run_id: None,
+            limit: policy.scan_limit(),
         })?;
         categories.push(category_from_output(
             RUNNER_DOWNLOADS_METADATA,
             apply,
-            output.file_count + output.directory_count,
-            usize::from(output.removed),
-            0,
-            output.size_bytes,
-            if output.removed { output.size_bytes } else { 0 },
+            output.inspected_count,
+            output.removed_count,
+            output.skipped_count,
+            output.planned_size_bytes,
+            output.removed_size_bytes,
             output,
         )?);
     }
@@ -1180,14 +1210,37 @@ struct CleanupCategorySelection {
     exclude: Vec<CleanupCategoryArg>,
 }
 
+/// Categories a bare `homeboy cleanup` deliberately does not sweep.
+///
+/// Everything else in the aggregate reclaims bytes Homeboy produced as a
+/// byproduct of its own work — scratch, build targets, temp trees, crash
+/// residue, remote workspaces. `runner-downloads` is different in kind: every
+/// byte under `<artifact-root>/runner` is the result of a fetch an operator
+/// asked for, and `homeboy runs artifact get` hands that exact path back to
+/// them as the location of their file. The predicate in
+/// [`homeboy::core::observation::runs_service::cleanup_runner_downloads`] proves
+/// the bytes are old and unclaimed, but it cannot prove the operator is *done*
+/// with them, because the single writer emits the same name shape for an
+/// operator pull and for an internal auto-fetch (#10564).
+///
+/// Until the writer tags its output, an explicit `--include runner-downloads`
+/// is the honest contract: being absent from a default sweep is cheap and
+/// reversible, and a wrong delete is neither. The category stays fully visible
+/// in `homeboy cleanup retained-storage`, which names the reclaim command.
+const OPT_IN_ONLY_CATEGORIES: &[CleanupCategoryArg] = &[CleanupCategoryArg::RunnerDownloads];
+
 impl CleanupCategorySelection {
     fn new(include: Vec<CleanupCategoryArg>, exclude: Vec<CleanupCategoryArg>) -> Self {
         Self { include, exclude }
     }
 
     fn includes(&self, category: CleanupCategoryArg) -> bool {
-        (self.include.is_empty() || self.include.contains(&category))
-            && !self.exclude.contains(&category)
+        let selected = if self.include.is_empty() {
+            !OPT_IN_ONLY_CATEGORIES.contains(&category)
+        } else {
+            self.include.contains(&category)
+        };
+        selected && !self.exclude.contains(&category)
     }
 }
 
@@ -2261,6 +2314,22 @@ mod tests {
                 CleanupCategoryArg::RuntimeTmp,
                 false,
             ),
+            // #10564: opt-in-only categories are absent from a bare sweep,
+            // reachable by an explicit `--include`, and still suppressible by
+            // `--exclude`.
+            (vec![], vec![], CleanupCategoryArg::RunnerDownloads, false),
+            (
+                vec![CleanupCategoryArg::RunnerDownloads],
+                vec![],
+                CleanupCategoryArg::RunnerDownloads,
+                true,
+            ),
+            (
+                vec![CleanupCategoryArg::RunnerDownloads],
+                vec![CleanupCategoryArg::RunnerDownloads],
+                CleanupCategoryArg::RunnerDownloads,
+                false,
+            ),
         ];
 
         for (include, exclude, category, expected) in cases {
@@ -2271,6 +2340,40 @@ mod tests {
         }
     }
 
+    #[test]
+    fn only_runner_downloads_is_withheld_from_the_bare_sweep() {
+        // A bare `homeboy cleanup --apply` must keep sweeping everything that
+        // reclaims Homeboy's own byproducts. Only the operator-owned download
+        // cache is withheld, and the withheld set is asserted exactly so a
+        // future category cannot be quietly dropped from the default (#10564).
+        assert_eq!(
+            OPT_IN_ONLY_CATEGORIES.to_vec(),
+            vec![CleanupCategoryArg::RunnerDownloads]
+        );
+
+        let bare = CleanupCategorySelection::new(Vec::new(), Vec::new());
+        for category in [
+            CleanupCategoryArg::RepoArtifacts,
+            CleanupCategoryArg::TaskWorktrees,
+            CleanupCategoryArg::WorktreeProviders,
+            CleanupCategoryArg::TerminalRuns,
+            CleanupCategoryArg::PersistedRunArtifacts,
+            CleanupCategoryArg::OrphanedArtifactBytes,
+            CleanupCategoryArg::RunnerBinaryCaches,
+            CleanupCategoryArg::RemoteLabWorkspaces,
+            CleanupCategoryArg::RuntimeTmp,
+            CleanupCategoryArg::ControllerScratch,
+            CleanupCategoryArg::SharedCargoTargets,
+            CleanupCategoryArg::ControllerRuntimes,
+        ] {
+            assert!(
+                bare.includes(category),
+                "bare cleanup must still sweep {category:?}"
+            );
+        }
+        assert!(!bare.includes(CleanupCategoryArg::RunnerDownloads));
+    }
+
     #[test]
     fn aggregate_repo_artifact_roots_do_not_depend_on_the_caller_directory() {
         let configured = vec![

crates/homeboy-cli/src/commands/runs/remote_artifact.rs

@@ -420,10 +420,16 @@ fn directory_contains_html_inner(path: &Path, seen: usize) -> bool {
 }
 
 pub fn cleanup_downloads(args: RunsArtifactCleanupDownloadsArgs) -> CmdResult<RunsOutput> {
+    // Same resolver as `homeboy cleanup --include runner-downloads`. `--runner`
+    // and `--run-id` are the narrowing filters that keep this specialist alive;
+    // the inspection budget is not one of them, and the age floor is fixed and
+    // deliberately unexposed (#10564).
+    let policy = resolve_cleanup_policy(CleanupPolicyOverrides::default())?;
     let outcome = runs_service::cleanup_runner_downloads(RunnerDownloadCleanupOptions {
         apply: args.apply,
         runner: args.runner,
         run_id: args.run_id,
+        limit: policy.scan_limit(),
     })?;
 
     Ok((
@@ -434,12 +440,20 @@ pub fn cleanup_downloads(args: RunsArtifactCleanupDownloadsArgs) -> CmdResult<Ru
                 .canonical_cleanup_command(args.apply),
             specialist_cleanup_command: RUNNER_DOWNLOADS_METADATA.specialist_command(args.apply),
             dry_run: outcome.dry_run,
+            retention: policy,
             root: outcome.root.display().to_string(),
-            removed: outcome.removed,
+            min_age_seconds: outcome.min_age_seconds,
+            liveness: outcome.liveness,
+            inspected_count: outcome.inspected_count,
+            planned_count: outcome.planned_count,
+            removed_count: outcome.removed_count,
+            skipped_count: outcome.skipped_count,
             file_count: outcome.file_count,
             directory_count: outcome.directory_count,
-            size_bytes: outcome.size_bytes,
-            paths: outcome.paths,
+            planned_size_bytes: outcome.planned_size_bytes,
+            removed_size_bytes: outcome.removed_size_bytes,
+            truncated: outcome.truncated,
+            rows: outcome.rows,
         }),
         0,
     ))
@@ -955,12 +969,13 @@ mod tests {
     }
 
     #[test]
-    fn cleanup_downloads_plans_and_removes_runner_cache() {
+    fn cleanup_downloads_retains_a_freshly_pulled_cache_and_stays_wired_to_the_aggregate() {
         let _guard = artifact_root_test_lock();
         with_isolated_home(|home| {
             let artifact_root = home.path().join("artifacts");
             homeboy::core::set_artifact_root_override(Some(artifact_root.clone()));
 
+            // Exactly what `homeboy runs artifacts <run> --pull` writes.
             let run_dir = artifact_root.join("runner").join("local").join("run-1");
             fs::create_dir_all(&run_dir).expect("run dir");
             fs::write(run_dir.join("trace.zip"), b"trace").expect("trace");
@@ -987,10 +1002,16 @@ mod tests {
                 dry.canonical_cleanup_command,
                 RUNNER_DOWNLOADS_METADATA.canonical_cleanup_command(false)
             );
-            assert!(!dry.removed);
-            assert_eq!(dry.file_count, 2);
-            assert_eq!(dry.directory_count, 0);
-            assert_eq!(dry.size_bytes, 7);
+            // #10564: the cache was written seconds ago, so it is a candidate
+            // the predicate inspects and retains — not bytes to plan away.
+            assert_eq!(dry.inspected_count, 1);
+            assert_eq!(dry.planned_count, 0);
+            assert_eq!(dry.skipped_count, 1);
+            assert_eq!(dry.min_age_seconds, 24 * 60 * 60);
+            assert_eq!(
+                dry.retention.schema,
+                homeboy::core::cleanup::CLEANUP_POLICY_SCHEMA
+            );
             assert!(run_dir.exists());
 
             let (inventory, _) =
@@ -1016,6 +1037,9 @@ mod tests {
                 dry.specialist_cleanup_command
             );
 
+            // An explicit `--apply` on the freshly pulled cache still removes
+            // nothing: the narrowing filter selects a candidate, it does not
+            // waive the age floor.
             let applied = cleanup_downloads(RunsArtifactCleanupDownloadsArgs {
                 apply: true,
                 runner: Some("local".to_string()),
@@ -1031,14 +1055,10 @@ mod tests {
                 applied.canonical_cleanup_command,
                 RUNNER_DOWNLOADS_METADATA.canonical_cleanup_command(true)
             );
-            assert_eq!(
-                applied.specialist_cleanup_command,
-                RUNNER_DOWNLOADS_METADATA.specialist_command(true)
-            );
-            assert!(applied.removed);
-            assert_eq!(applied.file_count, 2);
-            assert_eq!(applied.size_bytes, 7);
-            assert!(!run_dir.exists());
+            assert_eq!(applied.removed_count, 0);
+            assert_eq!(applied.skipped_count, 1);
+            assert!(run_dir.join("trace.zip").exists());
+            assert!(run_dir.join("report.json").exists());
         });
     }
 

crates/homeboy-cli/src/commands/runs/types.rs

@@ -726,12 +726,26 @@ pub struct RunsArtifactCleanupDownloadsOutput {
     pub canonical_cleanup_command: String,
     pub specialist_cleanup_command: &'static str,
     pub dry_run: bool,
+    /// The resolved policy this invocation applied.
+    pub retention: CleanupPolicy,
+    /// The cache scope inspected, narrowed by `--runner` / `--run-id`.
     pub root: String,
-    pub removed: bool,
+    /// Fixed age floor. Not overridable: see
+    /// `runs_service::RUNNER_DOWNLOAD_MIN_AGE`.
+    pub min_age_seconds: u64,
+    /// Whether the non-terminal-run veto could be evaluated. `unavailable`
+    /// means every candidate was retained.
+    pub liveness: runs_service::RunnerDownloadLiveness,
+    pub inspected_count: usize,
+    pub planned_count: usize,
+    pub removed_count: usize,
+    pub skipped_count: usize,
     pub file_count: usize,
     pub directory_count: usize,
-    pub size_bytes: u64,
-    pub paths: Vec<String>,
+    pub planned_size_bytes: u64,
+    pub removed_size_bytes: u64,
+    pub truncated: bool,
+    pub rows: Vec<runs_service::RunnerDownloadCleanupRow>,
 }
 
 #[derive(Args, Clone)]

crates/homeboy-core/src/observation/runs_service/mod.rs

@@ -57,24 +57,6 @@ pub enum ArtifactGetSource {
     Remote,
 }
 
-#[derive(Debug, Clone, Default)]
-pub struct RunnerDownloadCleanupOptions {
-    pub apply: bool,
-    pub runner: Option<String>,
-    pub run_id: Option<String>,
-}
-
-#[derive(Debug, Clone, Serialize)]
-pub struct RunnerDownloadCleanupOutcome {
-    pub dry_run: bool,
-    pub root: PathBuf,
-    pub removed: bool,
-    pub file_count: usize,
-    pub directory_count: usize,
-    pub size_bytes: u64,
-    pub paths: Vec<String>,
-}
-
 #[derive(Debug, Clone)]
 pub struct PersistedArtifactCleanupOptions {
     pub apply: bool,
@@ -153,14 +135,6 @@ pub struct TerminalRunLifecycleDirectory {
     pub size_bytes: u64,
 }
 
-#[derive(Debug, Default)]
-struct RunnerDownloadCleanupPreview {
-    file_count: usize,
-    directory_count: usize,
-    size_bytes: u64,
-    paths: Vec<String>,
-}
-
 /// Storage classes recognized by [`classify_artifact_storage`].
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ArtifactStorage {