fix: reconcile orphaned registry rows against live containers #440

Merged
didericis merged 6 commits from fix/registry-orphan-reconciliation into main 2026-07-21 00:05:04 -04:00

6 Commits

Author SHA1 Message Date
didericis-claude 28fcc3f2d2 test: cover inspect_container_network_ip parser directly
test / stage-firecracker-inputs (pull_request) Successful in 5s
test / unit (pull_request) Successful in 35s
test / integration-docker (pull_request) Successful in 9s
tracker-policy-pr / check-pr (pull_request) Successful in 8s
test / build-infra (pull_request) Successful in 3m48s
test / integration-firecracker (pull_request) Successful in 1m43s
test / coverage (pull_request) Successful in 1m53s
test / publish-infra (pull_request) Has been skipped
test / integration-docker (push) Successful in 11s
test / unit (push) Successful in 35s
test / stage-firecracker-inputs (push) Successful in 2s
Update Quality Badges / update-badges (push) Failing after 40s
lint / lint (push) Successful in 2m43s
test / build-infra (push) Successful in 3m25s
test / integration-firecracker (push) Successful in 1m45s
test / coverage (push) Successful in 1m50s
test / publish-infra (push) Successful in 2m7s
The None/empty-string distinction is the invariant that prevents
live_source_ips from treating a failed inspect as a legitimately
no-address container. Add TestInspectContainerNetworkIp to util tests
covering: IP found, CIDR stripping, no address yet, absent network
list, non-zero exit, malformed JSON, and unexpected JSON shape.
2026-07-21 03:49:19 +00:00
didericis-claude 571030b8e8 fix: require authoritative container snapshot before reconciling
test / stage-firecracker-inputs (pull_request) Successful in 5s
tracker-policy-pr / check-pr (pull_request) Successful in 12s
test / integration-docker (pull_request) Successful in 33s
test / unit (pull_request) Successful in 39s
lint / lint (push) Successful in 49s
test / build-infra (pull_request) Successful in 3m23s
test / integration-firecracker (pull_request) Successful in 1m42s
test / coverage (pull_request) Failing after 1m26s
test / publish-infra (pull_request) Has been skipped
A failed container list or a partial per-container inspect were
previously indistinguishable from a legitimately empty/partial live
set, so reconciliation could silently unregister healthy bottles.

- enumerate_active() now raises EnumerationError instead of returning []
  when `container list` fails
- Add inspect_container_network_ip() to util, which returns None on
  inspect failure (vs "" for "no DHCP address yet"), so live_source_ips
  can tell the two apart
- live_source_ips() raises EnumerationError on either failure mode;
  register_agent() catches it and skips reconciliation, same as
  OrchestratorClientError
- Update tests: rename test_empty_when_the_cli_fails to
  test_raises_when_the_cli_fails, update patching to the new function,
  add coverage for list failure, per-container inspect failure, and the
  launch-not-blocked path
2026-07-21 03:39:18 +00:00
didericis-codex 288b205a44 test(macos): isolate registration tests from container discovery
test / stage-firecracker-inputs (pull_request) Successful in 2s
test / integration-docker (pull_request) Successful in 6s
tracker-policy-pr / check-pr (pull_request) Successful in 5s
test / unit (pull_request) Successful in 30s
lint / lint (push) Successful in 42s
test / build-infra (pull_request) Successful in 3m18s
test / integration-firecracker (pull_request) Successful in 1m27s
test / coverage (pull_request) Successful in 1m51s
test / publish-infra (pull_request) Has been skipped
2026-07-21 03:27:16 +00:00
didericis 0c1d27b605 fix(egress): the unattributed message must name the token-mismatch cause too
`/resolve` fail-closes on a missing/ambiguous registry row *and* on a
request whose identity token doesn't match. The message named only the
first, so a bottle that was registered correctly but sent no token read as
"not registered" and sent the reader looking for a deregistered bottle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-21 03:27:16 +00:00
didericis 69361114d1 fix(egress): name the real fault when a deny-all is not an allowlist miss
An unattributed bottle, an unreachable orchestrator, and an unparseable
policy all become a deny-all Config, and a deny-all is indistinguishable
from "policy loaded, host not allowed" at the decision point — both are just
"no matching route". So every one of them reported `host X is not in the
bottle's egress.routes allowlist`, which reads as a config problem and sends
the operator hunting for a route that was never missing. Diagnosing a
bricked registration cost hours for exactly this reason.

Carry the structural reason on Config and prefer it in decide(). A genuine
allowlist miss — a policy that loaded and simply lacks the host — keeps the
original wording, so the message now tells the two cases apart.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-21 03:27:16 +00:00
didericis e4d53fd360 fix(orchestrator): reap registry rows whose bottle is no longer running
A registry row only ever left the registry two ways: an explicit
teardown_bottle (the launcher's cleanup callback) or the same-IP supersede
sweep in register(). Neither runs when the launching CLI dies hard, so the
row outlives its container.

That orphan is not inert. Source IPs are recycled by the backend's DHCP and
by_source_ip fail-closes on ambiguity, so a leftover row at a reused address
resolves no policy at all for the next bottle that lands there — and a
bottle with no policy denies every host, which surfaces to the agent as
"host X is not in the allowlist" for hosts that were never the problem.

Add reap_absent/reconcile and call it from the macOS launch path before
registering, so each launch self-heals the registry. Restores the invariant
the data plane needs: at most one active row per live address, and none for
a dead one. The second half matters as much as the first — when several rows
claim a *live* address the newest wins and the rest are swept, otherwise a
recycled address stays ambiguous, which is exactly the bricked state.

The host supplies the live set because the orchestrator runs inside the
infra container and cannot see the backend. A grace window exempts rows
younger than it, so reconciliation cannot race a bottle still coming up, and
a reconcile failure is logged rather than blocking an otherwise-fine launch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-21 03:27:16 +00:00