diff --git a/app/src-tauri/src/docker/disk.rs b/app/src-tauri/src/docker/disk.rs index ef0d818..43d7b65 100644 --- a/app/src-tauri/src/docker/disk.rs +++ b/app/src-tauri/src/docker/disk.rs @@ -97,7 +97,19 @@ pub struct ProjectDiskRow { pub snapshot_shared_bytes: i64, /// How many layers the snapshot has stacked **above its base image**. This /// is the number that explains the growth: one per recreation. + /// + /// Only means that when [`Self::base_lineage_known`] is true. Otherwise it + /// is every layer carrying bytes, base included — an upper bound, and a + /// misleading one to present as a recreation count. pub snapshot_commit_layers: u32, + /// Whether the base image this snapshot descends from could be identified. + /// + /// False when `triple-c.base-image-id` is absent, which is the **normal** + /// case for a project created before that label existed. The UI must not + /// present `snapshot_commit_layers` as a recreation count in that state, + /// and compaction is not offered, because a never-recreated project would + /// otherwise report its base's ~15 layers and qualify. + pub base_lineage_known: bool, /// Bytes those stacked layers account for. `None` when the base image the /// snapshot descends from is no longer on the daemon, so the split cannot /// be measured and must not be guessed. @@ -422,7 +434,18 @@ pub struct ReclaimOutcome { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct ReclaimResult { - pub target: ReclaimTarget, + /// The reclaim target this reports on, or `None` when it reports a + /// [`destroy`]. + /// + /// Deliberately not reused to carry a destructive action: an earlier + /// version returned `OrphanVolume { name }` for a home-volume deletion, + /// which named a volume that was never an orphan and would attribute the + /// outcome to a plan row the user never ticked. `destroyed` carries it + /// instead, and exactly one of the two is ever set. + pub target: Option, + /// The destructive action this reports on, when it is one. + #[serde(default)] + pub destroyed: Option, pub ok: bool, /// Bytes actually freed, measured after the fact. pub freed_bytes: i64, @@ -763,6 +786,14 @@ pub fn parse_reclaimed_space(output: &str) -> i64 { /// [`restore_image_config`], which is why this function does not try to emit it /// as Dockerfile instructions. A multi-line `CLAUDE_INSTRUCTIONS` env var alone /// makes that escaping a bad bet. +/// +/// The one label it *does* emit is `triple-c.managed=true`, and it is not +/// decoration. Everything that cleans up after this build — the discard path +/// when the result is not smaller, the untag after a successful commit — relies +/// on `sweep_orphaned_snapshots` collecting the intermediate, and that sweep +/// filters on `dangling=true` **and** this label. Without it the sweep can +/// never match, and the flattened intermediate is left to whatever `untag_image` +/// happens to delete on its own. pub fn compaction_dockerfile(snapshot_ref: &str, scrub_script: &str) -> String { // The scrub script is multi-line shell. `RUN` takes it verbatim only if the // newlines are escaped, so it is folded onto one line with `;` separators — @@ -773,7 +804,8 @@ pub fn compaction_dockerfile(snapshot_ref: &str, scrub_script: &str) -> String { "FROM {snapshot_ref} AS src\n\ RUN {folded}\n\ FROM scratch\n\ - COPY --from=src / /\n" + COPY --from=src / /\n\ + LABEL {LABEL_MANAGED}=true\n" ) } @@ -1000,7 +1032,13 @@ fn projects_json_health() -> (bool, bool) { /// `LABEL` lines carries neither label, and a globals block that could not name /// the 4.7 GB image every project sits on would be missing the obvious. fn is_base_image_reference(reference: &str) -> bool { - let repo = reference.split(':').next().unwrap_or(reference); + // Split on the *tag*, not the first colon: `localhost:5000/triple-c-sandbox:latest` + // has a registry port, and splitting on the first colon would yield + // `localhost`. A tag never contains `/`, which is what tells the two apart. + let repo = match reference.rsplit_once(':') { + Some((repo, tag)) if !tag.contains('/') => repo, + _ => reference, + }; repo == "triple-c" || repo.ends_with("/triple-c-sandbox") || repo == "triple-c-sandbox" @@ -1081,6 +1119,7 @@ pub async fn scan(projects: &[Project]) -> Result { }; let mut stats = LayerStats::default(); + let mut base_lineage_known = false; if let Some(image) = snapshot { let history = docker .image_history(&image.id) @@ -1112,6 +1151,7 @@ pub async fn scan(projects: &[Project]) -> Result { }, None => None, }; + base_lineage_known = base_len.is_some(); stats = layer_stats(&history, base_len); } @@ -1140,6 +1180,7 @@ pub async fn scan(projects: &[Project]) -> Result { snapshot_bytes, snapshot_shared_bytes, snapshot_commit_layers: stats.commit_layers, + base_lineage_known, // Prefer the daemon's own measurement of what is unique to this // image over layer arithmetic; fall back to the layer sum when // `df()` did not compute a shared size. @@ -1209,6 +1250,12 @@ pub async fn scan(projects: &[Project]) -> Result { } } base_images.sort_by(|a, b| b.bytes.cmp(&a.bytes)); + // Full size per base, not `size - shared_size`: a base's shared bytes are + // shared with *its own snapshots*, so netting them out would report the + // 4.7 GB image every project sits on as ~0. The residual imprecision is two + // *different* bases that share lower layers with each other, whose common + // layers are counted twice here — worth knowing before treating this total + // as exact. let base_images_bytes = base_images.iter().map(|b| b.bytes).sum(); let (json_exists, json_parsed) = projects_json_health(); @@ -1663,7 +1710,11 @@ pub async fn list_reclaimable( ) }) .unwrap_or(0); - if row.snapshot_exists && row.snapshot_commit_layers > 1 && ceiling > 0 { + if row.snapshot_exists + && row.base_lineage_known + && row.snapshot_commit_layers > 1 + && ceiling > 0 + { items.push({ // Safety and reach are read off the target, never restated: a literal // here that disagreed with the classifier is exactly the drift this @@ -1763,7 +1814,9 @@ pub async fn list_reclaimable( project_name: row.project_name.clone(), label: "Home volume".to_string(), loses: "Shell history, dotfiles, every toolchain installed under $HOME, and any \ - Playwright browsers. Not recoverable." + Playwright browsers. Not recoverable. The project's container is removed \ + too, because a stopped container still holds its volumes open — it is \ + rebuilt from the snapshot on the next start." .to_string(), bytes: row.home_volume_bytes, blocked: blocked.clone(), @@ -1778,7 +1831,9 @@ pub async fn list_reclaimable( project_name: row.project_name.clone(), label: "Claude config volume".to_string(), loses: "The Claude login credential, installed plugins and skills, and EVERY \ - conversation transcript for this project. Not recoverable." + conversation transcript for this project. Not recoverable. The project's \ + container is removed too, because a stopped container still holds its \ + volumes open — it is rebuilt from the snapshot on the next start." .to_string(), bytes: row.config_volume_bytes, blocked: blocked.clone(), @@ -2112,7 +2167,8 @@ fn find_project<'a>(projects: &'a [Project], project_id: &str) -> Result<&'a Pro fn failed(target: ReclaimTarget, message: String) -> ReclaimResult { ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: false, freed_bytes: 0, projected_bytes: None, @@ -2196,7 +2252,8 @@ async fn reclaim_dangling(target: &ReclaimTarget) -> ReclaimResult { message.push_str(&format!(" {} could not be removed; see the log.", errors)); } ReclaimResult { - target: target.clone(), + target: Some(target.clone()), + destroyed: None, ok: errors == 0, freed_bytes: freed, projected_bytes: None, @@ -2222,7 +2279,8 @@ async fn reclaim_build_cache(all: bool) -> ReclaimResult { } match docker_cli(&args).await { Ok(output) => ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: parse_reclaimed_space(&output), projected_bytes: None, @@ -2302,7 +2360,8 @@ async fn reclaim_migration_pins() -> ReclaimResult { sweep.reclaimed_bytes ); ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: sweep.reclaimed_bytes, projected_bytes: None, @@ -2352,7 +2411,8 @@ fn reclaim_migration_staging(projects: &[Project]) -> ReclaimResult { } } ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: freed, projected_bytes: None, @@ -2407,7 +2467,8 @@ async fn reclaim_containers( } } ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: errors == 0, freed_bytes: freed, projected_bytes: None, @@ -2459,7 +2520,8 @@ async fn reclaim_orphan_volume(name: &str, projects: &[Project]) -> ReclaimResul match docker.remove_volume(name, None).await { Ok(()) => ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: volume.bytes, projected_bytes: None, @@ -2606,7 +2668,8 @@ pub async fn compact_snapshot(project: &Project) -> ReclaimResult { let _ = migration::untag_image(&staging_ref).await; let _ = container::sweep_orphaned_snapshots().await; return ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: 0, projected_bytes: projected, @@ -2647,7 +2710,8 @@ pub async fn compact_snapshot(project: &Project) -> ReclaimResult { sweep.removed.len() ); ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: freed, projected_bytes: projected, @@ -2726,6 +2790,59 @@ async fn build_from_dockerfile(dockerfile: &str, tag: &str) -> Result<(), String Ok(()) } +/// Name prefix for the throwaway container that replays a compacted image's +/// config. Distinct from `triple-c-scrub-*` on purpose — see +/// [`restore_image_config`]. +const COMPACTION_CONTAINER_PREFIX: &str = "triple-c-compact-"; + +/// Remove any container left behind by an interrupted compaction. +/// +/// Runs at the start of a compaction rather than from a reclaim bucket, so +/// nothing can ever remove the container of a compaction that is still running: +/// by the time this is called, this task owns the compaction path. +async fn remove_stale_compaction_containers() { + let Ok(docker) = get_docker() else { + return; + }; + let containers = docker + .list_containers(Some(ListContainersOptions { + all: true, + size: false, + filters: HashMap::from([( + "name".to_string(), + vec![COMPACTION_CONTAINER_PREFIX.to_string()], + )]), + ..Default::default() + })) + .await + .unwrap_or_default(); + for summary in containers { + // Docker's `name` filter is a substring match; the full name decides. + if !is_compaction_container(&summary) { + continue; + } + if let Some(id) = summary.id.as_deref() { + match container::remove_container(id).await { + Ok(()) => log::info!("Removed stale compaction container {}", id), + Err(e) => log::warn!("Could not remove stale compaction container {}: {}", id, e), + } + } + } +} + +/// Whether a container is one of ours from an interrupted compaction. +fn is_compaction_container(summary: &ContainerSummary) -> bool { + summary + .names + .as_deref() + .unwrap_or(&[]) + .iter() + .any(|name| { + name.trim_start_matches('/') + .starts_with(COMPACTION_CONTAINER_PREFIX) + }) +} + /// Put a captured image config back onto a flattened image, under the original /// tag. /// @@ -2748,12 +2865,20 @@ async fn restore_image_config( use bollard::image::CommitContainerOptions; let docker = get_docker()?; - // Deliberately the same `triple-c-scrub-*` name `rewrite_image_without_secrets` - // uses. If this process dies between the create and the remove below, the - // leftover is already covered by the "secret-scrub scratch containers" - // bucket in this very panel rather than needing a second reaper. The two - // never run at once: `reclaim` executes its targets in sequence. - let scratch_name = format!("triple-c-scrub-{}", uuid::Uuid::new_v4().simple()); + + // **Its own prefix, not `triple-c-scrub-*`.** An earlier version reused the + // secret-rewrite name on the grounds that the existing reclaim bucket would + // then collect any leftover. It would — including the live one: that bucket + // removes with `force: true`, so a "remove scrub containers" reclaim fired + // from a second window while a compaction was mid-flight would destroy the + // container the commit is about to run against. Sequential execution inside + // one `reclaim` call is not a guarantee when two can be in flight. + // + // Stale ones are instead swept here, at the start of the next compaction — + // a created-but-never-started container has no writable layer, so a + // leftover costs almost nothing until then. + remove_stale_compaction_containers().await; + let scratch_name = format!("{}{}", COMPACTION_CONTAINER_PREFIX, uuid::Uuid::new_v4().simple()); // `image` is the flat build; everything else is copied from the original so // the committed image is byte-for-byte the same configuration. @@ -2877,7 +3002,8 @@ pub async fn clear_caches(project: &Project, include_rustup: bool) -> ReclaimRes match super::exec::exec_oneshot_as(&container_id, "claude", cmd, Vec::new()).await { Ok((output, _exit)) => match parse_cache_total(&output) { Some(bytes) => ReclaimResult { - target, + target: Some(target), + destroyed: None, ok: true, freed_bytes: bytes as i64, projected_bytes: None, @@ -2934,8 +3060,9 @@ pub async fn destroy( // A running container holds all three of these open, and Docker's refusal // is not something to lean on for the volumes: it would happily leave a // half-removed project behind. - if let Ok(Some(container_id)) = container::find_existing_container(project).await { - if container::is_container_running(&container_id) + let existing_container = container::find_existing_container(project).await.ok().flatten(); + if let Some(container_id) = existing_container.as_deref() { + if container::is_container_running(container_id) .await .unwrap_or(false) { @@ -2953,13 +3080,37 @@ pub async fn destroy( }; // Size it before it goes, so the report is a measurement. let bytes = volume_size(&name).await; + + // **A stopped container still pins its volumes.** Docker refuses + // `remove_volume` with a 409 while any container references one, + // and every project that has ever been started has exactly that — + // a stopped container is the resting state, not an edge case. So + // the container is removed first rather than letting the user type + // a project name and then meet a raw 409. It is regenerable from + // the snapshot; `DestructiveItem::loses` says so. + if let Some(container_id) = existing_container.as_deref() { + container::remove_container(container_id).await.map_err(|e| { + format!( + "Could not remove this project's container, which still holds the volume \ + open: {}. Nothing was removed.", + e + ) + })?; + log::info!( + "Removed container {} so {} could be deleted", + container_id, + name + ); + } + docker .remove_volume(&name, None) .await .map_err(|e| format!("Could not remove volume {}: {}", name, e))?; log::info!("Removed volume {} on explicit confirmation", name); Ok(ReclaimResult { - target: ReclaimTarget::OrphanVolume { name: name.clone() }, + target: None, + destroyed: Some(target.clone()), ok: true, freed_bytes: bytes, projected_bytes: None, @@ -2974,7 +3125,8 @@ pub async fn destroy( container::remove_snapshot_image(project).await?; let sweep = container::sweep_orphaned_snapshots().await; Ok(ReclaimResult { - target: ReclaimTarget::DanglingSnapshots, + target: None, + destroyed: Some(target.clone()), ok: true, freed_bytes: bytes + sweep.reclaimed_bytes, projected_bytes: None, @@ -2985,6 +3137,19 @@ pub async fn destroy( }) } DestructiveTarget::RollbackPin { tag, .. } => { + // **The one destructive variant carrying a free-form string.** + // Every other arm builds its target from constants; this one takes + // a tag over IPC and interpolates it into an image reference that + // is then removed. Unvalidated, `tag: "latest"` names the project's + // live snapshot — deleted under a dialog that says "rollback pin". + // `parse_rollback_tag` accepts only `pre-migration-`, + // which is exactly what `rollback_tag` produces and nothing else. + if migration::parse_rollback_tag(tag).is_none() { + return Err(format!( + "{:?} is not a rollback pin tag. Nothing was removed.", + tag + )); + } let reference = format!("triple-c-snapshot-{}:{}", project.id, tag); migration::untag_image(&reference).await?; // Untagging only makes the image dangling. Whatever came back came @@ -2992,7 +3157,8 @@ pub async fn destroy( let sweep = container::sweep_orphaned_snapshots().await; log::info!("Dropped rollback pin {} on explicit confirmation", reference); Ok(ReclaimResult { - target: ReclaimTarget::MigrationPins, + target: None, + destroyed: Some(target.clone()), ok: true, freed_bytes: sweep.reclaimed_bytes, projected_bytes: None, @@ -3011,8 +3177,13 @@ pub async fn destroy( /// The plain `Size` from `inspect_image` includes the base, which several other /// projects are still built from and which is not going anywhere. Reporting it /// as freed would overstate a snapshot removal by ~4.7 GB every time. Only -/// `df()` computes `SharedSize`, so this costs one — acceptable on the -/// destructive path, which handles exactly one object per call. +/// `df()` computes `SharedSize`, so each call costs a full daemon walk. +/// +/// That is three `df()`s on a compaction (before, after, and the scan that +/// planned it) and one per destructive removal. Acceptable because both are +/// single-object, user-initiated actions that already take seconds to minutes — +/// but it is why nothing in the *scan* path calls this: `scan` gets shared +/// sizes from the one `df()` it already makes. async fn image_unique_bytes(reference: &str) -> i64 { let Ok(docker) = get_docker() else { return 0; diff --git a/app/src-tauri/src/docker/disk_tests.rs b/app/src-tauri/src/docker/disk_tests.rs index 0aae0b1..8ec636a 100644 --- a/app/src-tauri/src/docker/disk_tests.rs +++ b/app/src-tauri/src/docker/disk_tests.rs @@ -338,6 +338,26 @@ fn a_scrub_container_is_matched_on_its_whole_name_not_a_substring() { assert!(!is_scrub_container(&summary(&[], &[]))); } +#[test] +fn a_compaction_container_is_never_matched_by_the_scrub_bucket() { + // These had the same `triple-c-scrub-*` prefix once. The scrub bucket + // removes with `force: true`, so a reclaim fired from a second window while + // a compaction was mid-flight would have destroyed the container the commit + // was about to run against. Separate prefixes, and neither predicate may + // reach the other's containers. + let compaction = summary(&["/triple-c-compact-abc123"], &[]); + let scrub = summary(&["/triple-c-scrub-abc123"], &[]); + + assert!(is_compaction_container(&compaction)); + assert!(!is_scrub_container(&compaction), "the scrub bucket must not reach it"); + + assert!(is_scrub_container(&scrub)); + assert!(!is_compaction_container(&scrub)); + + // Same substring-filter hazard applies to the new prefix. + assert!(!is_compaction_container(&summary(&["/my-triple-c-compact-notes"], &[]))); +} + #[test] fn a_probe_container_is_matched_on_its_label_not_on_the_daemons_filter() { // The `label=triple-c.probe=migration` filter is an exact match and would @@ -627,6 +647,24 @@ fn the_compaction_dockerfile_reuses_the_one_scrub_list() { assert!(!run_lines[0].contains('\n')); } +#[test] +fn the_compaction_build_is_labelled_so_the_sweep_can_collect_it() { + // Everything that cleans up after this build — the discard path when the + // result is not smaller, the untag after a successful commit — leans on + // `sweep_orphaned_snapshots`, and that sweep filters on `dangling=true` + // AND `triple-c.managed=true`. Without the label it can never match, and + // the flattened intermediate is stranded. + let df = compaction_dockerfile("x:latest", &container::snapshot_scrub_script()); + assert!( + df.contains("LABEL triple-c.managed=true"), + "the sweep filters on this label and would never match: {}", + df + ); + // It has to be on the *final* stage, not the discarded `src` one. + let after_scratch = df.split("FROM scratch").nth(1).expect("no final stage"); + assert!(after_scratch.contains("LABEL triple-c.managed=true"), "{}", df); +} + #[test] fn the_compaction_dockerfile_never_reaches_a_bind_mount() { let df = compaction_dockerfile("x:latest", &container::snapshot_scrub_script()); @@ -730,6 +768,77 @@ fn a_typed_confirmation_must_match_the_project_name_exactly() { assert!(!confirmation_matches("", "")); } +#[test] +fn only_a_real_rollback_tag_can_name_an_image_to_delete() { + // `DestructiveTarget::RollbackPin` is the one destructive variant carrying + // a free-form string from the frontend, and `destroy` interpolates it into + // an image reference it then removes. Unguarded, `tag: "latest"` names the + // project's *live snapshot* — deleted under a dialog that says "rollback + // pin". The guard is `parse_rollback_tag`, so this pins what it accepts. + assert!(migration::parse_rollback_tag("pre-migration-20260101-101500").is_some()); + + for hostile in [ + "latest", + "", + "pre-migration-", + "pre-migration-notatimestamp", + "../latest", + "latest\npre-migration-20260101-101500", + ] { + assert!( + migration::parse_rollback_tag(hostile).is_none(), + "{:?} must not be accepted as a rollback pin tag", + hostile + ); + } +} + +#[test] +fn a_destroy_result_never_claims_to_be_reclaim_work() { + // An earlier version returned `OrphanVolume { name }` for a home-volume + // deletion — naming a volume that was never an orphan, and attributing the + // outcome to a plan row the user never ticked. Exactly one of the two + // fields is ever set. + let reclaim_shaped = ReclaimResult { + target: Some(ReclaimTarget::DanglingSnapshots), + destroyed: None, + ok: true, + freed_bytes: 1, + projected_bytes: None, + message: String::new(), + }; + let destroy_shaped = ReclaimResult { + target: None, + destroyed: Some(DestructiveTarget::HomeVolume { + project_id: "p1".to_string(), + }), + ..reclaim_shaped.clone() + }; + assert!(reclaim_shaped.target.is_some() != reclaim_shaped.destroyed.is_some()); + assert!(destroy_shaped.target.is_some() != destroy_shaped.destroyed.is_some()); + + // And both shapes survive the wire. + let json = serde_json::to_string(&destroy_shaped).unwrap(); + assert_eq!(serde_json::from_str::(&json).unwrap(), destroy_shaped); +} + +#[test] +fn a_snapshot_with_no_known_base_is_not_offered_for_compaction() { + // With `triple-c.base-image-id` absent — the normal case for a project + // created before that label existed — `layer_stats` counts every layer that + // carries bytes, base included. A never-recreated project then reports ~15 + // "commit layers" and would sail past a `> 1` check. `base_lineage_known` + // is what stops the plan offering a rewrite sized from a number that does + // not mean what its name says. + let unknown = layer_stats(&[10, 20, 30, 40], None); + assert_eq!(unknown.commit_layers, 4); + assert_eq!(unknown.above_base_bytes, None, "the split must not be guessed"); + + let known = layer_stats(&[10, 20, 30, 40], Some(3)); + assert_eq!(known.commit_layers, 1); + assert_eq!(known.above_base_bytes, Some(10)); +} + // --------------------------------------------------------------------------- // Host detection // --------------------------------------------------------------------------- @@ -763,9 +872,14 @@ fn base_images_are_recognised_by_reference_for_display_only() { assert!(is_base_image_reference("triple-c-sandbox:latest")); assert!(is_base_image_reference("triple-c:latest")); + // A registry port must not be mistaken for a tag separator. + assert!(is_base_image_reference("localhost:5000/triple-c-sandbox:latest")); + assert!(is_base_image_reference("registry.example.com:8443/triple-c-sandbox")); + // A project's own snapshot is not a base image, and neither is anything of // the user's. assert!(!is_base_image_reference("triple-c-snapshot-abc:latest")); + assert!(!is_base_image_reference("localhost:5000/postgres:17")); assert!(!is_base_image_reference("triple-c-gateway:latest")); assert!(!is_base_image_reference("postgres:17-alpine")); } @@ -815,6 +929,7 @@ fn the_report_serialises_as_snake_case_like_every_other_ipc_struct() { }; let json = serde_json::to_value(&report).unwrap(); assert_eq!(json["projects"][0]["snapshot_commit_layers"], 14); + assert_eq!(json["projects"][0]["base_lineage_known"], false); assert_eq!(json["projects"][0]["container_writable_bytes"], 868_000_000i64); assert!(json["orphan_volumes_unavailable"].is_null()); // `Option` must reach the frontend as null, not be omitted — the TS diff --git a/app/src/components/settings/DiskProjectTable.tsx b/app/src/components/settings/DiskProjectTable.tsx index 891a200..0c9b9e3 100644 --- a/app/src/components/settings/DiskProjectTable.tsx +++ b/app/src/components/settings/DiskProjectTable.tsx @@ -11,6 +11,12 @@ interface Props { onDestroy: (item: DestructiveItem) => void; } +const LAYERS_HELP = + "Commit layers stacked above the base image — one for every time this project's container was recreated. Nothing merges them, so each one is paid for permanently until the snapshot is compacted."; + +const NEXT_COMMIT_HELP = + "The container's writable layer. This is exactly what the next recreation will stack onto the snapshot, and it never comes back after that."; + /** `—` for a column with nothing in it, so an empty cell never reads as zero. */ function cell(bytes: number, present: boolean) { return present ? formatBytes(bytes) : "—"; @@ -59,11 +65,18 @@ export default function DiskProjectTable({ rows, destructive, onDestroy }: Props Layers - + {/* `Tooltip` renders a portalled div with no `role` and no + `aria-describedby`, so its text reaches no assistive tech and + the trigger announces as "Help". These two headers are + meaningless without their explanation, so it is also emitted + as screen-reader-only text. */} + + — {LAYERS_HELP} Next commit adds - + + — {NEXT_COMMIT_HELP} Home vol @@ -104,7 +117,12 @@ export default function DiskProjectTable({ rows, destructive, onDestroy }: Props - {cell(row.snapshot_above_base_bytes ?? 0, row.snapshot_exists)} + {/* `null` means the split could not be measured. Rendering it + as 0 B would be the one guessed number in this table. */} + {cell( + row.snapshot_above_base_bytes ?? -1, + row.snapshot_exists && row.snapshot_above_base_bytes !== null, + )} {row.snapshot_exists && ( {/* The base is shared by every project, so charging it to @@ -117,18 +135,28 @@ export default function DiskProjectTable({ rows, destructive, onDestroy }: Props )} - {row.snapshot_exists ? ( - 5 - ? "text-[var(--warning)]" - : "text-[var(--text-primary)]" - } - > - {row.snapshot_commit_layers} - - ) : ( + {!row.snapshot_exists ? ( "—" + ) : !row.base_lineage_known ? ( + // The base this descends from is unknown, so the count + // includes the base's own layers and does not mean + // "recreations". Saying so beats printing a wrong number. + + unknown + + ) : ( + + {row.snapshot_commit_layers} + {/* Never colour alone: a count worth acting on says so in + a word, which is also what a screen reader gets. */} + {row.snapshot_commit_layers > 5 && ( + + stacked + + )} + )} diff --git a/app/src/components/settings/DiskSettings.test.tsx b/app/src/components/settings/DiskSettings.test.tsx index dad6168..07ec58d 100644 --- a/app/src/components/settings/DiskSettings.test.tsx +++ b/app/src/components/settings/DiskSettings.test.tsx @@ -35,6 +35,7 @@ const row = (over: Partial = {}): ProjectDiskRow => ({ snapshot_bytes: 12_273_392_374, snapshot_shared_bytes: 3_832_425_659, snapshot_commit_layers: 14, + base_lineage_known: true, snapshot_above_base_bytes: 8_440_966_715, container_exists: true, container_running: false, @@ -170,6 +171,36 @@ describe("DiskSettings", () => { expect(within(projectRow).getByText("14.6 GB")).toBeInTheDocument(); }); + it("refuses to present a layer count that does not mean recreations", async () => { + // Without `triple-c.base-image-id` — the normal case for a project created + // before that label existed — the count includes the base's own ~15 layers. + // Printing it beside a header that says "one per recreation" would be a + // wrong number in the column the table exists for. + getDockerDiskUsage.mockResolvedValue( + report({ projects: [row({ base_lineage_known: false, snapshot_commit_layers: 17 })] }), + ); + await renderAndScan(); + const projectRow = await screen.findByTestId("disk-row-p-whp"); + expect(within(projectRow).getByText("unknown")).toBeInTheDocument(); + expect(within(projectRow).queryByText("17")).not.toBeInTheDocument(); + }); + + it("renders an unmeasurable snapshot split as a dash, never as zero", async () => { + getDockerDiskUsage.mockResolvedValue( + report({ projects: [row({ snapshot_above_base_bytes: null })] }), + ); + await renderAndScan(); + const projectRow = await screen.findByTestId("disk-row-p-whp"); + expect(within(projectRow).queryByText("0 B")).not.toBeInTheDocument(); + expect(within(projectRow).getAllByText("—").length).toBeGreaterThan(0); + }); + + it("marks a heavily stacked snapshot with a word, not just a colour", async () => { + await renderAndScan(); + const projectRow = await screen.findByTestId("disk-row-p-whp"); + expect(within(projectRow).getByText("stacked")).toBeInTheDocument(); + }); + it("charges the shared base to the globals, not to every project row", async () => { // The base is one 4.7 GB image every project descends from. Counting it per // row would show it eight times and make the column meaningless. @@ -218,6 +249,45 @@ describe("DiskSettings", () => { expect(reclaim).toHaveBeenCalledWith([{ kind: "migration_staging" }]); }); + it("clears the tick list once the reclaim has run", async () => { + // The plan's rows describe objects the reclaim just removed; leaving them + // ticked lets the user fire the same call again against nothing. + await renderAndScan(); + await screen.findByTestId("disk-safe-bucket"); + fireEvent.click(screen.getAllByRole("checkbox")[0]); + expect(screen.getByText(/1 selected/)).toBeInTheDocument(); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Reclaim" })); + }); + expect(screen.queryByTestId("disk-safe-bucket")).not.toBeInTheDocument(); + expect(screen.queryByRole("checkbox")).not.toBeInTheDocument(); + // And it says why the list is gone rather than claiming nothing was found. + expect(screen.getByTestId("disk-plan-stale").textContent).toMatch( + /measured before that last action/, + ); + }); + + it("says why the build-cache figure is the under-reporting one", async () => { + // Without this, a `buildx du` failure silently shows `docker system df`'s + // number, which under-reports what a prune would free. + getDockerDiskUsage.mockResolvedValue( + report({ + build_cache: { + total_bytes: 28_000_000_000, + reclaimable_bytes: 1_000_000, + stale_bytes: 0, + source: "system df", + cli_error: "`docker buildx du` failed: executable not found", + }, + }), + ); + await renderAndScan(); + const globals = await screen.findByTestId("disk-globals"); + expect(globals.textContent).toMatch(/under-reports what a prune would free/); + expect(globals.textContent).toMatch(/executable not found/); + }); + it("cannot reclaim with nothing ticked", async () => { await renderAndScan(); await screen.findByTestId("disk-safe-bucket"); @@ -380,9 +450,7 @@ describe("DiskSettings", () => { ); await renderAndScan(); const note = await screen.findByTestId("disk-vhdx-note"); - expect( - within(note).getByText("Reclaiming here will not shrink your C: drive"), - ).toBeInTheDocument(); + expect(note.textContent).toMatch(/Warning: reclaiming here will not shrink your C: drive/); expect(within(note).getByText(/wsl --shutdown/)).toBeInTheDocument(); expect(within(note).getByText(/Optimize-VHD/)).toBeInTheDocument(); expect(within(note).getByText(/Purge data/)).toBeInTheDocument(); @@ -415,7 +483,8 @@ describe("DiskSettings", () => { }), ); destroyProjectDiskObject.mockResolvedValue({ - target: { kind: "orphan_volume", name: "triple-c-claude-config-p-whp" }, + target: null, + destroyed: { kind: "config_volume", project_id: "p-whp" }, ok: true, freed_bytes: 427_000_000, projected_bytes: null, @@ -450,6 +519,57 @@ describe("DiskSettings", () => { ); }); + it("keeps the confirmation open and busy while the deletion runs", async () => { + // The modal used to be unmounted before the call was awaited, which made + // its whole busy path dead code and left a multi-second volume removal with + // no indication it was happening. + listReclaimable.mockResolvedValue( + plan({ + destructive: [ + { + target: { kind: "home_volume", project_id: "p-whp" }, + project_id: "p-whp", + project_name: "whp", + label: "Home volume", + loses: "Shell history and toolchains.", + bytes: 4_860_000_000, + blocked: null, + }, + ], + }), + ); + let finish: (value: unknown) => void = () => {}; + destroyProjectDiskObject.mockReturnValue(new Promise((r) => (finish = r))); + + await renderAndScan(); + await screen.findByTestId("disk-row-p-whp"); + fireEvent.click(screen.getByRole("button", { name: "Delete whp data" })); + await act(async () => { + fireEvent.click(screen.getByRole("menuitem", { name: /Delete home volume/ })); + }); + + const dialog = screen.getByRole("dialog"); + fireEvent.change(within(dialog).getByLabelText(/Type/), { target: { value: "whp" } }); + fireEvent.click(within(dialog).getByRole("button", { name: "Delete home volume" })); + + // Still open, and saying so. + await waitFor(() => + expect(screen.getByRole("button", { name: "Working…" })).toBeDisabled(), + ); + + await act(async () => { + finish({ + target: null, + destroyed: { kind: "home_volume", project_id: "p-whp" }, + ok: true, + freed_bytes: 4_860_000_000, + projected_bytes: null, + message: "Removed volume.", + }); + }); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + }); + it("never routes a destructive object through the bulk Reclaim button", async () => { listReclaimable.mockResolvedValue( plan({ @@ -479,6 +599,7 @@ describe("DiskSettings", () => { results: [ { target: { kind: "compact_snapshot", project_id: "p-whp" }, + destroyed: null, ok: true, freed_bytes: 5_100_000_000, projected_bytes: 7_000_000_000, @@ -499,7 +620,7 @@ describe("DiskSettings", () => { expect(within(outcome).getByText(/projected up to 7\.0 GB, actually 5\.1 GB/)).toBeInTheDocument(); }); - it("surfaces a scan failure rather than showing stale numbers", async () => { + it("surfaces a scan failure as an alert", async () => { getDockerDiskUsage.mockRejectedValue("Could not read Docker disk usage: no such host"); render(); await act(async () => { diff --git a/app/src/components/settings/DiskSettings.tsx b/app/src/components/settings/DiskSettings.tsx index e3a3992..720fcfc 100644 --- a/app/src/components/settings/DiskSettings.tsx +++ b/app/src/components/settings/DiskSettings.tsx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useEffect, useState } from "react"; import Button from "../ui/Button"; import StatusIndicator, { type StatusTone } from "../ui/StatusIndicator"; import Modal from "../ui/Modal"; @@ -38,12 +38,29 @@ function targetKey(target: ReclaimTarget): string { * and the backend refuses it in bulk by taking a different type entirely. */ export default function DiskSettings() { - const { report, plan, scanning, working, error, outcome, scan, runReclaim, destroy } = - useDiskUsage(); + const { + report, + plan, + scanning, + working, + error, + outcome, + scan, + runReclaim, + destroy, + runSweep, + clearOutcome, + } = useDiskUsage(); const [ticked, setTicked] = useState>(new Set()); const [confirming, setConfirming] = useState(null); const [destroying, setDestroying] = useState(null); + // The plan is dropped after any reclaim, so a tick can never outlive the row + // it was made against and be re-fired at an object that is already gone. + useEffect(() => { + if (!plan) setTicked(new Set()); + }, [plan]); + const safeItems = plan?.items.filter((i) => i.safety === "safe") ?? []; const semiItems = plan?.items.filter((i) => i.safety === "semi_safe") ?? []; const selected = safeItems.filter( @@ -112,11 +129,14 @@ export default function DiskSettings() { className="border border-[var(--warning)]/40 bg-[var(--warning-muted)] rounded-[var(--radius-panel)] px-3.5 py-3 space-y-2" data-testid="disk-vhdx-note" > - + {/* `StatusIndicator` has no warning tone — `error` would put a + red glyph in a warning-toned panel. This is advisory, so it + carries its own glyph beside the words rather than relying on + the panel's colour. */} +

+ Warning: reclaiming here will not + shrink your C: drive +

{report.host.vhdx_note}

@@ -203,11 +223,20 @@ export default function DiskSettings() { )} + {report.build_cache.cli_error && ( +

+ {/* Without this the panel silently shows `docker system df`'s + under-reported build-cache figure and the user has no way + to know why it disagrees with their terminal. */} + Build-cache figures fell back to docker system df, which + under-reports what a prune would free: {report.build_cache.cli_error} +

+ )} {report.orphan_volumes.length > 0 && (

- That last figure means only that the volume’s project id is not in - your project list — it is not inferred from a project - being stopped or having no image. A project you have not opened in a + “Volumes with no matching project” above means only that the + volume’s project id is not in your project list — it is{" "} + not inferred from a project being stopped or having no image. A project you have not opened in a while has no container and no snapshot either, and that is normal, so each of these is ticked individually and shows the date Docker created it. @@ -223,7 +252,7 @@ export default function DiskSettings() { {/* --- Store failure, if any ------------------------------------ */} {report.orphan_volumes_unavailable && (

)} + {/* --- The plan was dropped by a reclaim -------------------------- */} + {!plan && ( +

+ The totals above were measured before that last action. Scan again to see + what is left to reclaim. +

+ )} + {/* --- Safe reclaim ---------------------------------------------- */} + {plan && (

Safe to reclaim @@ -260,7 +298,10 @@ export default function DiskSettings() {

+ )} {/* --- Semi-safe -------------------------------------------------- */} {semiItems.length > 0 && ( @@ -371,16 +413,13 @@ export default function DiskSettings() { {/* --- Sweep ------------------------------------------------------ */}
- - The same sweep that runs at startup and after every recreation — here you - can see what it found. + The same sweep that runs at startup and after every recreation. Unlike the + tick above it also reports what it refused to remove, which is how + a superseded image pinned by a stopped project shows itself.
@@ -394,11 +433,16 @@ export default function DiskSettings() { aria-live="polite" data-testid="disk-outcome" > - r.ok) ? "ok" : "error"} - label={`Reclaimed ${formatBytes(outcome.total_freed_bytes)}`} - className="text-xs" - /> +
+ r.ok) ? "ok" : "error"} + label={`Reclaimed ${formatBytes(outcome.total_freed_bytes)}`} + className="text-xs" + /> + +
    {outcome.results.map((result, index) => (
  • @@ -433,10 +477,11 @@ export default function DiskSettings() { size="md" variant="primary" disabled={working} - onClick={() => { - const target = confirming.target; + onClick={async () => { + // Same reasoning as the destructive modal: a compaction takes + // minutes, and the dialog reporting it beats it vanishing. + await runReclaim([confirming.target]); setConfirming(null); - void runReclaim([target]); }} > {working ? "Working…" : "Run it"} @@ -487,10 +532,12 @@ export default function DiskSettings() { confirmLabel={`Delete ${destroying.label.toLowerCase()}`} busy={working} onCancel={() => setDestroying(null)} - onConfirm={(typed) => { - const target = destroying.target; + onConfirm={async (typed) => { + // The modal stays mounted until the call settles, so its `busy` + // state is what the user sees while a multi-second volume removal + // runs. Clearing it first made the whole busy path dead code. + await destroy(destroying.target, typed); setDestroying(null); - void destroy(target, typed); }} >

    diff --git a/app/src/components/ui/TypedConfirmModal.tsx b/app/src/components/ui/TypedConfirmModal.tsx index 5a78278..8c7f036 100644 --- a/app/src/components/ui/TypedConfirmModal.tsx +++ b/app/src/components/ui/TypedConfirmModal.tsx @@ -1,4 +1,4 @@ -import { useRef, useState, type ReactNode } from "react"; +import { useId, useRef, useState, type ReactNode } from "react"; import Modal from "./Modal"; import Button from "./Button"; import { inputClass } from "./Field"; @@ -47,6 +47,9 @@ export default function TypedConfirmModal({ }: Props) { const [typed, setTyped] = useState(""); const inputRef = useRef(null); + // Every other `ui/` component uses `useId`; a hardcoded id breaks the + // label association as soon as two of these are mounted at once. + const inputId = useId(); const matches = expected.trim().length > 0 && typed.trim() === expected.trim(); return ( @@ -80,13 +83,13 @@ export default function TypedConfirmModal({ {children}

    setTyped(e.target.value)} diff --git a/app/src/hooks/useDiskUsage.test.tsx b/app/src/hooks/useDiskUsage.test.tsx index 0c21429..bda6e72 100644 --- a/app/src/hooks/useDiskUsage.test.tsx +++ b/app/src/hooks/useDiskUsage.test.tsx @@ -14,9 +14,11 @@ vi.mock("../lib/tauri-commands", () => ({ reclaim: (targets: unknown) => reclaim(targets), destroyProjectDiskObject: (target: unknown, confirmation: string) => destroyProjectDiskObject(target, confirmation), - sweepOrphanedSnapshots: vi.fn(), + sweepOrphanedSnapshots: () => sweepOrphanedSnapshots(), })); +const sweepOrphanedSnapshots = vi.fn(); + const report = (scanned_at: string): DiskUsageReport => ({ scanned_at, projects: [] }) as unknown as DiskUsageReport; @@ -149,13 +151,94 @@ describe("useDiskUsage", () => { expect(result.current.outcome?.total_freed_bytes).toBe(100); }); - it("surfaces a failure rather than leaving a stale report on screen", async () => { - getDockerDiskUsage.mockRejectedValue("daemon unreachable"); + it("reports a scan failure and keeps the last good measurement", async () => { + // The old report is still an accurate measurement of an earlier moment, + // and the error says the refresh failed. Blanking it would leave the panel + // with nothing while telling the user nothing more. + getDockerDiskUsage.mockResolvedValueOnce(report("first")); const { result } = renderHook(() => useDiskUsage()); await act(async () => { await result.current.scan(); }); + + getDockerDiskUsage.mockRejectedValueOnce("daemon unreachable"); + await act(async () => { + await result.current.scan(); + }); await waitFor(() => expect(result.current.error).toMatch(/daemon unreachable/)); + expect(result.current.report?.scanned_at).toBe("first"); expect(result.current.scanning).toBe(false); }); + + it("never shows fresh totals beside a stale tick list", async () => { + // `setReport` used to land before the plan call was awaited, so a plan + // failure rendered this scan's numbers above the previous scan's rows. + getDockerDiskUsage.mockResolvedValueOnce(report("first")); + const { result } = renderHook(() => useDiskUsage()); + await act(async () => { + await result.current.scan(); + }); + + getDockerDiskUsage.mockResolvedValueOnce(report("second")); + listReclaimable.mockRejectedValueOnce("planner exploded"); + await act(async () => { + await result.current.scan(); + }); + expect(result.current.error).toMatch(/planner exploded/); + expect(result.current.report?.scanned_at).toBe("first"); + }); + + it("drops the plan after a reclaim so ticks cannot be re-fired at nothing", async () => { + getDockerDiskUsage.mockResolvedValue(report("first")); + const { result } = renderHook(() => useDiskUsage()); + await act(async () => { + await result.current.scan(); + }); + expect(result.current.plan).toEqual(plan); + + await act(async () => { + await result.current.runReclaim([{ kind: "dangling_snapshots" }]); + }); + expect(result.current.plan).toBeNull(); + // The totals stay — they were measured before the reclaim and the outcome + // says what changed. + expect(result.current.report?.scanned_at).toBe("first"); + }); + + it("runs the sweep through its own command and reports what it refused", async () => { + // The sweep's `in_use` count — orphans Docker refused to delete because a + // stopped project still needs them — is invisible everywhere else in the + // app, because every other caller throws the report away. + sweepOrphanedSnapshots.mockResolvedValue({ + removed: ["sha256:a", "sha256:b"], + reclaimed_bytes: 11_900_000_000, + in_use: 3, + failed: [], + unavailable: null, + }); + const { result } = renderHook(() => useDiskUsage()); + await act(async () => { + await result.current.runSweep(); + }); + expect(sweepOrphanedSnapshots).toHaveBeenCalled(); + expect(result.current.outcome?.total_freed_bytes).toBe(11_900_000_000); + expect(result.current.outcome?.results[0].message).toMatch(/Swept 2 superseded image/); + expect(result.current.outcome?.results[0].message).toMatch(/3 were left alone/); + }); + + it("treats an unreachable daemon in the sweep report as an error", async () => { + sweepOrphanedSnapshots.mockResolvedValue({ + removed: [], + reclaimed_bytes: 0, + in_use: 0, + failed: [], + unavailable: "Could not reach the Docker engine", + }); + const { result } = renderHook(() => useDiskUsage()); + await act(async () => { + await result.current.runSweep(); + }); + expect(result.current.error).toMatch(/Could not reach the Docker engine/); + expect(result.current.outcome).toBeNull(); + }); }); diff --git a/app/src/hooks/useDiskUsage.ts b/app/src/hooks/useDiskUsage.ts index e3bf786..f82eb19 100644 --- a/app/src/hooks/useDiskUsage.ts +++ b/app/src/hooks/useDiskUsage.ts @@ -17,16 +17,24 @@ import type { * the daemon and computes shared-layer sizes. On a 100 GB store that is * seconds. `AccordionSection` unmounts its body when collapsed, so a * `useEffect` scan would re-run every single time the user opened the section. - * The scan is therefore a `scan()` the Scan button calls and nothing else, and - * the result lives in this hook rather than in the component so that reopening - * the section shows the last result instead of paying again. + * The scan is therefore only ever what the Scan button calls. + * + * Note what that does *not* buy: this hook lives inside `DiskSettings`, which + * the accordion unmounts on collapse, so its state goes with it and reopening + * the section shows an unscanned panel again. That is the honest behaviour — + * a stale total is worse than an absent one — but it means collapsing and + * reopening discards a scan the user paid for. Lifting the report into + * `appState` would fix that and is deliberately not done here: it would put a + * multi-megabyte, rapidly-stale blob into the app-wide store for one panel. * * ## The generation guard * * A user who hits Scan twice can have two `df()` calls in flight, and they can * land out of order — the second one is not necessarily slower. Every async - * write checks it is still the newest before it lands, the same pattern - * `useContainerMigration` uses. + * write in `scan` checks it is still the newest before it lands, the same + * pattern `useContainerMigration` uses. `runReclaim` and `destroy` do not need + * it: the UI disables their buttons while `working` is set, so there is never + * a second one to race. */ export interface DiskUsageState { report: DiskUsageReport | null; @@ -41,6 +49,8 @@ export interface DiskUsageState { scan: () => Promise; runReclaim: (targets: ReclaimTarget[]) => Promise; destroy: (target: DestructiveTarget, confirmation: string) => Promise; + /** Run the orphaned-snapshot sweep and report what it found *and refused*. */ + runSweep: () => Promise; clearOutcome: () => void; } @@ -63,16 +73,24 @@ export function useDiskUsage(): DiskUsageState { try { const next = await commands.getDockerDiskUsage(); if (generation.current !== mine) return; - setReport(next); // Planning is cheap and always wanted: the classification is what makes // the numbers actionable, and it reuses the report rather than scanning // again. const nextPlan = await commands.listReclaimable(next); if (generation.current !== mine) return; + // Both land together, or neither does. Setting the report before + // awaiting the plan would render this scan's totals above the *previous* + // scan's still-clickable tick list if the plan call failed. + setReport(next); setPlan(nextPlan); } catch (e) { if (generation.current !== mine) return; setError(String(e)); + // The old report is left on screen deliberately — it is still an + // accurate measurement of an earlier moment, and the error says the + // refresh failed. What must not survive is a plan describing a scan the + // user can no longer see the totals for, but that cannot happen: the two + // only ever move together. } finally { if (generation.current === mine) setScanning(false); } @@ -85,9 +103,16 @@ export function useDiskUsage(): DiskUsageState { try { const result = await commands.reclaim(targets); setOutcome(result); - // Deliberately no automatic re-scan. It costs another `df()`, and the + // **The plan is now stale and must not stay clickable.** Its rows + // describe objects this call just removed, so leaving them ticked lets + // the user fire the same reclaim again against nothing. Dropping the plan + // (not the report) leaves the totals on screen, marked as measured before + // the reclaim, with the tick list gone. + // + // Deliberately no automatic re-scan: it costs another `df()`, and the // outcome already reports measured bytes for every target — a user who // wants the new totals asks for them. + setPlan(null); } catch (e) { setError(String(e)); } finally { @@ -101,6 +126,53 @@ export function useDiskUsage(): DiskUsageState { try { const result = await commands.destroyProjectDiskObject(target, confirmation); setOutcome({ results: [result], total_freed_bytes: result.freed_bytes }); + // Same reasoning as `runReclaim`: the destructive list named an object + // that is now gone. + setPlan(null); + } catch (e) { + setError(String(e)); + } finally { + setWorking(false); + } + }, []); + + /** + * The startup sweep, on demand. + * + * Not the same as ticking "superseded snapshot layers", even though both end + * up removing the same images: this reports `in_use` — the orphans Docker + * *refused* to delete because a stopped project's container still needs + * them. That refusal is the sweep's third safety net and it is invisible + * everywhere else in the app, because every existing caller throws the + * report away. + */ + const runSweep = useCallback(async () => { + setWorking(true); + setError(null); + try { + const sweep = await commands.sweepOrphanedSnapshots(); + if (sweep.unavailable) { + setError(sweep.unavailable); + return; + } + const refused = + sweep.in_use > 0 + ? ` ${sweep.in_use} were left alone because a container is still built from them — start and stop, or recreate, that project and a later sweep gets them.` + : ""; + setOutcome({ + results: [ + { + target: { kind: "dangling_snapshots" }, + destroyed: null, + ok: sweep.failed.length === 0, + freed_bytes: sweep.reclaimed_bytes, + projected_bytes: null, + message: `Swept ${sweep.removed.length} superseded image(s).${refused}`, + }, + ], + total_freed_bytes: sweep.reclaimed_bytes, + }); + setPlan(null); } catch (e) { setError(String(e)); } finally { @@ -110,5 +182,17 @@ export function useDiskUsage(): DiskUsageState { const clearOutcome = useCallback(() => setOutcome(null), []); - return { report, plan, scanning, working, error, outcome, scan, runReclaim, destroy, clearOutcome }; + return { + report, + plan, + scanning, + working, + error, + outcome, + scan, + runReclaim, + destroy, + runSweep, + clearOutcome, + }; } diff --git a/app/src/lib/formatBytes.test.ts b/app/src/lib/formatBytes.test.ts index f832838..aabd3d1 100644 --- a/app/src/lib/formatBytes.test.ts +++ b/app/src/lib/formatBytes.test.ts @@ -45,6 +45,24 @@ describe("formatBytes", () => { expect(formatBytes(2_500_000_000_000)).toBe("2.5 TB"); }); + it("promotes the unit when rounding lands on a whole step", () => { + // `toFixed` runs after the divide loop, so a value just under a boundary + // rounds up into a unit the loop had already ruled out. This is the app's + // only byte formatter and the panel is full of near-boundary sizes. + expect(formatBytes(999_999)).toBe("1.0 MB"); + expect(formatBytes(999_999_999)).toBe("1.0 GB"); + expect(formatBytes(999_999_999_999)).toBe("1.0 TB"); + expect(formatBytes(1_048_575, { binary: true })).toBe("1.0 MB"); + + // Just below the rounding threshold it must NOT promote. + expect(formatBytes(999_949)).toBe("999.9 KB"); + expect(formatBytes(999_400, { precision: 0 })).toBe("999 KB"); + + // The top unit has nowhere to go: it renders a whole step rather than + // running off the end of the unit array. + expect(formatBytes(999_999_999_999_999_999)).toBe("1000.0 PB"); + }); + it("renders an em dash for a size the daemon did not compute", () => { // Docker reports -1 for "not calculated" on shared sizes and volume ref // counts. `NaN GB` in the middle of a table is worse than nothing. diff --git a/app/src/lib/formatBytes.ts b/app/src/lib/formatBytes.ts index 6c2b0a1..a66cb11 100644 --- a/app/src/lib/formatBytes.ts +++ b/app/src/lib/formatBytes.ts @@ -1,10 +1,14 @@ /** * The one byte formatter. * - * Before this existed the app had four of them — `projects/home/format.ts`, + * The app had four of them — `projects/home/format.ts`, * `projects/migrationCopy.ts`, `settings/UpdateDialog.tsx` and an inline * `toFixed(1)` in `useProjectActions.ts` — disagreeing about the divisor, the - * unit labels and the precision. They are now expressed in terms of this. + * unit labels and the precision. The first two now delegate here. + * + * The other two deliberately do not, yet: `UpdateDialog` renders KB at + * `toFixed(0)`, so re-pointing it would change what a download size reads as, + * and neither is on the Disk panel's path. They are the remaining copies. * * ## Why the default is base 1000 * @@ -14,8 +18,12 @@ * same build cache would read as a bug in the panel. So decimal is the default * and binary is opt-in, rather than the other way round. * - * Both existing conventions are preserved exactly, so re-pointing the old - * call sites changed no rendered string: + * Both existing conventions are preserved for every size either call site can + * realistically produce — a file size or a payload size, i.e. a non-negative + * finite number below a terabyte. Outside that range this deliberately differs + * from what it replaced: a negative or `NaN` input now renders `—` rather than + * `-1 B` or `NaN GB`, and the unit ladder continues past GB instead of + * stopping there. * * - `{ }` → `41.0 MB` (decimal, what migration used) * - `{ binary: true }` → `1.5 GB` (÷1024 with decimal-style @@ -56,6 +64,17 @@ export function formatBytes(bytes: number, options: FormatBytesOptions = {}): st value /= step; unit += 1; } + + // **Promote again if rounding pushed the value back up to a whole step.** + // `toFixed` runs after the loop, so 999,999 B divides to 999.999 KB and then + // renders as "1000.0 KB" — a unit the loop had already decided against. The + // same happens at every boundary (999,999,999 → "1000.0 MB", and 1,048,575 + // → "1024.0 KB" in binary). + if (unit < units.length - 1 && Number(value.toFixed(precision)) >= step) { + value /= step; + unit += 1; + } + // Whole bytes never get a decimal point: `512 B`, not `512.0 B`. return unit === 0 ? `${Math.round(bytes)} ${units[0]}` @@ -73,7 +92,7 @@ export function formatBytesDelta(bytes: number, options?: FormatBytesOptions): s } /** - * `up to 12.3 GB` / `nothing` — for a bound rather than a measurement. + * `up to 12.3 GB` — for a bound rather than a measurement. * * The Disk panel is careful about this distinction: every figure it shows is * measured except a compaction's yield, which cannot be known until it runs. diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index 5af5033..fa97cd5 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -836,8 +836,14 @@ export interface ProjectDiskRow { /** Bytes shared with another image — almost always the base. */ snapshot_shared_bytes: number; /** Layers stacked above the base image: **one per container recreation**. - * This is the number that explains why a snapshot grows. */ + * This is the number that explains why a snapshot grows — but only when + * `base_lineage_known` is true. Otherwise it counts the base's layers too. */ snapshot_commit_layers: number; + /** Whether the base image this snapshot descends from could be identified. + * False is the normal case for a project created before the + * `triple-c.base-image-id` label existed; the layer count must not be + * presented as a recreation count then. */ + base_lineage_known: boolean; /** Bytes those layers account for. `null` when the base image is gone and * the split cannot be measured — never a guess. */ snapshot_above_base_bytes: number | null; @@ -989,7 +995,11 @@ export interface ReclaimPlan { } export interface ReclaimResult { - target: ReclaimTarget; + /** The reclaim target this reports on, or `null` when it reports a destroy. + * Exactly one of `target` / `destroyed` is ever set — a destroy used to come + * back wearing a `ReclaimTarget` that named work it had not done. */ + target: ReclaimTarget | null; + destroyed: DestructiveTarget | null; ok: boolean; freed_bytes: number; /** What was projected beforehand, for the one action that projects. */