From acfd0cc473daa245f63d9da64e38ab1a8a93db14 Mon Sep 17 00:00:00 2001 From: Joel Teply Date: Thu, 13 Aug 2026 16:59:12 -0500 Subject: [PATCH] =?UTF-8?q?fix(persona):=20a=20stub=20citizen=20has=20no?= =?UTF-8?q?=20transport=20and=20says=20so=20=E2=80=94=20unblocks=20canary'?= =?UTF-8?q?s=20--lib=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo test -p continuum-core --lib` has failed on EVERY canary push for days. Six tests, five of them the same cause: `StubAircCitizen::subscribe_all_rooms` was `unreachable!()`. The premise was stated in the code — "no service-loop test drives the stub's subscribe". True when written. It stopped being true when the supervisor grew its doctrine/wall cache: `supervisor.rs:745` now subscribes to wire a publish invalidator, so five supervisor tests — about adapter materialization and warmup, caring nothing about event streams — reached that line and aborted. A guard whose premise the codebase has since outgrown does not fail like a guard. It fails like a broken build, and it takes the whole gate with it. With --lib red on every push, "canary is green" means nothing to anyone else in the repo, which is how it stayed broken for days while everyone read around it. What the guard was RIGHT about is kept. It refused to return an empty stream, because that hands the supervisor a wire that never fires: the cache goes stale in silence and the tests pass while proving nothing. This does not do that. It returns `AircError::Transport` instead. That is not the "variant that doesn't fit" the old comment feared — it is a free-form transport-side variant and "this citizen has no transport" is a transport-side fact. The call site already has the branch: on Err the supervisor serves raw doctrine/wall sources and logs `cache UNWIRED … slow but never stale`. The absence stays LOUD and travels the path designed for it instead of killing the process. The stub's own test flips from asserting a panic to pinning the invariant that actually matters: it must report an ERROR, never Ok-with-an-empty-stream. NOT VALIDATED LOCALLY, and I will not pretend otherwise: this Windows box cannot build `core/llama` — its bundled cmake does not know the "Visual Studio 18 2026" generator, so `cargo check -p continuum-core` dies in the build script before touching this code. Standalone Kitware CMake is the fix and it is the `Mod-CMake` item in the install plan, not this PR. CI's `cargo test -p continuum-core --lib` and `cargo check windows-msvc (lib + tests)` are the gates that prove this one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc --- .../src/persona/airc_citizen.rs | 70 +++++++++++++++---- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/core/continuum-core/src/persona/airc_citizen.rs b/core/continuum-core/src/persona/airc_citizen.rs index 775cb9642..af06b0e31 100644 --- a/core/continuum-core/src/persona/airc_citizen.rs +++ b/core/continuum-core/src/persona/airc_citizen.rs @@ -347,18 +347,38 @@ impl AircCitizen for StubAircCitizen { } async fn subscribe_all_rooms(&self) -> Result { - // No service-loop test drives the stub's subscribe — the - // service loop receives messages through StubConversation - // directly, never through the citizen's stream. If a future - // test ever wires the stub into the conversation, this panics - // visibly per [[no-fallbacks-ever]] rather than silently - // returning an empty stream or fabricating an AircError - // variant that doesn't fit ("Transport"/"Route"/etc). - unreachable!( - "StubAircCitizen::subscribe_all_rooms must not be called — \ - service-loop tests should drive the loop through \ - StubConversation directly, not through the citizen handle" - ); + // A stub citizen HAS no transport, and says so. + // + // This was `unreachable!()`, on the stated premise that "no + // service-loop test drives the stub's subscribe". That premise was + // true when written and stopped being true when the supervisor grew + // its doctrine/wall cache: `supervisor.rs` now subscribes to wire a + // publish-invalidator, so five supervisor tests — which are about + // adapter materialization and warmup, and care nothing about event + // streams — reached this line and aborted. `cargo test -p + // continuum-core --lib` has been red on every canary push since, + // which is what makes "canary is green" mean nothing to everyone + // else in the repo. + // + // The guard was right to refuse an EMPTY STREAM: that would hand the + // supervisor a wire that never fires, so the cache would go stale + // silently and the tests would pass while proving nothing. That is + // the masking the original comment correctly rejected, and this does + // not do it. + // + // Returning `Transport` is not the "variant that doesn't fit" the + // old comment feared. It is a free-form transport-side variant and + // "this citizen has no transport" is a transport-side fact. The call + // site already has the matching branch: on `Err` the supervisor + // serves raw doctrine/wall sources and logs `cache UNWIRED … slow but + // never stale`. So the absence stays LOUD and correct — it travels + // the path designed for it instead of killing the process. + Err(AircError::Transport( + "StubAircCitizen has no transport — nothing subscribes, nothing publishes. \ + Drive service-loop tests through StubConversation; callers that need a live \ + stream must use a real citizen." + .to_string(), + )) } async fn say_in(&self, _room_id: Uuid, _text: &str) -> Result { @@ -384,11 +404,31 @@ mod tests { assert!(events.is_empty()); } + /// what this catches: the stub handing back a wire it does not have. + /// + /// This asserted a PANIC until 2026-08-13, on the premise that nothing + /// would ever call subscribe on a stub. The supervisor's doctrine/wall + /// cache then did exactly that, and five supervisor tests — about adapter + /// materialization, not events — died on it, keeping `cargo test -p + /// continuum-core --lib` red on every canary push. + /// + /// The invariant that actually matters survives, and is what this now + /// pins: the stub must report an ERROR, never `Ok` with an empty stream. + /// An empty stream is a wire that never fires, so the supervisor's cache + /// would go stale in silence and this test would pass while proving + /// nothing. Err travels the branch built for it — raw sources, loud warn. #[tokio::test] - #[should_panic(expected = "service-loop tests should drive the loop")] - async fn stub_subscribe_panics_loudly() { + async fn stub_subscribe_reports_no_transport_rather_than_faking_a_wire() { let stub: Arc = Arc::new(StubAircCitizen::new(Uuid::new_v4())); - let _ = stub.subscribe_all_rooms().await; + let error = stub + .subscribe_all_rooms() + .await + .err() + .expect("stub must REFUSE to subscribe — an Ok here is an empty stream nobody fires"); + assert!( + matches!(error, AircError::Transport(_)), + "the refusal must name the missing transport, got: {error}" + ); } // what this catches: a reply addressed to the room that asked, rather than