The four probes in get_container_staleness are now taken as Results and funnelled through a pure collect_probe_inputs, following the pick_probe_source / stopped_probe_policy shape already in that file, so the rule is unit-testable without touching Docker. The first error in call order becomes probe_error.
The command still returns Ok on a daemon fault, as the issue specified — useContainerMigration.ts:118-122 sets staleness to null in its catch and the banner returns early on null, so an Err would hide the fault rather than report it. Verified, not assumed.
One premise in the issue did not hold.is_container_running does not distinguish absent from unreachable: its body is match docker.inspect_container(...) { Ok(info) => ..., Err(_) => Ok(false) }, so every inspect failure including transport is already flattened to Ok(false), and the only Err it can produce is get_docker()? failing. The swallow is inside that function, not at the call site the issue points at.
I threaded its Result through anyway — the get_docker failure is a genuine daemon-unreachable signal, and this layer no longer adds a second swallow on top — and documented the remaining gap at the decision point rather than editing docker/container.rs, which the issue puts out of scope and whose doc comment states the swallow is deliberate ("Returns false if the container doesn't exist or Docker is unavailable"). Other callers may depend on that. Practical impact is small: find_existing_container runs immediately before and would already have errored if the daemon were down.
Worth a follow-up issue (deliberately not done here, per the issue's scope note): is_container_running's internal swallow, plus mig::image_created, mig::image_labels (returns an empty map on any failure, including get_docker) and container_label, which still swallow unconditionally. With this fix a daemon that is down at the start short-circuits before those run, so the common case no longer misreports — but a mid-call fault would still read as "lineage unknown".
No frontend change, as the issue predicted. ContainerMigrationBanner.tsx:140 computes probeUnavailable = !staleness.known && !!staleness.probe_error and renders "Some checks did not complete: …" at :210-213; known is false on this path, so the new message lands there.
Verification
Rust: 589 passed, 0 failed (3 new). Three tests cover each probe failing in turn, first-error-wins, and the all-ok path — including an assertion that the daemon message is never NOTHING_TO_PROBE, which is the confidently-wrong string the issue is about.
Not runtime-tested: reproducing it needs a daemon fault mid-probe.
Closes #56.
The four probes in `get_container_staleness` are now taken as `Result`s and funnelled through a pure `collect_probe_inputs`, following the `pick_probe_source` / `stopped_probe_policy` shape already in that file, so the rule is unit-testable without touching Docker. The first error in call order becomes `probe_error`.
The command still returns `Ok` on a daemon fault, as the issue specified — `useContainerMigration.ts:118-122` sets `staleness` to `null` in its `catch` and the banner returns early on null, so an `Err` would hide the fault rather than report it. Verified, not assumed.
**One premise in the issue did not hold.** `is_container_running` does *not* distinguish absent from unreachable: its body is `match docker.inspect_container(...) { Ok(info) => ..., Err(_) => Ok(false) }`, so every inspect failure including transport is already flattened to `Ok(false)`, and the only `Err` it can produce is `get_docker()?` failing. The swallow is inside that function, not at the call site the issue points at.
I threaded its `Result` through anyway — the `get_docker` failure is a genuine daemon-unreachable signal, and this layer no longer adds a second swallow on top — and documented the remaining gap at the decision point rather than editing `docker/container.rs`, which the issue puts out of scope and whose doc comment states the swallow is deliberate ("Returns false if the container doesn't exist or Docker is unavailable"). Other callers may depend on that. Practical impact is small: `find_existing_container` runs immediately before and would already have errored if the daemon were down.
**Worth a follow-up issue** (deliberately not done here, per the issue's scope note): `is_container_running`'s internal swallow, plus `mig::image_created`, `mig::image_labels` (returns an empty map on *any* failure, including `get_docker`) and `container_label`, which still swallow unconditionally. With this fix a daemon that is down at the start short-circuits before those run, so the common case no longer misreports — but a mid-call fault would still read as "lineage unknown".
**No frontend change**, as the issue predicted. `ContainerMigrationBanner.tsx:140` computes `probeUnavailable = !staleness.known && !!staleness.probe_error` and renders "Some checks did not complete: …" at :210-213; `known` is false on this path, so the new message lands there.
### Verification
Rust: **589 passed, 0 failed** (3 new). Three tests cover each probe failing in turn, first-error-wins, and the all-ok path — including an assertion that the daemon message is never `NOTHING_TO_PROBE`, which is the confidently-wrong string the issue is about.
Not runtime-tested: reproducing it needs a daemon fault mid-probe.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
`get_container_staleness` collected four probes through `unwrap_or`, so a
transient daemon fault landed on the same arm as a genuine absence and the
banner said, confidently and wrongly, that the project has no container or
snapshot image to compare against.
The four readings are now taken as `Result`s and funnelled through a pure
`collect_probe_inputs`, following `pick_probe_source` and
`stopped_probe_policy` in the same file, so the rule is unit-testable
without touching Docker. The first error in call order wins and becomes
`probe_error`; the command still returns `Ok`, because the hook's `catch`
sets `staleness` to null and the banner returns early on null -- an `Err`
here would hide the fault instead of reporting it.
One of the issue's premises did not hold. `is_container_running` does not
distinguish absent from unreachable: its body flattens every
`inspect_container` failure to `Ok(false)`, so only a `get_docker` failure
can surface as `Err`. Its `Result` is threaded through anyway, since that
one case is a real daemon-unreachable signal and this layer no longer adds
a second swallow on top, and the remaining gap is documented where the
decision is made rather than patched in `docker/container.rs`, which the
issue puts out of scope and whose doc comment says the swallow is
deliberate. In practice `find_existing_container` runs immediately before
and would already have errored if the daemon were down.
No frontend change: `probeUnavailable` in ContainerMigrationBanner already
routes a set `probe_error` to "Some checks did not complete".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of this branch found the first cut made every probe error fatal,
including one that is usually irrelevant. `snapshot_exists` is consulted
only when there is no container, or when a stopped container coincides with
a busy project -- `pick_probe_source` discards it outright for a running
one. So a daemon hiccup between the four sequential readings turned a full
report into a bare "could not be checked" with Update disabled, in a change
whose whole purpose is handling exactly that hiccup better.
It is now carried as a `Result` to the points that consult it and surfaced
only there. `stopped_probe_policy` carries its own message, because
"try again once it finishes" claims waiting is the only obstacle, which a
failed `image_exists` has not established.
`base_image_id` stays fatal, deliberately: it is the right-hand side of the
comparison, and `image_id` already distinguishes "not pulled locally"
(`Ok(None)`, a legitimate not-stale) from "could not ask". Letting an `Err`
through as `None` would report a project up to date on a reading nobody
got -- #56 one field over.
The message no longer blames the daemon. Three of the four callees can
`Err` from a daemon that answered perfectly: `image_id` maps only 404 to
`Ok(None)`, and the base image name is user-supplied, so a malformed
reference told the user to go fix a daemon that was running fine. That is
the same category of error as #56 itself.
`ContainerState` makes "running is known but no container was found"
unrepresentable rather than merely unreached, so the downstream match has
no impossible arm and the invariant is enforced where it is established.
Finally, the tests covered the new function but not the line the bug was
on: a partial revert to `.unwrap_or(None)` kept them all green. The
readings now travel as a named struct of `Result`s, so that revert is a
compile error -- verified by performing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two accuracy defects from re-review, both the same class as the bug this
branch exists to fix.
`probe_failed` rendered every failure as "This project's container could
not be inspected", but only two of the four readings are about the
container -- the others are the base image and the snapshot. A malformed
base image name in settings therefore pointed the user at the wrong object.
The sentence now names the check rather than the container.
The doc claimed "the first error wins, in call order". It does not: the
checks run container_id, base_image_id, container_running, while the daemon
is called in a different order entirely. The priority is deliberate -- it
puts the reading that stopped the probe first -- so the comment now says
that, instead of describing an order the code does not use.
The test guarding the first point asserted the message does not contain
"Docker", using a synthetic payload. The real bollard error for that case
is "Docker responded with status code 400: invalid reference format", so
the assertion passed only because the payload was invented. It now uses the
real shape and asserts what actually matters: that nothing we add claims
the daemon was unreachable or names the container.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jknapp
merged commit d647b56b43 into main2026-09-19 02:59:20 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #56.
The four probes in
get_container_stalenessare now taken asResults and funnelled through a purecollect_probe_inputs, following thepick_probe_source/stopped_probe_policyshape already in that file, so the rule is unit-testable without touching Docker. The first error in call order becomesprobe_error.The command still returns
Okon a daemon fault, as the issue specified —useContainerMigration.ts:118-122setsstalenesstonullin itscatchand the banner returns early on null, so anErrwould hide the fault rather than report it. Verified, not assumed.One premise in the issue did not hold.
is_container_runningdoes not distinguish absent from unreachable: its body ismatch docker.inspect_container(...) { Ok(info) => ..., Err(_) => Ok(false) }, so every inspect failure including transport is already flattened toOk(false), and the onlyErrit can produce isget_docker()?failing. The swallow is inside that function, not at the call site the issue points at.I threaded its
Resultthrough anyway — theget_dockerfailure is a genuine daemon-unreachable signal, and this layer no longer adds a second swallow on top — and documented the remaining gap at the decision point rather than editingdocker/container.rs, which the issue puts out of scope and whose doc comment states the swallow is deliberate ("Returns false if the container doesn't exist or Docker is unavailable"). Other callers may depend on that. Practical impact is small:find_existing_containerruns immediately before and would already have errored if the daemon were down.Worth a follow-up issue (deliberately not done here, per the issue's scope note):
is_container_running's internal swallow, plusmig::image_created,mig::image_labels(returns an empty map on any failure, includingget_docker) andcontainer_label, which still swallow unconditionally. With this fix a daemon that is down at the start short-circuits before those run, so the common case no longer misreports — but a mid-call fault would still read as "lineage unknown".No frontend change, as the issue predicted.
ContainerMigrationBanner.tsx:140computesprobeUnavailable = !staleness.known && !!staleness.probe_errorand renders "Some checks did not complete: …" at :210-213;knownis false on this path, so the new message lands there.Verification
Rust: 589 passed, 0 failed (3 new). Three tests cover each probe failing in turn, first-error-wins, and the all-ok path — including an assertion that the daemon message is never
NOTHING_TO_PROBE, which is the confidently-wrong string the issue is about.Not runtime-tested: reproducing it needs a daemon fault mid-probe.
🤖 Generated with Claude Code