fix(canary): B-01 queue-status coherence — backend-consistent Side B + confirm-re-read (#1450)#1495
Merged
Merged
Conversation
Two pre-existing breakages landed via #1472 (367a5e1, merged today) in root-level tests/test_canary_invariants.py — a file the CI unit job (tests/unit/ only) does not run, so they merged undetected: 1. Duplicate `CREATE TABLE agent_schedules` in the `canary_db` fixture DDL — the E-06 work added a second definition next to the pre-existing one, so `executescript` raised "table agent_schedules already exists" and every fixture-dependent canary test errored. Removed the older, unused block (its `message`/`owner_id` columns have no consumer; `_add_schedule` and the E-06/L-03 collectors use only the kept block's columns). 2. `test_run_invariants_all` expected registry set omitted "E-06" (added to the invariants registry by the same PR). Added E-06 to the expected set and asserted it is green on a clean platform. Restores a green baseline (92 passed) so the #1450 B-01 work can be verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…+ confirm-re-read (#1450) B-01 compared two reads that were neither temporally atomic nor backend-consistent (both latent while the canary is default-OFF and SQLite makes the two reads hit one file): (a) Temporal non-atomicity — a concurrent enqueue/backlog-drain landing between the two reads produced a transient count mismatch → spurious critical + green→red Slack alert. (b) Backend divergence (#300/#1093) — Side A (`db.get_queued_count`) honors `get_engine()`/DATABASE_URL; Side B read raw sqlite3 at DB_PATH. On Postgres those are two different databases, so B-01 compared Postgres truth to a stale/absent SQLite file (fatal under the Postgres direction + SQLite EOL, #1278). Production-side residue of #1446 (PR #1452), which fixed only the test-harness `sys.modules` leak and deferred these two gaps. Fix (localized to B-01): - New `_collect_queued_ids_via_engine` reads B-01's Side B through the SAME `get_engine()` seam as the accessor, on a dedicated `queued_ids_via_engine` snapshot field. Independent code path (SELECT id/literal 'queued' vs COUNT(*)/the QUEUED enum) — shares a database, not a code path, so a cache/status-filter regression still surfaces (non-tautology, AC #3). No cache / second count of the queue (AC #4). - The collector performs one confirm-re-read on a mismatch: a transient race self-heals; a persistent drift survives and fires (AC #2). An engine-read or unconfirmable confirm degrades to a B-01 skip — it never compares an engine count against the raw-sqlite id-set (the blocker the reviews surfaced). - `queued_exec_ids` (raw sqlite) stays untouched for B-02/E-02, so blast radius is B-01-only and the sibling #1077 merge stays clean. The running-side / known-agents reads remain raw-sqlite (half-migrated collector); the collector-wide migration + a dark-canary tripwire are a filed follow-up. Tests (test_canary_invariants.py): retarget the synthetic B-01 tests at `queued_ids_via_engine`; add the AC #5 regression net — a diverged-backend proxy (raw ≠ engine temp files) that false-fires pre-fix and is green post-fix, a transient-race confirm-absorb + persistent-drift-still-fires pair, and an engine-read-failure→skip test. New split fixtures (`canary_db_split` / `reload_canary_split`) model the diverged backend without a live PG. Convert the file to the sanctioned `_STUBBED_MODULE_NAMES` + `_restore_sys_modules` escape hatch so its import-time stubs no longer leak (the #1446 mechanism) and the reimport `del`s are lint-clean. Docs: architecture.md Canary B-01 row updated for the engine-backed read path + confirm-re-read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sync-feature-flows: canary-internal change, no dedicated flow doc (architecture.md is canonical); append one Recent Updates row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # docs/memory/feature-flows.md
vybe
approved these changes
Jul 7, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
Validated: Fixes #1450 ✅, focused 5-file diff. Side-B migration to the get_engine() seam is correct and the non-tautology property is preserved (SELECT id + literal 'queued' vs COUNT(*) + enum). Confirm-re-read races degrade to skip, never a spurious critical — right call. Resolved the feature-flows.md index conflict against dev (kept both rows) and pushed the merge; ran tests/test_canary_invariants.py locally on the merged branch — 97 passed.
vybe
enabled auto-merge (squash)
July 7, 2026 08:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1450
Summary
Canary B-01 (queue-status coherence) collected its two compared values from two reads that were neither temporally atomic nor backend-consistent. Latent on SQLite (canary default-OFF, both reads hit the same file), but fatal under the Postgres direction. This is the production-side residue #1446 explicitly deferred (that P1 fixed only the test-harness
sys.modulesleak).The two gaps closed
(a) Temporal non-atomicity — the accessor count and the id-list read happened at different instants; a concurrent enqueue/backlog-drain landing between them produced a transient mismatch → spurious
critical+ green→red Slack alert.(b) Backend divergence (#300/#1093) — Side B (
queued_exec_ids) was collected via raw sqlite3 atDB_PATH, while Side A (db.get_queued_count) honorsget_engine()/DATABASE_URL. On Postgres these are two different databases, so B-01 compared Postgres truth against a stale/absent SQLite file.Fix
queued_ids_via_engine(snapshot.py::_collect_queued_ids_via_engine) reads queued ids through the sameget_engine()seam as the accessor —SELECT id/literal'queued'vs the accessor'sCOUNT(*)/TaskExecutionStatus.QUEUEDenum, so it shares a database but not a code path (stays a genuine coherence check, not a tautology — AC Feature/vector log retention #3).queued_exec_idsstays raw-sqlite for B-02/E-02.collect_snapshot(): on a first-read mismatch, re-read both sides once. A transient race self-resolves (green); a persistent drift survives and still fires. An unconfirmable confirm degrades to a skip, never a falsecritical(AC Feature/gemini runtime support #2).queued_ids_via_engine=None), recorded insources_unavailable— never falls back to the raw set, which would re-arm gap (b) on Postgres.Collector is now half-migrated (B-01's queued id-set is engine-backed; running-side/known-agents/orphan reads stay raw-sqlite — invisible on SQLite, tracked for the collector-wide migration follow-up).
Tests
tests/test_canary_invariants.py(all 97 pass):test_b01_backend_consistency_on_diverged_backend— newcanary_db_splitfixture models a Postgres-style diverged backend (raw file ≠ engine file) without a live PG; proves B-01 stays green when the two backends diverge (AC Setup improvements #5).test_b01_tolerates_transient_race— confirm-re-read absorbs a between-reads race.test_b01_confirm_does_not_mask_persistent_drift— confirm does not hide a real regression.test_b01_skips_on_engine_read_failure/test_skips_when_engine_ids_none— engine-read failure degrades to skip._STUBBED_MODULE_NAMES+ autouse_restore_sys_modules— file is now lint-clean undertests/lint_sys_modules.py(the bug: canary B-01 queue-status coherence violation fires under full-suite load — snapshot race or real projection gap #1446 discipline).Docs
architecture.md— B-01 row updated (both sides viaget_engine(), confirm-re-read, engine-fail skip, half-migrated note).feature-flows.md— Recent Updates row.Validation
/validate-prclean: security scans clear, no new backend module (no Dockerfile packaging), no new env var.sys.moduleslinter: this file5 → 0. All 97 canary tests pass.🤖 Generated with Claude Code