fix(observe): a failed observation is not evidence of absence (audit theme 3) - #141
fix(observe): a failed observation is not evidence of absence (audit theme 3)#141passcod wants to merge 2 commits into
Conversation
Two places turned "we could not determine the state" into "the thing is gone", and everything downstream trusted the degraded answer. A failed observe returned an ObservedInstance with every flag false, which is byte-for-byte what confirmed-absent looks like. actuate_one_pod's Job terminal predicate (!container_exists && !is_running && previously_ran) then stopped the Job and recorded it in completed_jobs, from which job-terminal.defense guarantees it is killed again if it reappears — one transient podman or systemd hiccup permanently destroyed an in-flight batch workload. PodObservation splits the two, so the compiler forces every caller to route Failed explicitly; absence stays inside Observed, because a ContainerMissing from a successful query is a real observation. parse_container_status's catch-all swallowed podman's real transitional states, and Unknown was mapped to ContainerMissing in an arm that had already proven the container exists. A container draining through a long stop_timeout_secs was recorded as removed, so barriers sequenced after the stop released while it still held its volumes and network. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HtxQdsF6YhLzz9RrDmHEDv
Code Coverage OverviewLanguages: TypeScript, Rust TypeScript / code-coverage/vitestThe overall coverage in commit 8f40819 in the Rust / code-coverage/rustThe overall coverage in commit 8f40819 in the Show a code coverage summary of the most impacted files.
Updated |
|
CI red on Vitest, and it isn't this PR: the failure is Leaving it rather than papering over it here: tightening that assertion is a frontend change that doesn't belong in an observation-semantics PR, and it would hide the same flake on every other branch. Worth its own fix — the assertion wants Will re-check when CI re-runs. Generated by Claude Code |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HtxQdsF6YhLzz9RrDmHEDv
2 participants
Closes cross-cutting theme 3 from the logic bug audit: absence of observation conflated with observation of absence.
The class
The code had one vocabulary for "state of the world" and none for "quality of the evidence", so missing or unclassifiable evidence was coerced into the most destructive definite answer. Two instances:
Failed queries defaulted to all-absent.
observe_one_podhandled anobserver.observe()error by returning a fully actuatableObservedInstancewith every flag false — byte-for-byte what "confirmed absent" looks like.actuate_one_podthen ran its Job terminal predicate(!container_exists && !is_running && previously_ran)with no check on the failure, stopped the Job, and recorded it incompleted_jobs, from whichjob-terminal.defenseguarantees it is killed again if it ever reappears. One transient hiccup in any of the three probes permanently destroyed an in-flight batch workload.Unrecognised-but-present states mapped to "removed".
parse_container_status's catch-all swallowed podman's real transitional statesstopping,removingandinitializedintoUnknown, whichObserver::observemapped toContainerMissing— in an arm that had just proven the container exists, because the inspect returned it. That persists ascontainer_removed, which the oracle reads as the transition toUnscheduledandtermination_successreads as terminal success. A postgres draining through a longstop_timeout_secswas recorded as removed, and barriers sequenced after the stop were satisfied while the old container still held its volumes and network.The change
Uncertainty is resolved at the observer boundary and does not cross it disguised as fact.
PodObservationisObserved(..) | Failed { .. }, andactuate_one_podtakes anObservedInstance— so "failure looks like absence" is unrepresentable and the compiler forces every caller to routeFailedexplicitly. Failed instances skip actuation entirely: no start, no stop, no job-terminal detection, nocompleted_jobsinsertion. Theirobserve_failurestill reaches the fault path, and because they emit no facts, the oracle keeps the last state it derived. The observation stays atomic per instance — reporting the probes that did succeed would resurrect the bug in a subtler form (container_exists: falsebecause the inspect failed while the network probe succeeded).Note absence stays inside
Observed: aContainerMissingfrom a successful query is a real observation and must keep driving teardown and job-terminal logic.ContainerStatusgainsStopping, andparse_container_statusmodels podman's documented state set with no silent catch-all — an unrecognised string logs an error naming it.StoppingandUnknownboth map to a new in-tick-onlyContainerPresentIndeterminatefact that setscontainer_existsand persists nothing, so a draining container is present (it still holds its volumes) and the oracle is not told it went away. BecauseContainerStatusmatches carry no_arm, adding a variant breaks compilation at every consumer that must decide what it means — which is how this change found its own call sites.Finally, the
observe_failedfault is gated on 3 consecutive failures per instance, so a single blip logs an error without minting an operator-visible fault. The streak resets on the first success and lives in memory beside the reconciler's other per-tick state; losing it on restart is fine, because a restart re-observes anyway.Findings closed
parse_container_statuscatch-all mapsstopping/removing/initializedtoUnknownNo persistence change is needed:
world_observationsis an append-only log of definite transitions, and "unknown this tick" is correctly represented by writing nothing — which makes the oracle hold the last-known state for free. No newobs_kind, no migration.Enforcement
r[observe.failure-not-absence]inruntime.md— a failed observation yields no facts; no destructive actuation may be based on one; lifecycle derivation retains the last observed state; an attempt succeeds or fails in full; an observed-but-unrecognised state is evidence the thing exists; a single failure must not fault, a persistent one must.r[observe.deployment]'s state list is widened to name the indeterminate-present case.stopping/removingare not absence,initializedisCreated, an unmodelled state is present-but-indeterminate;ContainerPresentIndeterminatepersists no observation whileContainerMissingstill recordscontainer_removed.Not in scope
Wrong answers from successful queries — H4's
SubState-vs-Resultmis-read is a different bug. The genuine ambiguity of absence for--rmJobs, where "exited 0 and auto-removed" and "crashed and auto-removed" both surface ascontainer_removedand onlyunit_faileddisambiguates. And systemd'sCollectMode=inactive-or-failederasing evidence before the observer runs.The stub
Systemstill cannot fail a probe, which is exactly why the existing tests passed while production died; fault-injection hooks on the stubContainerRuntime/ProcessManagerwould let the reconcile-loop behaviour be tested end to end rather than at the unit level, and are worth adding next.