app/src-tauri/src/commands/migration_commands.rs

@@ -101,25 +101,56 @@ fn pick_recorded_lineage(
 /// snapshot to fall back on.
 const NOTHING_TO_PROBE: &str = "This project has no container or snapshot image yet, so there is nothing to compare against the base image.";
 
+/// The project's container, as the daemon reported it.
+///
+/// `running` lives *inside* `Present` because it is only ever read about a
+/// container that was found: `is_container_running` needs an id. Keeping the
+/// two in one variant makes "running, but no container" unrepresentable rather
+/// than merely unreached, which is what [`pick_probe_source`] relies on when it
+/// hands a container id to the container probe arms.
+#[derive(Debug, PartialEq, Eq)]
+enum ContainerState {
+    /// The project genuinely has no container — an answer, not a failure to
+    /// look.
+    Absent,
+    Present {
+        id: String,
+        running: bool,
+    },
+}
+
+impl ContainerState {
+    fn id(&self) -> Option<&str> {
+        match self {
+            ContainerState::Absent => None,
+            ContainerState::Present { id, .. } => Some(id),
+        }
+    }
+}
+
 /// Where [`get_container_staleness`] reads the project's *current* filesystem
 /// from, in descending order of how current the answer is.
+///
+/// The container variants carry the id they will be probed with, so that
+/// "there is a container to read" and "here is which one" cannot come apart
+/// downstream.
 #[derive(Debug, PartialEq, Eq)]
-enum ProbeSource {
+enum ProbeSource<'a> {
     /// `docker exec` into the live container. The only source that includes
     /// everything installed since the last commit *in this session*.
-    RunningContainer,
+    RunningContainer(&'a str),
     /// Commit the stopped container's writable layer to a throwaway image and
     /// probe that. Exactly as current as the container, which is what makes it
     /// preferable to the snapshot — see below.
-    StoppedContainer,
+    StoppedContainer(&'a str),
     /// A throwaway container from `triple-c-snapshot-<id>:latest`.
     Snapshot,
     /// Nothing to read: no container, no snapshot.
     Nothing,
 }
 
-/// Pick the probe source. `container_running` is `None` when the project has no
+/// Pick the probe source, or report the one reading this decision needed and
-/// container at all, `Some(false)` when it has a stopped one.
+/// did not get.
 ///
 /// **A stopped container outranks the snapshot.** The snapshot image is not a
 /// checkpoint — `commit_container_snapshot` runs only before a removal (a
@@ -133,12 +164,30 @@ enum ProbeSource {
 /// Getting this wrong is what made a stopped, never-recreated project report
 /// "no container or snapshot image yet" — with its container sitting right
 /// there — and left Update disabled on the projects that most needed it.
-fn pick_probe_source(container_running: Option<bool>, snapshot_exists: bool) -> ProbeSource {
+///
-    match (container_running, snapshot_exists) {
+/// **`snapshot_exists` is consulted only where it decides something.** When a
-        (Some(true), _) => ProbeSource::RunningContainer,
+/// container answered, the snapshot is not part of this decision at all, so a
-        (Some(false), _) => ProbeSource::StoppedContainer,
+/// failed `image_exists` is passed over rather than surfaced: destroying a
-        (None, true) => ProbeSource::Snapshot,
+/// report the running container could have supplied in full would be the same
-        (None, false) => ProbeSource::Nothing,
+/// mistake, in the other direction, as reading an unreachable daemon as an
+/// absent container. It is load-bearing only with no container at all, and
+/// there its failure *is* the answer this function cannot give.
+fn pick_probe_source<'a>(
+    container: &'a ContainerState,
+    snapshot_exists: &Result<bool, String>,
+) -> Result<ProbeSource<'a>, String> {
+    match container {
+        ContainerState::Present { id, running: true } => Ok(ProbeSource::RunningContainer(id)),
+        // The stopped path may still want the snapshot, but only as a fallback
+        // it can do without — see `stopped_probe_policy` and the commit-failure
+        // arm in `get_container_staleness`, which each handle an unreadable
+        // snapshot themselves.
+        ContainerState::Present { id, running: false } => Ok(ProbeSource::StoppedContainer(id)),
+        ContainerState::Absent => match snapshot_exists {
+            Ok(true) => Ok(ProbeSource::Snapshot),
+            Ok(false) => Ok(ProbeSource::Nothing),
+            Err(e) => Err(probe_failed(e)),
+        },
     }
 }
 
@@ -159,8 +208,8 @@ enum StoppedProbe {
     /// touches nothing, which is what makes it the right answer while another
     /// operation owns the container.
     SnapshotInstead,
-    /// Report rather than guess.
+    /// Report rather than guess, with the message to report.
-    Defer,
+    Defer(String),
 }
 
 /// Pick what to do about a stopped container.
@@ -173,46 +222,50 @@ enum StoppedProbe {
 /// container with a hard `?`, so a non-404 from a remove that raced this commit
 /// fails the whole Start with an opaque "Failed to remove container". Reading
 /// the claim costs nothing and takes that failure off the table.
-fn stopped_probe_policy(project_is_busy: bool, snapshot_exists: bool) -> StoppedProbe {
+///
+/// `snapshot_exists` matters only once the project is busy, because that is the
+/// only state in which the snapshot is the alternative to committing. An
+/// unreadable snapshot there leaves nothing to fall back *to*, so its error is
+/// what gets reported: "try again once it finishes" alone would be a claim that
+/// waiting is all that stands in the way, which a failed `image_exists` has not
+/// established.
+fn stopped_probe_policy(
+    project_is_busy: bool,
+    snapshot_exists: &Result<bool, String>,
+) -> StoppedProbe {
     match (project_is_busy, snapshot_exists) {
         (false, _) => StoppedProbe::Commit,
-        (true, true) => StoppedProbe::SnapshotInstead,
+        (true, Ok(true)) => StoppedProbe::SnapshotInstead,
-        (true, false) => StoppedProbe::Defer,
+        (true, Ok(false)) => StoppedProbe::Defer(PROJECT_BUSY.to_string()),
+        (true, Err(e)) => StoppedProbe::Defer(probe_failed(e)),
     }
 }
 
-/// Reported as `probe_error` when a probe input could not be read at all,
+/// Reported as `probe_error` when a probe input could not be read at all.
-/// because the Docker daemon did not answer.
 ///
-/// Deliberately distinct from [`NOTHING_TO_PROBE`]: an unreachable daemon is
+/// Deliberately distinct from [`NOTHING_TO_PROBE`]: a failed reading is not
-/// not evidence that the project has no container, and saying "no container or
+/// evidence that the project has no container, and saying "no container or
-/// snapshot image yet" on a transient socket fault was confidently wrong about
+/// snapshot image yet" on a transient fault was confidently wrong about a
-/// a project that may well have both. The underlying error is carried through
+/// project that may well have both.
-/// verbatim, because "Docker is not running" and "permission denied on
+///
-/// /var/run/docker.sock" call for different fixes from the user.
+/// Deliberately *neutral about the cause*, too. Only one of the four readings
-fn daemon_unreachable(e: &str) -> String {
+/// implies an unreachable daemon: `mig::image_id` maps a 404 to `Ok(None)` and
-    format!(
+/// returns `Err` for any other status, and `find_existing_container` /
-        "Docker could not be reached, so this project's container could not be inspected: {}",
+/// `image_exists` wrap every list failure the same way — all of which a daemon
-        e
+/// that answered perfectly well can produce. The base image name comes from
-    )
+/// user settings, so a malformed reference alone reaches here, and telling that
+/// user to go fix a running daemon would be the same unestablished claim about
+/// a cause that this whole probe path exists to stop making.
+///
+/// The underlying error is carried through verbatim, because "Docker is not
+/// running" and "permission denied on /var/run/docker.sock" call for different
+/// fixes from the user.
+fn probe_failed(e: &str) -> String {
+    format!("This project's container could not be inspected: {}", e)
 }
 
-/// The four daemon readings [`get_container_staleness`] needs before it can
+/// The four daemon readings [`get_container_staleness`] takes before it can
-/// choose a probe source, once each has been confirmed to be an *answer*.
+/// choose a probe source, each still carrying whether it is an answer.
-#[derive(Debug)]
-struct ProbeInputs {
-    /// The current base image's ID, or `None` when it is not pulled locally —
-    /// which [`mig::image_id`] reports as `Ok(None)`, not an error.
-    current_base_image_id: Option<String>,
-    /// The project's container, or `None` when it genuinely has none.
-    container_id: Option<String>,
-    /// `None` when there is no container to ask about; otherwise its state.
-    container_running: Option<bool>,
-    snapshot_exists: bool,
-}
-
-/// Turn the four collected daemon readings into probe inputs, or into the first
-/// daemon error among them.
 ///
 /// **Absence and unreachability are different answers, and only one of them is
 /// an answer.** All four callees already draw that line — `image_id` maps a 404
@@ -220,13 +273,80 @@ struct ProbeInputs {
 /// an empty filtered list — so a call site that writes `.unwrap_or(None)` /
 /// `.unwrap_or(false)` is not defaulting, it is *discarding a distinction the
 /// callee went to the trouble of making*. That is what let an unreachable
-/// daemon reach [`pick_probe_source`] as `(None, false)` and report
+/// daemon reach [`pick_probe_source`] as "no container, no snapshot" and report
 /// [`NOTHING_TO_PROBE`] — a confident claim about a project nothing had
 /// actually looked at.
 ///
-/// The first error wins, in call order, because they are all the same fault:
+/// The `Result` fields are the guard against that returning: the call site
-/// when the daemon is down, all four fail, and the user needs the reason once,
+/// hands over what the daemon said, unmodified, and an `.unwrap_or` there no
-/// not four times.
+/// longer type-checks.
+#[derive(Debug)]
+struct ProbeReadings {
+    /// `docker::find_existing_container`.
+    container_id: Result<Option<String>, String>,
+    /// `docker::is_container_running`, and `None` when there was no container
+    /// to ask about — not a swallowed error.
+    container_running: Option<Result<bool, String>>,
+    /// `mig::image_id` for the configured base image.
+    base_image_id: Result<Option<String>, String>,
+    /// `docker::image_exists` for the project's snapshot image.
+    snapshot_exists: Result<bool, String>,
+}
+
+/// The readings [`get_container_staleness`] carries past the point where a
+/// missing one would have stopped it.
+#[derive(Debug)]
+struct ProbeInputs {
+    /// The current base image's ID, or `None` when it is not pulled locally —
+    /// which [`mig::image_id`] reports as `Ok(None)`, not an error.
+    current_base_image_id: Option<String>,
+    container: ContainerState,
+    /// Still a `Result`, because whether it is load-bearing depends on the
+    /// container: see [`pick_probe_source`].
+    snapshot_exists: Result<bool, String>,
+}
+
+/// What [`get_container_staleness`] does next, once the readings are in.
+#[derive(Debug)]
+enum ProbeStart {
+    /// Go ahead, with these inputs.
+    Inputs(Box<ProbeInputs>),
+    /// Stop, and hand the user this report.
+    ///
+    /// **Reported, not returned.** The hook's `catch` sets `staleness` to
+    /// `null`, and `ContainerMigrationBanner` renders nothing at all for a null
+    /// staleness — so an `Err` out of the command would make the banner vanish
+    /// at exactly the moment it has something to say. A `probe_error` on an
+    /// otherwise-default report keeps it on screen, reading "Container base
+    /// could not be checked". Carrying a `ContainerStaleness` rather than an
+    /// error string is what keeps that decision here, where it is tested,
+    /// instead of in the `?` someone adds at the call site later.
+    Report(Box<ContainerStaleness>),
+}
+
+/// Decide whether the collected readings are enough to probe with.
+///
+/// Only the readings this decision actually rests on can stop it:
+///
+/// * `container_id` selects the probe source outright, so a failure to read it
+///   leaves nothing to choose between. Fatal.
+/// * `base_image_id` is fatal too, and deliberately so: it is the right-hand
+///   side of the staleness comparison, where `None` ("not pulled locally", an
+///   answer) and `Err` ("could not ask") both otherwise collapse into
+///   `stale: false`. Reporting a project as up to date because the base image
+///   could not be read is exactly the #56 mistake, one field over.
+/// * `container_running` is asked only about a container that was found, and
+///   decides between two live probe sources. Fatal when present.
+/// * `snapshot_exists` is *not* fatal here, because it is load-bearing in only
+///   two of the downstream states — no container at all, and a stopped
+///   container on a busy project. It travels as a `Result` so each of those can
+///   surface it, and the states that never consult it are not punished for it.
+///
+/// The first error wins, in call order, because when the daemon is unreachable
+/// they fail together and the user needs the reason once, not three times.
+/// `container_id` leads so that the reported error is most often the one that
+/// stopped the probe. (It is at most three, not four: `container_running` is
+/// only attempted when `container_id` answered with a container.)
 ///
 /// **A caveat this cannot fix here.** `docker::is_container_running` swallows
 /// `inspect_container` failures into `Ok(false)` itself and errors only when the
@@ -234,23 +354,45 @@ struct ProbeInputs {
 /// inspect still reads as "stopped" rather than as an error. That is a fix
 /// inside that function, not at this call site; threading its `Result` through
 /// at least stops *this* layer from adding a second swallow on top.
-fn collect_probe_inputs(
+fn start_probe(readings: ProbeReadings) -> ProbeStart {
-    base_image_id: Result<Option<String>, String>,
+    match collect_probe_inputs(readings) {
-    container_id: Result<Option<String>, String>,
+        Ok(inputs) => ProbeStart::Inputs(Box::new(inputs)),
-    // `None` when there was no container to ask about — not a swallowed error.
+        Err(e) => ProbeStart::Report(Box::new(ContainerStaleness {
-    container_running: Option<Result<bool, String>>,
+            probe_error: Some(e),
-    snapshot_exists: Result<bool, String>,
+            ..Default::default()
-) -> Result<ProbeInputs, String> {
+        })),
-    let current_base_image_id = base_image_id.map_err(|e| daemon_unreachable(&e))?;
+    }
-    let container_id = container_id.map_err(|e| daemon_unreachable(&e))?;
+}
-    let container_running = container_running
+
-        .transpose()
+fn collect_probe_inputs(readings: ProbeReadings) -> Result<ProbeInputs, String> {
-        .map_err(|e| daemon_unreachable(&e))?;
+    let ProbeReadings {
-    let snapshot_exists = snapshot_exists.map_err(|e| daemon_unreachable(&e))?;
-    Ok(ProbeInputs {
-        current_base_image_id,
         container_id,
         container_running,
+        base_image_id,
+        snapshot_exists,
+    } = readings;
+
+    let container_id = container_id.map_err(|e| probe_failed(&e))?;
+    let current_base_image_id = base_image_id.map_err(|e| probe_failed(&e))?;
+    let container_running = container_running
+        .transpose()
+        .map_err(|e| probe_failed(&e))?;
+
+    let container = match (container_id, container_running) {
+        (Some(id), Some(running)) => ContainerState::Present { id, running },
+        // No container: whatever `container_running` says is about nothing, and
+        // the caller only produces `None` here anyway.
+        (None, _) => ContainerState::Absent,
+        // A container was found but nobody asked whether it was running. The
+        // caller cannot produce this, and guessing "stopped" would cost a
+        // running project the only probe source that sees this session's
+        // installs — so say what happened instead.
+        (Some(_), None) => return Err(probe_failed("the container's state was not read")),
+    };
+
+    Ok(ProbeInputs {
+        current_base_image_id,
+        container,
         snapshot_exists,
     })
 }
@@ -289,39 +431,32 @@ pub async fn get_container_staleness(
 
     let mut out = ContainerStaleness::default();
 
-    // Every reading the daemon owes us, taken up front and kept as a `Result`
+    // Every reading the daemon owes us, taken up front and handed on exactly as
-    // so that "could not ask" stays distinguishable from "asked, and the answer
+    // it came back, so that "could not ask" stays distinguishable from "asked,
-    // is no" — see [`collect_probe_inputs`], which is where that distinction is
+    // and the answer is no". [`start_probe`] is where that distinction is acted
-    // acted on. Nothing between here and there may collapse one into the other.
+    // on; nothing between here and there may collapse one into the other, and
-    let base_image_id = mig::image_id(&base_image).await;
+    // the `Result` fields of [`ProbeReadings`] are what stop it being possible.
     let container_id_result = docker::find_existing_container(&project).await;
     let container_running_result = match &container_id_result {
         Ok(Some(id)) => Some(docker::is_container_running(id).await),
         // No container, or no usable reading of one: nothing to inspect, and
-        // the error below is the container lookup's, reported once.
+        // the container lookup's own error is what gets reported.
         _ => None,
     };
-    let snapshot_exists_result = docker::image_exists(&snapshot_image).await;
+    let readings = ProbeReadings {
-
+        container_id: container_id_result,
-    let inputs = match collect_probe_inputs(
+        container_running: container_running_result,
-        base_image_id,
+        base_image_id: mig::image_id(&base_image).await,
-        container_id_result,
+        snapshot_exists: docker::image_exists(&snapshot_image).await,
-        container_running_result,
-        snapshot_exists_result,
-    ) {
-        Ok(inputs) => inputs,
-        // **Reported, not returned.** The hook's `catch` sets `staleness` to
-        // `null`, and `ContainerMigrationBanner` renders nothing at all for a
-        // null staleness — so an `Err` here would make the banner vanish at
-        // exactly the moment it has something to say. A `probe_error` on an
-        // otherwise-default report keeps it on screen, reading "Container base
-        // could not be checked".
-        Err(e) => {
-            out.probe_error = Some(e);
-            return Ok(out);
-        }
     };
-    let container_id = inputs.container_id;
+
+    let inputs = match start_probe(readings) {
+        ProbeStart::Inputs(inputs) => *inputs,
+        // Reported, not returned — see [`ProbeStart::Report`].
+        ProbeStart::Report(report) => return Ok(*report),
+    };
+    let container = inputs.container;
+    let container_id = container.id();
 
     out.current_base_image_id = inputs.current_base_image_id;
     out.snapshot_created_at = mig::image_created(&snapshot_image).await;
@@ -337,7 +472,7 @@ pub async fn get_container_staleness(
     // 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 from_container = match &container_id {
+    let from_container = match container_id {
         Some(id) => container_label(id, mig::LABEL_BASE_IMAGE_ID).await,
         None => None,
     };
@@ -356,14 +491,19 @@ pub async fn get_container_staleness(
     };
 
     // ── Probes ───────────────────────────────────────────────────────────
-    let container_running = inputs.container_running;
+    let snapshot_exists = &inputs.snapshot_exists;
-    let snapshot_exists = inputs.snapshot_exists;
+    let source = match pick_probe_source(&container, snapshot_exists) {
-    let from_manifest = match (
+        Ok(source) => source,
-        pick_probe_source(container_running, snapshot_exists),
+        // The only reading this decision needed and did not get — see
-        &container_id,
+        // [`ProbeStart::Report`] for why this is a report and not an `Err`.
-    ) {
+        Err(e) => {
-        (ProbeSource::RunningContainer, Some(id)) => mig::manifest_from_container(id).await,
+            out.probe_error = Some(e);
-        (ProbeSource::StoppedContainer, Some(id)) => {
+            return Ok(out);
+        }
+    };
+    let from_manifest = match source {
+        ProbeSource::RunningContainer(id) => mig::manifest_from_container(id).await,
+        ProbeSource::StoppedContainer(id) => {
             let busy = crate::project_lock::held(&project_id).is_some();
             match stopped_probe_policy(busy, snapshot_exists) {
                 StoppedProbe::Commit => {
@@ -380,7 +520,12 @@ pub async fn get_container_staleness(
                         // layer; the snapshot probe allocates nothing) and a
                         // 409 from an operation that claimed the project after
                         // the check above.
-                        Err(e) if snapshot_exists => {
+                        // `Ok(true)` specifically: an `image_exists` that
+                        // failed has not established that there is anything to
+                        // fall back to, and probing a snapshot that may not
+                        // exist would replace the commit's real error with a
+                        // confusing one.
+                        Err(e) if matches!(snapshot_exists, Ok(true)) => {
                             log::warn!(
                                 "Probing the stopped container for project {} failed ({}) — \
                                  falling back to its snapshot image, which may lag it",
@@ -400,15 +545,16 @@ pub async fn get_container_staleness(
                     );
                     mig::manifest_from_image(&snapshot_image).await
                 }
-                StoppedProbe::Defer => Err(PROJECT_BUSY.to_string()),
+                StoppedProbe::Defer(message) => Err(message),
             }
         }
-        (ProbeSource::Snapshot, _) => mig::manifest_from_image(&snapshot_image).await,
+        ProbeSource::Snapshot => mig::manifest_from_image(&snapshot_image).await,
-        // `container_running` is `Some` exactly when `container_id` is, so the
+        // Reached only when there is genuinely neither a container nor a
-        // two arms above are the only ones those variants can reach. This arm
+        // snapshot: `ProbeSource` carries the container id in its container
-        // is `ProbeSource::Nothing` — and now *only* that: it used to also
+        // variants, so a container that exists can no longer fall through to
-        // swallow every stopped container, which is the bug.
+        // here — which is the bug this arm used to hide, swallowing every
-        (_, _) => Err(NOTHING_TO_PROBE.to_string()),
+        // stopped container.
+        ProbeSource::Nothing => Err(NOTHING_TO_PROBE.to_string()),
     };
 
     let (from_manifest, base_manifest) = match from_manifest {
@@ -2218,13 +2364,39 @@ mod tests {
         assert_eq!(pick_recorded_lineage(some(""), None), None);
     }
 
+    /// The readings as the daemon answered them, all four healthy: no
+    /// container, nothing pulled, no snapshot. Tests override the one reading
+    /// they are about, which keeps it obvious which reading each case is
+    /// actually exercising.
+    fn readings() -> ProbeReadings {
+        ProbeReadings {
+            container_id: Ok(None),
+            container_running: None,
+            base_image_id: Ok(None),
+            snapshot_exists: Ok(false),
+        }
+    }
+
+    fn present(running: bool) -> ContainerState {
+        ContainerState::Present {
+            id: "c1".to_string(),
+            running,
+        }
+    }
+
+    /// What every one of the four readings looks like when the socket is gone:
+    /// generic over what it was going to return.
+    fn daemon<T>() -> Result<T, String> {
+        Err("Failed to list containers: connection refused".to_string())
+    }
+
     #[test]
     fn a_stopped_container_is_probed_rather_than_reported_missing() {
         // The regression: a container that exists but is stopped, with no
         // snapshot ever taken, read as "nothing to compare against".
         assert_eq!(
-            pick_probe_source(Some(false), false),
+            pick_probe_source(&present(false), &Ok(false)),
-            ProbeSource::StoppedContainer
+            Ok(ProbeSource::StoppedContainer("c1"))
         );
     }
 
@@ -2233,127 +2405,276 @@ mod tests {
         // The snapshot lags the container by everything installed since the
         // last commit, in both states.
         assert_eq!(
-            pick_probe_source(Some(true), true),
+            pick_probe_source(&present(true), &Ok(true)),
-            ProbeSource::RunningContainer
+            Ok(ProbeSource::RunningContainer("c1"))
         );
         assert_eq!(
-            pick_probe_source(Some(false), true),
+            pick_probe_source(&present(false), &Ok(true)),
-            ProbeSource::StoppedContainer
+            Ok(ProbeSource::StoppedContainer("c1"))
         );
     }
 
     #[test]
     fn the_snapshot_is_the_fallback_only_once_the_container_is_gone() {
-        assert_eq!(pick_probe_source(None, true), ProbeSource::Snapshot);
+        assert_eq!(
+            pick_probe_source(&ContainerState::Absent, &Ok(true)),
+            Ok(ProbeSource::Snapshot)
+        );
     }
 
     #[test]
     fn nothing_to_probe_is_reserved_for_no_container_and_no_snapshot() {
         // The one case the "no container or snapshot image yet" message may
         // still describe.
-        assert_eq!(pick_probe_source(None, false), ProbeSource::Nothing);
+        assert_eq!(
+            pick_probe_source(&ContainerState::Absent, &Ok(false)),
+            Ok(ProbeSource::Nothing)
+        );
+    }
+
+    #[test]
+    fn an_unreadable_snapshot_only_costs_the_report_where_the_snapshot_is_the_answer() {
+        // A container answered, so `image_exists` decides nothing: its failure
+        // must not cost a report the container can supply in full. Treating it
+        // as fatal turned "running container, one flaky `image_exists`" into a
+        // bare probe_error with Update disabled.
+        assert_eq!(
+            pick_probe_source(&present(true), &daemon()),
+            Ok(ProbeSource::RunningContainer("c1"))
+        );
+        assert_eq!(
+            pick_probe_source(&present(false), &daemon()),
+            Ok(ProbeSource::StoppedContainer("c1"))
+        );
+
+        // With no container, the snapshot is the whole decision, so its failure
+        // is reported — and never as "no container or snapshot image yet",
+        // which nothing has established.
+        let e = pick_probe_source(&ContainerState::Absent, &daemon()).unwrap_err();
+        assert!(e.contains("connection refused"), "{}", e);
+        assert_ne!(e, NOTHING_TO_PROBE);
     }
 
     #[test]
     fn a_stopped_container_is_committed_only_when_nothing_else_owns_the_project() {
-        assert_eq!(stopped_probe_policy(false, false), StoppedProbe::Commit);
+        assert_eq!(
-        assert_eq!(stopped_probe_policy(false, true), StoppedProbe::Commit);
+            stopped_probe_policy(false, &Ok(false)),
+            StoppedProbe::Commit
+        );
+        assert_eq!(stopped_probe_policy(false, &Ok(true)), StoppedProbe::Commit);
+        // Not the snapshot's business either way when the project is free: an
+        // unreadable `image_exists` does not stop the commit that would not
+        // have consulted it.
+        assert_eq!(stopped_probe_policy(false, &daemon()), StoppedProbe::Commit);
     }
 
     #[test]
     fn a_busy_project_falls_back_rather_than_racing_a_recreate() {
         // The snapshot lags, but a stale answer beats failing someone's Start.
         assert_eq!(
-            stopped_probe_policy(true, true),
+            stopped_probe_policy(true, &Ok(true)),
             StoppedProbe::SnapshotInstead
         );
         // Nothing to fall back to: say so instead of committing anyway.
-        assert_eq!(stopped_probe_policy(true, false), StoppedProbe::Defer);
+        assert_eq!(
+            stopped_probe_policy(true, &Ok(false)),
+            StoppedProbe::Defer(PROJECT_BUSY.to_string())
+        );
+
+        // Busy *and* the fallback could not be read: "try again once it
+        // finishes" would promise that waiting is all that stands in the way,
+        // which the failed reading has not established. Report what happened.
+        match stopped_probe_policy(true, &daemon()) {
+            StoppedProbe::Defer(message) => {
+                assert!(message.contains("connection refused"), "{}", message);
+                assert_ne!(message, PROJECT_BUSY);
+            }
+            other => panic!("expected Defer, got {:?}", other),
+        }
     }
 
     #[test]
     fn an_unreachable_daemon_is_never_read_as_an_absent_container() {
         // The bug: every one of these used to be flattened to "no" by an
-        // `unwrap_or`, which reached `pick_probe_source` as (None, false) and
+        // `unwrap_or`, which reached `pick_probe_source` as "no container, no
-        // reported "no container or snapshot image yet" about a project nobody
+        // snapshot" and reported "no container or snapshot image yet" about a
-        // had managed to look at.
+        // project nobody had managed to look at.
-        // Generic over the reading that failed: when the socket is gone, every
+        let e = collect_probe_inputs(ProbeReadings {
-        // one of the four fails the same way, whatever it was going to return.
+            container_id: daemon(),
-        fn daemon<T>() -> Result<T, String> {
+            ..readings()
-            Err("Failed to list containers: connection refused".to_string())
+        })
-        }
+        .unwrap_err();
+        assert!(e.contains("connection refused"), "{}", e);
+        assert_ne!(e, NOTHING_TO_PROBE);
 
-        let e = collect_probe_inputs(daemon(), Ok(None), None, Ok(false)).unwrap_err();
+        let e = collect_probe_inputs(ProbeReadings {
-        assert!(e.starts_with("Docker could not be reached"), "{}", e);
+            base_image_id: daemon(),
+            ..readings()
+        })
+        .unwrap_err();
         assert!(e.contains("connection refused"), "{}", e);
 
-        let e = collect_probe_inputs(Ok(None), daemon(), None, Ok(false)).unwrap_err();
+        let e = collect_probe_inputs(ProbeReadings {
+            container_id: Ok(Some("c1".into())),
+            container_running: Some(daemon()),
+            ..readings()
+        })
+        .unwrap_err();
         assert!(e.contains("connection refused"), "{}", e);
 
-        let e =
+        // The fourth reading is not fatal here — see
-            collect_probe_inputs(Ok(None), Ok(Some("c1".into())), Some(daemon()), Ok(false))
+        // `an_unreadable_snapshot_only_costs_the_report_where_the_snapshot_is_the_answer`
-                .unwrap_err();
+        // — but it must still arrive as an error rather than as "no snapshot".
-        assert!(e.contains("connection refused"), "{}", e);
+        let inputs = collect_probe_inputs(ProbeReadings {
+            snapshot_exists: daemon(),
+            ..readings()
+        })
+        .unwrap();
+        assert!(inputs.snapshot_exists.is_err());
+        let e = pick_probe_source(&inputs.container, &inputs.snapshot_exists).unwrap_err();
+        assert_ne!(e, NOTHING_TO_PROBE);
+    }
 
-        let e = collect_probe_inputs(Ok(None), Ok(None), None, daemon()).unwrap_err();
+    #[test]
-        assert!(e.contains("connection refused"), "{}", e);
+    fn a_base_image_that_could_not_be_read_is_never_reported_as_up_to_date() {
+        // `image_id` answers `Ok(None)` for "not pulled locally", which is a
+        // legitimate `stale: false`. An `Err` is not: it is the right-hand side
+        // of the comparison missing, and letting it through as `None` would
+        // report the project up to date on the strength of a reading nobody
+        // got. This is #56 one field over, so it is fatal on purpose.
+        let e = collect_probe_inputs(ProbeReadings {
+            base_image_id: Err("invalid reference format".into()),
+            container_id: Ok(Some("c1".into())),
+            container_running: Some(Ok(true)),
+            snapshot_exists: Ok(true),
+        })
+        .unwrap_err();
+        assert!(e.contains("invalid reference format"), "{}", e);
+    }
 
-        // And never the message that is only true of a project with neither.
+    #[test]
-        assert_ne!(
+    fn a_failed_reading_is_not_blamed_on_a_daemon_that_answered() {
-            collect_probe_inputs(Ok(None), daemon(), None, Ok(false)).unwrap_err(),
+        // Three of the four readings return `Err` from a daemon that replied
-            NOTHING_TO_PROBE
+        // perfectly well: `image_id` maps only a 404 to `Ok(None)`, and the two
-        );
+        // list-based readings wrap any failure. The base image name is
+        // user-supplied, so a typo in settings lands here — and used to be
+        // reported as "Docker could not be reached", sending the user to fix a
+        // daemon that was running.
+        let e = collect_probe_inputs(ProbeReadings {
+            base_image_id: Err("invalid reference format".into()),
+            ..readings()
+        })
+        .unwrap_err();
+        assert!(!e.contains("could not be reached"), "{}", e);
+        assert!(!e.contains("Docker"), "{}", e);
+        // The cause still comes through verbatim: "Docker isn't running" and
+        // "permission denied on the socket" need different fixes and must stay
+        // distinguishable.
+        assert!(e.contains("invalid reference format"), "{}", e);
     }
 
     #[test]
     fn the_first_daemon_error_is_the_one_reported() {
-        // When the daemon is down all four fail for the same reason, and the
+        // When the daemon is down these fail together, and the user needs the
-        // user needs that reason once rather than four times. Call order wins.
+        // reason once rather than three times. Call order wins, and
-        let e = collect_probe_inputs(
+        // `container_id` leads because it is what selects the probe source.
-            Err("first".into()),
+        //
-            Err("second".into()),
+        // At most three fail, not four: `container_running` is only attempted
-            Some(Err("third".into())),
+        // when `container_id` answered with a container, so the caller cannot
-            Err("fourth".into()),
+        // produce an `Err` container id alongside a `Some(..)` running reading.
-        )
+        let e = collect_probe_inputs(ProbeReadings {
+            container_id: Err("first".into()),
+            container_running: None,
+            base_image_id: Err("second".into()),
+            snapshot_exists: Err("third".into()),
+        })
         .unwrap_err();
         assert!(e.ends_with("first"), "{}", e);
 
-        let e = collect_probe_inputs(
+        let e = collect_probe_inputs(ProbeReadings {
-            Ok(None),
+            container_id: Ok(Some("c1".into())),
-            Err("second".into()),
+            container_running: Some(Err("third".into())),
-            Some(Err("third".into())),
+            base_image_id: Err("second".into()),
-            Err("fourth".into()),
+            snapshot_exists: Err("fourth".into()),
-        )
+        })
         .unwrap_err();
         assert!(e.ends_with("second"), "{}", e);
     }
 
+    #[test]
+    fn a_container_id_cannot_arrive_without_a_reading_of_its_state() {
+        // `ContainerState` makes "running, but no container" unrepresentable;
+        // this is the other half — a container found, but never asked about.
+        // The caller cannot produce it, and guessing "stopped" would cost a
+        // running project the only probe source that sees this session's
+        // installs.
+        let e = collect_probe_inputs(ProbeReadings {
+            container_id: Ok(Some("c1".into())),
+            container_running: None,
+            ..readings()
+        })
+        .unwrap_err();
+        assert!(e.contains("state was not read"), "{}", e);
+    }
+
     #[test]
     fn a_daemon_that_answers_no_is_an_answer_and_passes_through() {
-        // No container, no snapshot, base image not pulled: all four are `Ok`,
+        // No container, no snapshot, base image not pulled: all the readings
-        // and the "nothing to probe" path downstream is then genuinely earned.
+        // are `Ok`, and the "nothing to probe" path downstream is then
-        let inputs = collect_probe_inputs(Ok(None), Ok(None), None, Ok(false)).unwrap();
+        // genuinely earned.
+        let inputs = collect_probe_inputs(readings()).unwrap();
         assert_eq!(inputs.current_base_image_id, None);
-        assert_eq!(inputs.container_id, None);
+        assert_eq!(inputs.container, ContainerState::Absent);
-        assert_eq!(inputs.container_running, None);
+        assert_eq!(inputs.snapshot_exists, Ok(false));
-        assert!(!inputs.snapshot_exists);
         assert_eq!(
-            pick_probe_source(inputs.container_running, inputs.snapshot_exists),
+            pick_probe_source(&inputs.container, &inputs.snapshot_exists),
-            ProbeSource::Nothing
+            Ok(ProbeSource::Nothing)
         );
 
         // And the fully populated reading survives intact.
-        let inputs = collect_probe_inputs(
+        let inputs = collect_probe_inputs(ProbeReadings {
-            Ok(Some("sha256:base".into())),
+            container_id: Ok(Some("c1".into())),
-            Ok(Some("c1".into())),
+            container_running: Some(Ok(true)),
-            Some(Ok(true)),
+            base_image_id: Ok(Some("sha256:base".into())),
-            Ok(true),
+            snapshot_exists: Ok(true),
-        )
+        })
         .unwrap();
         assert_eq!(inputs.current_base_image_id.as_deref(), Some("sha256:base"));
-        assert_eq!(inputs.container_id.as_deref(), Some("c1"));
+        assert_eq!(inputs.container, present(true));
-        assert_eq!(inputs.container_running, Some(true));
+        assert_eq!(inputs.snapshot_exists, Ok(true));
-        assert!(inputs.snapshot_exists);
+    }
+
+    #[test]
+    fn a_failed_reading_keeps_the_banner_on_screen_instead_of_erroring() {
+        // The load-bearing design decision of this path: a failed reading is a
+        // report with `probe_error` set, never an `Err` out of the command. An
+        // `Err` reaches the hook's `catch`, which nulls `staleness`, and
+        // `ContainerMigrationBanner` renders nothing at all for a null one — so
+        // the banner would vanish at exactly the moment it has something to say.
+        match start_probe(ProbeReadings {
+            container_id: daemon(),
+            ..readings()
+        }) {
+            ProbeStart::Report(report) => {
+                let message = report.probe_error.clone().expect("probe_error");
+                assert!(message.contains("connection refused"), "{}", message);
+                // Everything else at its default: a field being empty means
+                // "nothing found", and nothing was found because nothing was
+                // read. `stale: false` here is the absence of a claim, which is
+                // only honest because `probe_error` is carrying the reason.
+                assert_eq!(
+                    *report,
+                    ContainerStaleness {
+                        probe_error: Some(message),
+                        ..Default::default()
+                    }
+                );
+            }
+            ProbeStart::Inputs(_) => panic!("a failed reading must not be probed on"),
+        }
+
+        // And a healthy set of readings still goes on to probe.
+        assert!(matches!(start_probe(readings()), ProbeStart::Inputs(_)));
     }
 
     #[test]