Skip to content

Commit 493a98c

Browse files
fix(cleanup): resolve every cleanup entry point through one retention policy (#10562)
Cleanup is reachable through an aggregate planner and a set of category specialists. Every one of them is a delete path, and every one of them used to carry its own `default_value_t` retention literal. Those literals happened to equal the shipped configuration defaults, which made the drift invisible: the moment an operator widened `retention.runtime_tmp_days` to 30, `homeboy self cleanup-runtime-tmp --apply` still deleted at 7 days while `homeboy cleanup --include runtime-tmp` honored the configuration. Same shape for `runs retention` and `runs artifact cleanup-persisted` against `retention.terminal_run_days` and `retention.limit`. A configured retention window being silently ignored by a delete path is fail-open. Add `homeboy_core::cleanup::resolve_cleanup_policy`, following the precedent `controller_runtime::resolve_cleanup_options` set in #10288. `None` on a command flag now means "use the configured value" and is the only way a specialist can answer. The resolved policy is the manifest reported in output, so a report cannot describe a window the deletion did not apply. Fail-closed rules the resolver enforces: - A negative window or non-positive limit is rejected on every entry point, including when it arrives from configuration rather than from an argument. - `scan_limit()` converts the record budget with `unwrap_or(0)`. The CLI aggregate used `unwrap_or(usize::MAX)` in four places, which widens a delete budget when a conversion fails. - The two runner age floors become one named constant instead of a literal `24` in the aggregate beside `default_value_t = 24` in each specialist. They stay per-invocation arguments rather than becoming configuration keys: both live on a remote host whose clock and in-flight uploads the controller cannot fully observe, so lowering them should not persist into every future sweep. - `terminal_only` on persisted-artifact cleanup is documented as non-overridable. Releasing evidence for a run that is still executing, or whose state cannot be read, is data loss, not a preference. Delete `homeboy runs retention`. It was the one specialist with no narrowing argument left — its `--apply`, `--older-than-days`, and `--limit` were exactly the aggregate's. Every other specialist survives because it accepts a narrowing argument the aggregate cannot express (`--run-id`, `--kind`, `--component`, `--prefix`, `--runner`, `--passes`, `--cursor`) or is an explicit destructive escape hatch (`runtime controller-prune --ignore-retention`). Folding those into `--include`-scoped flags would put a dozen single-category flags on a surface that sweeps thirteen categories. Also account for the artifact root in `cleanup retained-storage`. It accumulated from five sources and never called `artifacts::root()`, so the one command whose purpose is "where did my disk go" was blind to the product's primary output store. Persisted run artifacts, runner downloads, and orphaned artifact bytes now contribute read-only plans, and `safe_next_commands` names a reclaim command for each. Bytes a planner would reclaim report `liveness: reclaimable` and are totalled separately: "cleanup cannot free this" and "cleanup has not freed this yet" are different answers and are never summed together. Unmeasured sizes are reported as zero and never inferred. Refs #10316 Co-authored-by: chubes-bot <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 41875c7 commit 493a98c

18 files changed

Lines changed: 1026 additions & 171 deletions

File tree

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

Lines changed: 327 additions & 72 deletions
Large diffs are not rendered by default.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,10 @@ pub(super) enum RunnerCommand {
358358
apply: bool,
359359

360360
/// Minimum slot age before an unselected slot is eligible.
361-
#[arg(long, default_value_t = 24)]
362-
min_age_hours: u64,
361+
/// Defaults to the shared runner age floor
362+
/// (`cleanup::RUNNER_MIN_AGE_HOURS`).
363+
#[arg(long)]
364+
min_age_hours: Option<u64>,
363365
},
364366
/// Execute a command on a configured runner. Use `homeboy runner exec [HOMEBOY_OPTIONS] <RUNNER> -- <COMMAND>...`.
365367
#[command(

crates/homeboy-cli/src/commands/runner/dispatch.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,10 @@ pub fn run(args: RunnerArgs) -> CmdResult<RunnerCommandOutput> {
220220
&runner_id,
221221
runner::RunnerBinaryCachePruneOptions {
222222
apply,
223-
min_age_hours,
223+
// One named age floor shared with `homeboy cleanup --include
224+
// runner-binary-caches` (#10316).
225+
min_age_hours: min_age_hours
226+
.unwrap_or(homeboy::core::cleanup::RUNNER_MIN_AGE_HOURS),
224227
},
225228
)),
226229
RunnerCommand::Exec {

crates/homeboy-cli/src/commands/runner/workspace/mod.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use clap::{Subcommand, ValueEnum};
22
use serde::Serialize;
33

4+
use homeboy::core::cleanup;
45
use homeboy::runner::runners::{
56
self as runner, RunnerWorkspaceApplyOutput, RunnerWorkspaceListOutput,
67
RunnerWorkspacePruneOutput, RunnerWorkspacePullOutput, RunnerWorkspaceSnapshotFilters,
@@ -128,12 +129,16 @@ pub(super) enum RunnerWorkspaceCommand {
128129
apply: bool,
129130

130131
/// Minimum workspace age before it can be considered orphaned.
131-
#[arg(long, default_value_t = 24)]
132-
min_age_hours: u64,
132+
/// Defaults to the shared runner age floor
133+
/// (`cleanup::RUNNER_MIN_AGE_HOURS`).
134+
#[arg(long)]
135+
min_age_hours: Option<u64>,
133136

134-
/// Maximum number of orphan candidates to report or remove.
135-
#[arg(long, default_value_t = 25)]
136-
limit: usize,
137+
/// Maximum number of orphan candidates to report or remove per pass.
138+
/// Defaults to the shared page size
139+
/// (`cleanup::RUNNER_WORKSPACE_PAGE_LIMIT`).
140+
#[arg(long)]
141+
limit: Option<usize>,
137142

138143
/// Maximum apply passes to run. Each pass re-scans and removes at most --limit candidates.
139144
#[arg(long, default_value_t = 1)]
@@ -221,8 +226,11 @@ pub(super) fn run(command: RunnerWorkspaceCommand) -> CmdResult<RunnerWorkspaceO
221226
&runner_id,
222227
runner::RunnerWorkspacePruneOptions {
223228
apply,
224-
min_age_hours,
225-
limit,
229+
// One named age floor shared with `homeboy cleanup --include
230+
// remote-lab-workspaces`, which used to carry its own literal
231+
// `24` beside this command's `default_value_t = 24` (#10316).
232+
min_age_hours: min_age_hours.unwrap_or(cleanup::RUNNER_MIN_AGE_HOURS),
233+
limit: limit.unwrap_or(cleanup::RUNNER_WORKSPACE_PAGE_LIMIT),
226234
passes,
227235
cursor,
228236
},

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use super::types::{RunsArgs, RunsArtifactArgs, RunsArtifactCommand, RunsCommand,
99
use super::CmdResult;
1010
use super::{
1111
bench, compare, distribution, dossier, drift, evidence, findings, fuzz_compare, handlers,
12-
hotspots, latest, loop_sync, proof, query, reconcile, refs, resources, retention, watch,
12+
hotspots, latest, loop_sync, proof, query, reconcile, refs, resources, watch,
1313
};
1414

1515
impl RunsArgs {
@@ -172,7 +172,6 @@ pub fn run(args: RunsArgs) -> CmdResult<RunsOutput> {
172172
RunsCommand::FuzzCompare(args) => fuzz_compare::fuzz_compare_from_args(args),
173173
RunsCommand::Hotspots(args) => hotspots::runs_hotspots(args),
174174
RunsCommand::Reconcile(args) => reconcile::reconcile_runs(args),
175-
RunsCommand::Retention(args) => retention::retain_terminal_runs(args),
176175
RunsCommand::Watch(args) => watch::watch_run(args),
177176
RunsCommand::Cancel { run_id } => handlers::cancel_run(&run_id),
178177
RunsCommand::Show {

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ mod refs;
3535
mod remote;
3636
mod remote_artifact;
3737
mod resources;
38-
mod retention;
3938
#[cfg(test)]
4039
mod tests;
4140
mod types;
@@ -46,9 +45,7 @@ use super::CmdResult;
4645
// Public command-layer API consumed by routing, raw/json output, rig, and bench.
4746
pub use dispatch::{global_runner_error, run, run_markdown};
4847
pub use handlers::list_runs;
49-
pub use types::{
50-
RunsArgs, RunsOutput, RunsRetentionArgs, RunsRetentionOutput, HOSTED_BLUEPRINT_VIEWER,
51-
};
48+
pub use types::{RunsArgs, RunsOutput, HOSTED_BLUEPRINT_VIEWER};
5249

5350
// Intra-module re-exports so sibling submodules (and the test modules) can
5451
// reference shared items via `super::` without depending on each other's

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

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::fs;
22
use std::path::{Path, PathBuf};
33

4+
use homeboy::core::cleanup::{resolve_cleanup_policy, CleanupPolicyOverrides};
45
use homeboy::core::observation::artifact_preview;
56
use homeboy::core::observation::runs_service::{
67
self, PersistedArtifactCleanupOptions, RunnerDownloadCleanupOptions,
@@ -445,22 +446,35 @@ pub fn cleanup_downloads(args: RunsArtifactCleanupDownloadsArgs) -> CmdResult<Ru
445446
}
446447

447448
pub fn cleanup_persisted(args: RunsArtifactCleanupPersistedArgs) -> CmdResult<RunsOutput> {
449+
// Same resolver as `homeboy cleanup --include persisted-run-artifacts`.
450+
// The narrowing filters below (`--run-id`, `--kind`, `--type`,
451+
// `--run-kind`, `--component`) are what keep this specialist alive; the
452+
// retention window is not one of them (#10316).
453+
let policy = resolve_cleanup_policy(CleanupPolicyOverrides {
454+
terminal_run_days: args.older_than_days,
455+
limit: args.limit,
456+
..CleanupPolicyOverrides::default()
457+
})?;
448458
let outcome = runs_service::cleanup_persisted_artifacts(PersistedArtifactCleanupOptions {
449459
apply: args.apply,
450-
older_than_days: args.older_than_days,
460+
older_than_days: policy.terminal_run_days,
451461
run_id: args.run_id,
452462
kind: args.kind,
453463
artifact_type: args.artifact_type,
454464
run_kind: args.run_kind,
455465
component_id: args.component_id,
456-
limit: args.limit,
466+
limit: policy.limit,
467+
// Never widened by an operator flag: releasing evidence for a run that
468+
// is still executing, or whose state cannot be read, is a data-loss
469+
// path, not a retention preference.
457470
terminal_only: true,
458471
})?;
459472

460473
Ok((
461474
RunsOutput::ArtifactCleanupPersisted(RunsArtifactCleanupPersistedOutput {
462475
command: "runs.artifact.cleanup-persisted",
463476
dry_run: outcome.dry_run,
477+
retention: policy,
464478
artifact_root: outcome.artifact_root.display().to_string(),
465479
older_than_days: outcome.older_than_days,
466480
inspected_count: outcome.totals.inspected_count,
@@ -1057,13 +1071,13 @@ mod tests {
10571071
// active lease, even when the age threshold is zero.
10581072
let active = cleanup_persisted(RunsArtifactCleanupPersistedArgs {
10591073
apply: true,
1060-
older_than_days: 0,
1074+
older_than_days: Some(0),
10611075
run_id: Some(run.id.clone()),
10621076
kind: None,
10631077
artifact_type: None,
10641078
run_kind: None,
10651079
component_id: None,
1066-
limit: 100,
1080+
limit: Some(100),
10671081
})
10681082
.expect("active cleanup")
10691083
.0;
@@ -1081,13 +1095,13 @@ mod tests {
10811095

10821096
let dry = cleanup_persisted(RunsArtifactCleanupPersistedArgs {
10831097
apply: false,
1084-
older_than_days: 0,
1098+
older_than_days: Some(0),
10851099
run_id: Some(run.id.clone()),
10861100
kind: None,
10871101
artifact_type: None,
10881102
run_kind: None,
10891103
component_id: None,
1090-
limit: 100,
1104+
limit: Some(100),
10911105
})
10921106
.expect("dry-run")
10931107
.0;
@@ -1103,13 +1117,13 @@ mod tests {
11031117

11041118
let applied = cleanup_persisted(RunsArtifactCleanupPersistedArgs {
11051119
apply: true,
1106-
older_than_days: 0,
1120+
older_than_days: Some(0),
11071121
run_id: Some(run.id.clone()),
11081122
kind: None,
11091123
artifact_type: None,
11101124
run_kind: None,
11111125
component_id: None,
1112-
limit: 100,
1126+
limit: Some(100),
11131127
})
11141128
.expect("apply")
11151129
.0;

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

Lines changed: 0 additions & 24 deletions
This file was deleted.

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

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use serde_json::Value;
1111

1212
use homeboy::core::artifact_links::ArtifactViewerDescriptor;
1313
use homeboy::core::artifacts::{ArtifactPreviewEntrypoint, MatrixArtifactSummary};
14+
use homeboy::core::cleanup::CleanupPolicy;
1415
use homeboy::core::observation::evidence_report::DirectoryArtifactPublicationGuidance;
1516
use homeboy::core::observation::runs_service;
1617
use homeboy::core::observation::ArtifactRecord;
@@ -85,8 +86,6 @@ pub(super) enum RunsCommand {
8586
Hotspots(RunsHotspotsArgs),
8687
/// Mark orphaned running observation records stale
8788
Reconcile(RunsReconcileArgs),
88-
/// Plan or apply bounded retention of terminal observation rows and dependent records
89-
Retention(RunsRetentionArgs),
9089
/// Block and stream a run's status until it reaches a terminal state,
9190
/// exiting with a code that reflects pass/fail. Works for attached and
9291
/// detached/offloaded runs.
@@ -214,31 +213,6 @@ pub struct RunsListArgs {
214213
pub include_active_runner_jobs: bool,
215214
}
216215

217-
#[derive(Args, Clone)]
218-
pub struct RunsRetentionArgs {
219-
/// Delete the planned terminal rows. Without this flag, only reports the plan.
220-
#[arg(long)]
221-
pub apply: bool,
222-
/// Only include terminal runs finished more than this many days ago.
223-
#[arg(long, default_value_t = 30)]
224-
pub older_than_days: i64,
225-
/// Maximum terminal run rows to inspect and remove in one invocation.
226-
#[arg(long, default_value_t = 1000)]
227-
pub limit: i64,
228-
}
229-
230-
#[derive(Serialize)]
231-
pub struct RunsRetentionOutput {
232-
pub command: &'static str,
233-
pub dry_run: bool,
234-
pub older_than_days: i64,
235-
pub candidate_run_ids: Vec<String>,
236-
pub artifact_cleanup: Vec<runs_service::PersistedArtifactCleanupOutcome>,
237-
pub lifecycle_directories: Vec<runs_service::TerminalRunLifecycleDirectory>,
238-
pub skipped_run_ids: Vec<String>,
239-
pub removed_run_count: usize,
240-
}
241-
242216
#[derive(Serialize)]
243217
#[serde(tag = "variant", content = "payload", rename_all = "snake_case")]
244218
pub enum RunsOutput {
@@ -269,7 +243,6 @@ pub enum RunsOutput {
269243
FuzzCompare(FuzzCompareOutput),
270244
Hotspots(RunsHotspotsOutput),
271245
Reconcile(RunsReconcileOutput),
272-
Retention(RunsRetentionOutput),
273246
Watch(RunsWatchOutput),
274247
Cancel(RunsCancelOutput),
275248
Export(RunsExportOutput),
@@ -767,8 +740,9 @@ pub struct RunsArtifactCleanupPersistedArgs {
767740
#[arg(long)]
768741
pub apply: bool,
769742
/// Only include artifacts older than this many days.
770-
#[arg(long, default_value_t = 30)]
771-
pub older_than_days: i64,
743+
/// Defaults to the configured `retention.terminal_run_days`.
744+
#[arg(long)]
745+
pub older_than_days: Option<i64>,
772746
/// Limit cleanup to one run id.
773747
#[arg(long)]
774748
pub run_id: Option<String>,
@@ -785,14 +759,17 @@ pub struct RunsArtifactCleanupPersistedArgs {
785759
#[arg(long = "component")]
786760
pub component_id: Option<String>,
787761
/// Maximum artifact rows to inspect in one invocation.
788-
#[arg(long, default_value_t = 1000)]
789-
pub limit: i64,
762+
/// Defaults to the configured `retention.limit`.
763+
#[arg(long)]
764+
pub limit: Option<i64>,
790765
}
791766

792767
#[derive(Serialize)]
793768
pub struct RunsArtifactCleanupPersistedOutput {
794769
pub command: &'static str,
795770
pub dry_run: bool,
771+
/// The resolved policy this invocation applied.
772+
pub retention: CleanupPolicy,
796773
pub artifact_root: String,
797774
pub older_than_days: i64,
798775
pub inspected_count: usize,

0 commit comments

Comments
 (0)