feat(canary): fail loud when the collector is blind on a provably live fleet (#1813)

[obasilakis]

Summary

• Adds H-01, the canary's first self-check: it fires when the SQL roster read returns zero rows — or raises — while an independent, non-SQL source proves the fleet is alive.
• Closes the failure shape Canary invariant harness is blind on PostgreSQL — reads stale SQLite /data/trinity.db #1540 left behind. Canary invariant harness is blind on PostgreSQL — reads stale SQLite /data/trinity.db #1540 repointed the collectors onto the configured engine; it did not change the fact that zero rows is indistinguishable from a clean fleet, so a re-blinded collector still reports all-clear.
• New H- (harness health) id family — every other invariant means the system is broken; H-01 means the observer is blind, and its violation invalidates every other green in that cycle.

Why this wasn't already covered

Verified against the real collect_snapshot() over a diverged backend — a two-agent fleet with a running execution produces:

known_agents = set()   sources_unavailable = []   violations = {}

A silent all-clear while completely blind. Prior partial coverage was worse than none: L-03 fires in this state only when an execution happens to hold a Redis slot (an idle fleet holds none), and when it does it reports agent_name 'a1' ... absent from agent_ownership — sending the on-call after a delete-cascade ghost-agent bug instead of a blind collector.

Design

roster read	independent evidence	outcome
non-empty	—	pass, clear marker
raised	—	roster_read_failed · critical
empty	Docker proves agents exist	roster_empty_contradicted · critical
empty	Redis only	roster_empty_unverifiable · major
empty	source unavailable / never read	roster_empty_unverifiable · major
empty	available, agrees empty	pass, clear marker

Every firing arm goes through the confirmation gate, roster_read_failed included — see below.

Evidence is deliberately non-circular. Snapshot gains docker_agent_names, read from the container list before any exec_run — not reused from zombie_counts, which is keyed by exec success and silently thins on a degraded container.

The Docker collector runs BEFORE the roster read (it never depended on it). collect_snapshot returns early when the roster read raises, so collecting Docker below it left the roster_read_failed arm — the most severe one — with no evidence at all.

Docker and Redis are not interchangeable for severity. orphan_redis_slots is by definition slot keys whose agent is absent from agent_ownership — the leaked-slot state L-03 exists to report. A genuinely empty fleet holding one leaked key would otherwise page critical claiming the harness is blind, over a correct roster and an unrelated Redis leak. Docker evidence is required for critical; Redis-only lands on roster_empty_unverifiable. The names still ride in evidence_sample — demoted, not discarded.

docker_available / redis_available are tri-state, backed by Snapshot.collectors_ran: True ran-and-fine, False ran-and-failed, None never ran. sources_unavailable records nothing on success and nothing when a collector is skipped, so a boolean structurally cannot tell those apart — which is how the roster_read_failed arm came to render docker=up · redis=up … 0 vs 0 agent(s) on a cycle where neither source had been consulted. None renders as not read.

Confirmation is on elapsed wall-clock, not "a second cycle." Prod runs uvicorn --workers 2 and canary_service holds only a per-process asyncio.Lock with no leader lease, so two loops share the marker — a cycle-count rule would let worker B confirm worker A seconds later, collapsing the gate to nothing inside the container-teardown window it exists to ride out. (This was caught in /review; captured to learnings.md.)

Fail-loud, never fail-silent. An unreadable or unwritable marker fires unconfirmed rather than skipping — a guard that cannot self-check must say so, or it reproduces the silent-green failure it exists to prevent.

The gate covers every arm, including roster_read_failed. That arm has no delete race to ride out, but a raised roster read is very often a momentary DB blip (connection reset, PG restart, pool exhaustion), and paging critical on one of those is how a safety net gets muted.

The marker carries a 24h TTL, refreshed on every suspicious cycle. _clear_marker is best-effort and a run-cycle filtered to other invariant_ids never reaches it, so without an expiry an orphaned marker stays armed forever and the next genuine episode confirms on its first cycle. Refreshing makes it an idle timeout rather than an absolute lifetime, so a long episode cannot silently re-arm and re-alert.

A whole-database outage now reaches the check at all. _run_cycle_inner read db.get_latest_canary_violation_per_invariant() before collecting the snapshot, unguarded — so a DB-down cycle raised out of the loop and H-01, whose entire job is to announce that the harness cannot see the fleet, never executed. That read is now fail-open. On its own that would trade silence for spam (an empty previous_latest makes every violation look like a fresh flip, so a multi-hour outage would alert every 5 minutes), so transition detection falls back to canary:last_cycle_red — a Redis-held record of the previous cycle's red set, in a separate failure domain from the DB.

Scoped to the roster read alone. On a live-but-quiet fleet terminal_rows, enabled_schedules, orphan_refs and terminal_exec_statuses are all legitimately empty, so a general "any SQL collector reads zero" rule would false-alarm on every idle install (AC #3).

Acceptance criteria

• Zero rows on a non-empty fleet emits a loud signal, not a green pass
• "Provably non-empty" derives from an independent live source (Docker presence ∪ Redis slots) — never the SQL tier under test
• A genuinely clean and genuinely empty fleet does not false-alarm
• Works on SQLite and PostgreSQL (feat(db): adopt Alembic for PostgreSQL migrations (retire SQLite-only bespoke runner) #1183) — pure function over the Snapshot, plus a new real-PG firing test
• Test simulating a re-blinded collector asserts the loud path fires

Changes

File	What
canary/invariants/h01_collector_blindness.py	new — the invariant
canary/snapshot.py	docker_agent_names field + populate from the container list pre-exec
canary/invariants/__init__.py	registry entry
services/canary_alerts.py	H-01 name, runbook, and a _render_message branch (it carries no agent_name, so the generic fallback would be useless for the one alarm whose job is legibility)
tests/test_canary_invariants.py	22 tests; also fixes FakeRedis.delete, which never handled string keys (real DEL is type-agnostic)
docs	architecture.md catalog row + H- family note · requirements/infrastructure.md §31 Phase 5 · invariant catalog §15 · learnings.md

No schema change, no migration, no new endpoint, no config, no feature flag.

Test plan

• tests/test_canary_invariants.py — 159 passed, randomized and ordered
• tests/unit — 5525 passed, exit 0 (4 hypothesis files excluded; package absent locally, pre-existing)
• Real PostgreSQL, both arms — including test_pg_h01_fires_on_a_blind_roster, which reproduces the Canary invariant harness is blind on PostgreSQL — reads stale SQLite /data/trinity.db #1540 shape on psycopg2 and asserts every other invariant is vacuously green there while H-01 fires critical
• Mutation check — replacing the guard with return [] (the exact silent-green failure mode) fails 14 tests, including the PG firing test. A guard never shown to fire is decorative

Known residuals (documented, not covered)

• An entirely stopped fleet has no running containers and holds no Redis slots, so no evidence exists — H-01 can only reach roster_empty_unverifiable.
• Partial blindness (roster returns 1 of 20) is out of scope: a count comparison would false-fire on legitimate create/stop races between the two reads.

Pre-existing gaps found, deliberately not fixed here

• The canary has no cross-worker leader lease at all, so under --workers 2 both loops run every cycle — double-probing the fleet (including R-01's docker exec into every agent) and double-writing E-02's Redis state. Out of blast radius; worth its own issue.
• Phase-4 invariants E-03/E-04/E-06/G-03/G-04 have no _INVARIANT_NAMES/_INVARIANT_RUNBOOKS entries and alert as "E-03 fired 1 violation(s)". Noted in requirements/infrastructure.md.

Note for reviewers

The reason-code strings (roster_read_failed / roster_empty_contradicted / roster_empty_unverifiable), the H-01 id, and the H- family are my choice — not specified by the issue. abilityai/trinity-enterprise#202 (benchmark scorer, @vybe) will score against canary output, and its AC requires "any unavailable data source fails the score loud rather than scoring green" — so these names are effectively a contract with it. Worth confirming before they harden.

Origin: PR #1690's "Follow-ups (for Andrii to file)" — the silent re-blinding operator alarm — building directly on @AndriiPasternak31's #1540 fix.

Fixes #1813

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[dolho]

Review — H-01 collector blindness

Strong direction, and the framing is right: a detector that goes quiet is worse than no detector, and the H- family separation from G- is the correct call. The reason-code strings look fine to harden as a contract with trinity-enterprise#202.

Four things before this lands, one of them substantive.

1. The roster_read_failed arm is mostly unreachable, and its payload misreports the evidence sources

Two separate problems, same arm.

(a) The snapshot early-returns before the evidence collectors run. snapshot.py:

try:
    agent_rows = _collect_known_agents()
except Exception as exc:
    snap.sources_unavailable.append(f"sqlite.agent_ownership: {exc}")
    return snap          # <-- docker + redis collectors never run

So on the one path where roster_failed is True, docker_agent_names and orphan_redis_slots are always empty and sources_unavailable carries no docker/redis entry. _violation then reports docker_available: True, redis_available: True, evidence_agent_count: 0, and the Slack forensic block renders:

*Sources:* docker=up · redis=up
*Roster vs evidence:* 0 vs 0 agent(s)

For the one alarm whose stated job is legibility, that reads as "everything else is fine and the fleet is empty" when in fact neither source was consulted. Suggest a tri-state — docker_available: None / "not read" when the collector never ran (a collectors_ran set on the Snapshot, or just move the Docker collect above the roster read since it has no dependency on it).

(b) A whole-DB outage never reaches the check at all. canary_service._run_cycle_inner does an unguarded DB read before collecting the snapshot:

previous_latest = db.get_latest_canary_violation_per_invariant()   # ~line 195
...
snapshot = await asyncio.to_thread(collect_snapshot)

If the database is down, that raises, the loop logs canary cycle raised; will retry next interval, and H-01 is never executed. So roster_read_failed covers only a failure confined to the agent_ownership query, not the DB-down case the docstring's "raised → definitive → critical" row implies. Either wrap that read (fail-open to {} so the cycle still runs and H-01 can fire), or say plainly in the docstring which failures this arm can and cannot see. As written the module promises coverage it does not have — the same shape of problem it exists to fix.

2. Redis-only evidence can page critical on an L-03 condition

evidence = set(snapshot.docker_agent_names) | set(snapshot.orphan_redis_slots)
...
elif evidence:
    reason, severity = REASON_CONTRADICTED, SEVERITY_CONFIRMED

orphan_redis_slots is by definition slot keys whose agent is absent from agent_ownership — i.e. the leaked-slot state L-03 exists to report. On a genuinely empty fleet holding one leaked slot key, Docker evidence is empty and H-01 fires roster_empty_contradicted / critical: a correct roster, an unrelated Redis leak, and a critical page saying the harness is blind.

The docstring already says Redis is "corroborating evidence, never the sole basis for the fleet is empty" — but the code makes it a sufficient basis for the contradiction, which is the direction that pages. Suggest: Docker evidence required for SEVERITY_CONFIRMED; redis-only → roster_empty_unverifiable / major.

3. Doc/code mismatch on the confirmation gate

The PR table says a raised roster read is "not needed — definitive", but every arm falls through to the CONFIRMATION_MIN_SECONDS marker gate, so roster_read_failed also waits 60s. Either exempt it (it has no delete-race to ride out — the race only exists for the empty-roster case) or correct the table.

4. Marker has no TTL and clears best-effort

_clear_marker swallows failures and the key has no expiry, so a persistently failing DEL leaves it armed forever — and POST /api/canary/run-cycle with an invariant_ids filter that excludes H-01 means nothing clears it either. A TTL of a few cycles bounds the staleness without weakening the gate (the marker is rewritten on every suspicious cycle anyway).

Verified fine

• sqlite.agent_ownership prefix matches the collector's literal label.
• slot_service client is decode_responses=True, so _to_utc(blind_since) gets a str — no bytes/TypeError hole.
• All four #1880 alert surfaces present (name, runbook, _render_message, _render_forensic), so the parity test will pass.
• Recording names before exec_run is the right call and the reason is worth the comment it got.

Merge order

This conflicts with #2022 (both rewrite _collect_zombie_counts's return dict and the Snapshot zombie fields) and with #1997 (canary_alerts). Worth deciding an order across the three and rebasing rather than resolving three-way at merge time.

[obasilakis]

All four findings addressed in 36428fc8. PR description table and design notes updated to match.

1a — the roster_read_failed arm misreporting its evidence

Both halves were right, and they wanted different fixes.

(a) Took your second suggestion for the evidence and the first for the reporting, because neither alone is enough. Docker now collects before the roster read — it never depended on it, and collecting it below a call that early-returns left the most severe arm with no evidence at all. So roster_read_failed now carries real container names. Redis cannot move with it (_collect_redis_slot_state takes known_agents), so Snapshot.collectors_ran makes availability tri-state and it honestly reports None → rendered not read.

Worth stating why the tri-state is structural rather than cosmetic: sources_unavailable records nothing on success and nothing when a collector is skipped. Those two states are byte-identical, so no amount of care at the read site can distinguish them — the collector has to say it ran. The forensic block that used to print docker=up · redis=up · 0 vs 0 agent(s) now prints docker=up · redis=not read · 2 vs 2 agent(s).

(b) Wrapped the read. But fail-open alone would have swapped one defect for another: with previous_latest empty, _is_green_to_red returns True for everything, so a multi-hour DB outage would alert every 5 minutes — breaking the "a persistent condition chirps once" property the module leans on. Added canary:last_cycle_red, a Redis-held record of the previous cycle's red set, as the fallback authority. Redis is a separate failure domain from the DB, so it can still remember what was red while the DB cannot. Written on every cycle (a fallback is only useful if it predates the outage), TTL'd at 12 intervals, and None (Redis also down) keeps the existing verbose-on-failure policy.

Also corrected the docstring's residuals section: what remains genuinely uncoverable is a failure that stops the process running the cycle at all, which external liveness monitoring owns.

2 — Redis-only evidence

Agreed, and the docstring was the tell: it said "corroborating evidence, never the sole basis" while the code made it a sufficient basis for the branch that pages. Docker evidence is now required for SEVERITY_CONFIRMED; Redis-only lands on roster_empty_unverifiable / major. Redis names still ride in evidence_sample — demoted, not discarded, since the operator still wants to see them.

3 — the confirmation gate

Kept the gate, corrected the table. The gate earns its keep on that arm for a different transient than the delete race: a raised roster read is very often a momentary DB blip (connection reset, PG restart, brief pool exhaustion), and paging critical on one of those is precisely how a safety net gets muted. Cost is 60s of latency on a real outage, which a persistent outage is unaffected by. Docstring, PR table, architecture.md, the catalog and infrastructure.md all say this now.

4 — marker TTL

24h, set via ex= on the write (one round-trip, and it can't be forgotten on a future write path) and refreshed on every suspicious cycle. The refresh matters: a bare TTL would be an absolute lifetime, so an episode outliving it would silently re-arm, go green for one cycle and re-alert on a condition that never changed. As an idle timeout it bounds the orphaned-marker staleness you identified without touching the live case.

Tests

26 new ones in tests/unit/test_1813_h01_collector_blindness.py rather than beside the existing H-01 suite — tests/test_canary_invariants.py is executed by no workflow (filed as #2037 off your review of #2022), so a guard placed there would never go red. Each pins a finding and fails against the pre-review code. The in-place suite is updated too, and FakeRedis gained real ex= / expire / ttl semantics — a fake that ignored ex= would have let finding 4 regress silently.

Suite: 7707 passed, 18 skipped. Canary file: 146 passed, 2 skipped (up from 122; the pre-existing utils.safe_yaml fixture errors are unchanged and reproduce on dev).

Merge order

Agreed on sequencing. #1997 is merged; #2022 is rebased and green pending one re-run, and lands next. I'll merge dev into this branch afterwards and resolve the _collect_zombie_counts / Snapshot zombie-field overlap once, rather than three-way at merge time. One thing to flag for that resolution: moving the Docker collect to the top removes the bias R-01's docstring relies on ("snapshot_time is stamped at the start of collection while the docker exec runs at the end — which is the safe direction"). The dwell becomes exact rather than biased long. Not a correctness problem, but the comment will need amending.

[obasilakis]

Rebased on dev now that #2022 has landed (e0206dee). Resolved the overlap you flagged in one pass rather than three-way at merge time.

• Snapshot — both sides added fields, none conflicting. Kept all three: Canary: fail loud when a SQL-tier collector reads zero rows on a provably non-empty fleet (follow-up to #1540) #1813's docker_agent_names, ent#337's zombie_pids and zombie_container_started_at.
• _collect_zombie_counts — took fix(canary): stop three false-positive classes drowning the harness (ent#335/336/337) #2022's four-key contract and re-added names, making it five. Worth keeping the distinction explicit in the docstring, because the merge is exactly where it would get lost: counts/pids are keyed by exec_run success, so they thin on a degraded container — a liveness signal. names comes from the container list before any exec — a presence signal, which is what H-01 needs as independent proof the fleet is alive.
• collect_snapshot — this PR moves the collector to the top of the function, so ent#337's two new assignments moved with it.
• architecture.md — fix(canary): stop three false-positive classes drowning the harness (ent#335/336/337) #2022's rewritten R-01 row plus this PR's H-01 row.
• R-01 docstring — corrected the stale claim I flagged earlier ("biases the effective dwell slightly LONG — snapshot_time is stamped at the start of collection while the docker exec runs at the end"). The two instants are adjacent now. Nothing depended on the gap; noted so nothing starts to, and recorded that the ordering is owned by H-01.

tests/unit/: 7801 passed, 18 skipped. Canary + H-01 + R-01 dwell suites together: 219 passed, 2 skipped.

Ready for another look.

[dolho]

Re-review — H-01, after 36428fc8 + e0206dee

All four findings are genuinely fixed, not just answered — I read the code rather than the response. Two new things, both minor; only the first is worth a commit.

Prior findings — verified closed

• 1a evidence — the Docker collector now runs at the top of collect_snapshot with its own finally: collectors_ran.add(COLLECTOR_DOCKER), so docker_agent_names is populated on the roster_read_failed arm. collectors_ran is the right shape: you're correct that sources_unavailable is byte-identical for "succeeded" and "skipped", so no discipline at the read site could have recovered the third state — the collector has to declare it. _availability's is not True in the fall-through branch is the load-bearing detail and it's commented as such.
• 1b DB outage — previous_latest is wrapped, and the canary:last_cycle_red fallback is the part that makes fail-open safe rather than just louder. I checked the thing the fix depends on and the response doesn't mention: insert_canary_violation is already per-violation try/except, so a DB-down cycle still reaches emit_transition instead of dying at persist. The fix works end to end.
• 2 severity ladder — docker_evidence split out; Redis-only lands on roster_empty_unverifiable/major with the names still in evidence_sample. test_per_agent_exec_failure_still_confirms pins the ordering subtlety (a docker.exec[...] failure can't reach the unverifiable branch because the name was recorded pre-exec).
• 3 doc/code mismatch — resolved in the honest direction: the gate now covers every arm and the docs say so, with a momentary-DB-blip rationale that stands on its own. test_the_docstring_no_longer_claims_an_exempt_arm guarding a docstring against drift is a nice touch.
• 4 marker TTL — 24h via ex= on the write, refreshed each suspicious cycle. _refresh_marker_ttl making it an idle timeout rather than an absolute lifetime is a case I hadn't made and it's the right call.

Also confirmed: rebase is clean (0 behind dev, #1997 and #2022 both landed), the five-key _collect_zombie_counts contract keeps names (presence) distinct from counts/pids (liveness), and the R-01 docstring correction is accurate — snapshot_time and the docker exec are now adjacent.

1. A filtered run-cycle clobbers the Redis red set (new, ~2 lines)

run_invariants(snapshot, ids) returns only the selected ids, so on POST /api/canary/run-cycle {"invariant_ids": [...]} the new write

self._write_prev_cycle_red([i for i, v in results.items() if v])

persists that subset as the entire red set. Every other red invariant is dropped. Reproduced:

full cycle   → canary:last_cycle_red = ["H-01", "S-01"]
run-cycle(invariant_ids=["S-01"]) → canary:last_cycle_red = ["S-01"]

Consequence lands exactly where the fallback matters: with the DB down, the next background cycle evaluates inv_id not in prev_red, finds H-01 absent, and re-alerts — re-breaking the "a persistent condition chirps once" property this key exists to preserve. And it's reachable through the same invariant_ids path you already identified as the reason the H-01 marker needs a TTL, so the filtered cycle now has two ways to disturb cross-cycle state.

Either skip the write when a filter is in force, or merge instead of replace:

if invariant_ids is None:
    self._write_prev_cycle_red([i for i, v in results.items() if v])

(_write_prev_cycle_at has the same shape, but it's one timestamp with "when did we last look" semantics and it predates this PR — not asking you to touch it.)

2. A per-agent exec failure renders as a wholesale Docker outage (nit)

_has(snapshot, COLLECTOR_DOCKER) matches docker.exec[<agent>], which is a per-container skip, not a collector outage. On roster_empty_contradicted with one degraded container the forensic block renders:

*Sources:* docker=unavailable · redis=up
*Roster vs evidence:* 0 vs 3 agent(s)

Docker is simultaneously down and reporting three agents. The branch is right and tested; it's the render that misreports, on the one alarm whose stated job is legibility — and docker_available is also the field trinity-enterprise#202 will score. Matching only the wholesale prefixes (docker:, docker.import, docker.list) and letting docker.exec[...] ride as a separate partial-degradation count would keep both surfaces honest.

Nit, no action needed: when the DB recovers mid-episode, previous_latest holds no rows from the outage (the inserts failed too), so the primary rule reads the still-red condition as a fresh flip and chirps once more. Arguably the behaviour you want on recovery.

Verification

• tests/unit/test_1813_h01_collector_blindness.py + both test_1880_* — 137 passed
• tests/test_canary_invariants.py — 149 passed, 2 skipped. The 20 collection errors are a pre-existing local utils.safe_yaml import artifact, not this branch: origin/dev reproduces 19 of the same.
• CI green end to end — all six pytest seeds (base + head), schema-parity, CodeQL, prod-image-smoke, gitleaks, non-root.

Approving on fix 1. The reason codes and the H- family are good to harden as the trinity-enterprise#202 contract.