diff --git a/app/src-tauri/src/commands/migration_commands.rs b/app/src-tauri/src/commands/migration_commands.rs index 7b86388..f43e348 100644 --- a/app/src-tauri/src/commands/migration_commands.rs +++ b/app/src-tauri/src/commands/migration_commands.rs @@ -73,6 +73,25 @@ use crate::AppState; /// Report how far behind the current base image a project's container is, and /// what migrating it would actually carry across. /// +/// Choose the recorded lineage from the two places it can be written, most +/// authoritative first: the live container's label, then the snapshot image's. +/// +/// **An empty label is absence, not an answer.** `create_container` always +/// writes `triple-c.base-image-id`, even when the value is unknown — that is +/// deliberate, because Docker merges an image's labels into a container's and +/// an inherited value would otherwise ride a snapshot forever. The consequence +/// is that `Some("")` is the *common* reading from a container whose lineage +/// was never established, so treating it as an answer silently skips the +/// snapshot, which may well have recorded a real one. +fn pick_recorded_lineage( + from_container: Option, + from_snapshot: Option, +) -> Option { + from_container + .filter(|v| !v.is_empty()) + .or_else(|| from_snapshot.filter(|v| !v.is_empty())) +} + /// Read-only. Runs two filesystem probes (~3 s each) and is therefore meant to /// be called on demand, not polled. #[tauri::command] @@ -98,20 +117,24 @@ pub async fn get_container_staleness( // Lineage, most authoritative source first: the live container's label, // then the snapshot image's. Both are written by `create_container` and // propagated onto the snapshot by `docker commit`. + // Each source is filtered for emptiness *before* it is allowed to satisfy + // the lookup. `create_container` always writes this label, even when the + // value is unknown — deliberately, so an inherited image label cannot ride + // a snapshot forever — which means the container's copy is very often + // `Some("")`. Filtering only the final result let that empty string count + // as an answer and skip the snapshot entirely, so a snapshot that *did* + // record a lineage was never consulted and the project reported "unknown" + // with the information sitting one lookup away. let container_id = docker::find_existing_container(&project).await.unwrap_or(None); - let recorded = match &container_id { + let from_container = match &container_id { Some(id) => container_label(id, mig::LABEL_BASE_IMAGE_ID).await, None => None, - } - .or_else(|| None); - let recorded = match recorded { - Some(v) => Some(v), - None => mig::image_labels(&snapshot_image) - .await - .get(mig::LABEL_BASE_IMAGE_ID) - .cloned(), - } - .filter(|v| !v.is_empty()); + }; + let from_snapshot = mig::image_labels(&snapshot_image) + .await + .get(mig::LABEL_BASE_IMAGE_ID) + .cloned(); + let recorded = pick_recorded_lineage(from_container, from_snapshot); out.base_image_id = recorded.clone(); out.known = recorded.is_some(); @@ -1671,6 +1694,32 @@ fn summarize( mod tests { use super::*; + #[test] + fn an_empty_lineage_label_is_absence_and_falls_through_to_the_snapshot() { + let some = |s: &str| Some(s.to_string()); + + // The regression: the container always carries the label, so an + // unknown lineage reads as `Some("")`. Letting that satisfy the lookup + // skipped a snapshot that had recorded the real thing. + assert_eq!( + pick_recorded_lineage(some(""), some("sha256:base")), + some("sha256:base") + ); + + // Ordinary precedence still holds: the container wins when it has one. + assert_eq!( + pick_recorded_lineage(some("sha256:container"), some("sha256:snapshot")), + some("sha256:container") + ); + assert_eq!(pick_recorded_lineage(None, some("sha256:snap")), some("sha256:snap")); + + // Genuinely unknown stays unknown — "probe instead", never a lineage + // invented to make the comparison succeed. + assert_eq!(pick_recorded_lineage(None, None), None); + assert_eq!(pick_recorded_lineage(some(""), some("")), None); + assert_eq!(pick_recorded_lineage(some(""), None), None); + } + #[test] fn byte_sizes_read_the_way_a_disk_warning_should() { assert_eq!(human_bytes(512), "512 B"); diff --git a/app/src/components/projects/home/ContainerMigrationBanner.test.tsx b/app/src/components/projects/home/ContainerMigrationBanner.test.tsx index 9b328a5..d140053 100644 --- a/app/src/components/projects/home/ContainerMigrationBanner.test.tsx +++ b/app/src/components/projects/home/ContainerMigrationBanner.test.tsx @@ -127,6 +127,46 @@ describe("ContainerMigrationBanner", () => { expect(container).toBeEmptyDOMElement(); }); + it("speaks up when an unlabelled container could not be probed at all", () => { + // The probe is the only signal a container with no lineage label has. If + // it fails and the banner stays silent, that is indistinguishable from + // "up to date" — the exact reading that let an out-of-date project go + // unnoticed indefinitely. + renderBanner( + migration({ + staleness: { + ...FRESH, + known: false, + stale: false, + probe_error: "output exceeded the inspection limit", + }, + probeSettled: false, + }), + ); + expect( + screen.getByText(/Container base could not be checked/i), + ).toBeInTheDocument(); + expect( + screen.getByText(/output exceeded the inspection limit/i), + ).toBeInTheDocument(); + // And it must not pose as a finding about the container itself. + expect( + screen.queryByText(/Container is missing things/i), + ).not.toBeInTheDocument(); + }); + + it("stays quiet when a labelled container's probe fails but its lineage is current", () => { + // `known` means the version comparison already answered the question, so + // a failed probe is not grounds to raise anything. + const { container } = renderBanner( + migration({ + staleness: { ...FRESH, probe_error: "could not exec in the container" }, + probeSettled: false, + }), + ); + expect(container).toBeEmptyDOMElement(); + }); + it("disables the action and explains why while the container is running", () => { renderBanner(migration({ staleness: STALE }), false); expect( diff --git a/app/src/components/projects/home/ContainerMigrationBanner.tsx b/app/src/components/projects/home/ContainerMigrationBanner.tsx index e8dbae5..c359d35 100644 --- a/app/src/components/projects/home/ContainerMigrationBanner.tsx +++ b/app/src/components/projects/home/ContainerMigrationBanner.tsx @@ -131,7 +131,15 @@ export default function ContainerMigrationBanner({ const probeFoundGaps = !staleness.known && (staleness.missing_features.length > 0 || staleness.missing_paths.length > 0); - if (!staleness.stale && !probeFoundGaps) return null; + + // The probe is the *only* signal a container with no lineage label has, so + // when it fails there is nothing left to be quiet about. Staying silent here + // is indistinguishable from "everything is fine" — and it is the likeliest + // outcome for the oldest, largest projects, whose manifests are the ones apt + // to exceed the inspection limit. Say that the check did not run instead. + const probeUnavailable = !staleness.known && !!staleness.probe_error; + + if (!staleness.stale && !probeFoundGaps && !probeUnavailable) return null; const snapshot = formatSnapshotDate(staleness.snapshot_created_at); const features = joinFeatures(staleness.missing_features); @@ -139,16 +147,25 @@ export default function ContainerMigrationBanner({ return (
@@ -158,7 +175,9 @@ export default function ContainerMigrationBanner({ ? snapshot ? `Running on a saved image from ${snapshot}.` : "Running on a saved image older than the current base." - : "This container predates base-image tracking, so it was probed directly."} + : probeUnavailable + ? "This container predates base-image tracking, so probing it is the only way to tell whether it is behind — and that did not complete." + : "This container predates base-image tracking, so it was probed directly."}

{staleness.missing_features.length > 0 && (