From 6e4067c47f81dfec9ccb8ff83b7a01b6e651de8a Mon Sep 17 00:00:00 2001
From: iceteaSA <171169159+iceteaSA@users.noreply.github.com>
Date: Sun, 30 Aug 2026 21:35:49 +0200
Subject: [PATCH] tests: assert the refresh_failed observation the comment
 promised

The engine-driven latch test says, in a comment, that the engine "writes a
`refresh_failed` observation here that no direct store call produces" -- and
that was the stated reason the test had to run the engine rather than
reproduce its outcome. Nothing checked it.

A claim in prose with no assertion behind it is the same defect as a guard
whose coverage is invisible from its green result: the test passed, the
comment sounded like a guarantee, and removing the observation entirely would
have kept it passing. The reviewer asked for the removal case; misclassifying
the detail is the same hole and is covered too.

The row is the only place that says a provider was reached and what it
answered. The audit chain's entry here is a generic `invalidate` that cannot
distinguish a terminal invalid_grant from a transient transport failure, and
only the terminal one latches -- so a `refresh_failed` with the wrong detail
is worse than an absent one, because it reads as diagnosed.

Both cases mutation-proved. Passing `None` where the engine builds its
observation drops the count to zero and reports the surviving
`consumer_report_stale` row in the failure message. Replacing
`e.variant_name()` with a hardcoded "transport" leaves the row in place and
fails on the detail instead, which is the misclassification arm the count
assertion alone would not have caught.

Gate: exit 0, nine real-daemon e2e arms executing, pinned at subconscious
572e247d -- master's lock moved to subc-core 0.13.0 while this was in flight,
and the wire crates did not move with it.
---
 crates/credentials-module/src/main.rs | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/crates/credentials-module/src/main.rs b/crates/credentials-module/src/main.rs
index 6bedf3b..86c0842 100644
--- a/crates/credentials-module/src/main.rs
+++ b/crates/credentials-module/src/main.rs
@@ -4744,6 +4744,35 @@ mod tests {
             "the forced refresh must fail through the engine: {err:?}"
         );
 
+        // The observation, asserted rather than advertised. The comment above claims the
+        // engine writes a diagnostic no direct store call produces -- and until this
+        // assertion existed that was a claim in prose with nothing behind it, which is the
+        // same defect as a guard whose coverage is invisible from its green result. The
+        // audit chain records a generic `invalidate` here; only this row says the provider
+        // was reached and what it answered.
+        let events = store.recent_auth_events(10).expect("read events");
+        let refresh_failed: Vec<_> = events
+            .iter()
+            .filter(|e| e.kind == "refresh_failed")
+            .collect();
+        assert_eq!(
+            refresh_failed.len(),
+            1,
+            "the engine's invalid_grant arm must leave exactly one refresh_failed row; \
+             got {events:?}"
+        );
+        assert_eq!(
+            refresh_failed[0].credential_id, "oauth:needs_reauth_after_stale",
+            "the row must name the credential the refresh was attempted for"
+        );
+        assert_eq!(
+            refresh_failed[0].detail.as_deref(),
+            Some("invalid_grant"),
+            "the row must carry WHICH provider verdict was seen -- a `refresh_failed` with \
+             no detail cannot distinguish a terminal invalid_grant from a transient \
+             transport failure, and only the terminal one latches"
+        );
+
         // Precondition checks: the construction actually reproduced the live shape, so a
         // green fix can be trusted to mean the fix is real and not a different test
         // passing for a different reason.