From bc8755367beb30999e4bcba2f29fdcba3310a66e Mon Sep 17 00:00:00 2001 From: iceteaSA <171169159+iceteaSA@users.noreply.github.com> Date: Sat, 29 Aug 2026 12:47:57 +0200 Subject: [PATCH 1/2] status: publish stale_pending, the mark that predicts a slow get credential.status reported ready:true / last_error_code:null on a record whose next get was about to buy an upstream token exchange -- measured 2026-08-25, twelve samples over five minutes with the chain row already written. `ready` answers "would a get succeed" (genuinely yes; the mark exists so the next get refreshes rather than refusing) and could not answer what that get would cost: active, stale_pending=false -> local read, sub-millisecond active, stale_pending=TRUE -> forces an upstream exchange, seconds needs_reauth -> fails fast, no upstream call Withheld until now on the grounds that a field added for a hypothetical caller is machinery nobody can test against a real requirement. That condition ended: the first vault consumer warms a credential cache at startup and needs to SKIP the accounts that would overrun its bound rather than discover them by timing out -- and the only other way to observe the mark was credential.get, the very call whose cost is in question. Free to serve: stale_pending is a plaintext column, store::meta() already selects it, and RecordMeta already carries it. This path was fetching the value and discarding it before serialization -- the state record_version was in before it was published. No decrypt, no master key, no extra query. Absent rather than false when no handle resolved: a defaulted false would read as "no repair pending" for a revoked handle, an assertion this path cannot make. Test pins both halves and was mutation-checked (publish -> None turns it red): the mark is visible without calling get, AND ready stays true -- expensive is not unhealthy. If that second assertion ever flips, the field has been folded into health and consumers will start treating usable credentials as broken. Lockfile regenerated against subconscious f52c1309 (subc-core 0.9.0->0.11.0, subc-control 0.7.0->0.9.0). Wire crates unchanged: subc-protocol 0.13.0, subc-transport 0.5.1 -- protocol 2 compatibility preserved. Gate: 421 tests, exit 0. --- crates/credentials-module/src/main.rs | 99 +++++++++++++++++++ crates/credentials-module/src/read_surface.rs | 78 +++++++++++---- 2 files changed, 159 insertions(+), 18 deletions(-) diff --git a/crates/credentials-module/src/main.rs b/crates/credentials-module/src/main.rs index b7d2bf4..fb2017c 100644 --- a/crates/credentials-module/src/main.rs +++ b/crates/credentials-module/src/main.rs @@ -4158,6 +4158,105 @@ mod tests { ); } + /// The mark that predicts a SLOW get on a record every other field calls healthy. + /// + /// Pins the exact reading that was invisible before this field existed: `ready: + /// true`, `last_error_code: null`, and the next `get` about to buy an upstream token + /// exchange. A consumer sizing a startup bound cannot get that from `ready`, because + /// `ready` is genuinely TRUE -- the mark exists so the next get refreshes rather than + /// refusing. + #[tokio::test] + async fn status_publishes_the_stale_mark_without_calling_the_credential_unhealthy() { + let (surface, store, _db) = tmp_surface_with_store(16); + let handle = credentials_core::store::mint_handle().expect("mint handle"); + store + .put_handle_hash( + &handle.hash, + "apikey:active", + AuditCtx::admin(AuditOp::MintHandle), + ) + .expect("put handle"); + + let clean = surface + .status( + 1, + &crate::read_surface::StatusParams { + handle: Some(handle.raw.clone()), + }, + ) + .await; + assert_eq!( + clean.stale_pending, + Some(false), + "a resolved handle must report the mark explicitly, not by omission" + ); + assert!(clean.ready, "precondition: the record starts healthy"); + + // Exactly what a consumer's 401 report does, at the version it was served. + let served = clean + .record_version + .expect("resolved handle reports version"); + store + .mark_stale_if_version_reported( + "apikey:active", + served, + AuditCtx::admin(AuditOp::ReportAuthFailure), + credentials_core::store::AuthObservation { + kind: "consumer_report_stale", + provider_status: Some(401), + detail: None, + }, + ) + .expect("mark stale"); + + let marked = surface + .status( + 1, + &crate::read_surface::StatusParams { + handle: Some(handle.raw.clone()), + }, + ) + .await; + assert_eq!( + marked.stale_pending, + Some(true), + "the mark must be visible WITHOUT calling get -- the whole point is to avoid \ + the call whose cost is in question" + ); + // The load-bearing half. If this ever flips to false, the field has been folded + // into health and a consumer will start treating a usable credential as broken. + assert!( + marked.ready, + "a stale-marked record is still USABLE -- expensive is not unhealthy" + ); + assert!( + marked.last_error_code.is_none(), + "a pending repair is not an error that has occurred" + ); + + // ABSENT, never defaulted false: claiming "no repair pending" for a record this + // path could not read would be an assertion with no basis behind it. + let overall = surface + .status(1, &crate::read_surface::StatusParams { handle: None }) + .await; + assert!( + overall.stale_pending.is_none(), + "overall readiness names no credential, so it can report no mark" + ); + let unknown = surface + .status( + 2, + &crate::read_surface::StatusParams { + handle: Some("ckh_definitely-not-a-handle".to_string()), + }, + ) + .await; + assert!( + unknown.stale_pending.is_none(), + "an unresolvable handle must not assert anything about a record" + ); + } + #[tokio::test] async fn status_handle_probe_runs_the_limiter() { let (surface, store, _db) = tmp_surface_with_store(15); diff --git a/crates/credentials-module/src/read_surface.rs b/crates/credentials-module/src/read_surface.rs index 806ceaf..1dbeb71 100644 --- a/crates/credentials-module/src/read_surface.rs +++ b/crates/credentials-module/src/read_surface.rs @@ -464,24 +464,9 @@ pub struct StatusResult { /// would defeat itself by moving the version it matched on). So a stable version /// with `ready: false` is a normal reading, not a stuck cursor. /// - /// WHAT `status` DELIBERATELY DOES NOT PUBLISH: `stale_pending`. For the whole - /// window between a version-matched report and the next `get`, this surface reports - /// a healthy credential while the store has already recorded the mark. Measured by - /// an external contributor on 2026-08-25: twelve samples over five minutes, all - /// `ready: true`, with the chain row already written. - /// - /// That is correct for what `ready` MEANS -- "would a get succeed" -- and it stays - /// true, because the mark exists precisely so the next get refreshes rather than - /// refusing. A get-path consumer receives the entire benefit without ever needing to - /// see the flag, which is why no consumer has asked for it. - /// - /// The gap is real for a consumer that POLLS status as its health surface: it cannot - /// learn a mark is outstanding, so it cannot distinguish "healthy" from "healthy, - /// with a repair pending". Not published today because no such consumer exists, and - /// a field added for a hypothetical caller is machinery nobody can test against a - /// real requirement. Revisit when one arrives -- the disclosure cost is small - /// (`ready: false` already tells a handle holder that someone observed a failure), - /// so the decision is about usefulness rather than safety. + /// See [`StatusResult::stale_pending`] for the mark that predicts a SLOW get on an + /// otherwise healthy record -- the two fields answer different questions and a + /// consumer sizing a timeout needs the other one. /// /// AND IT DOES NOT CATCH EVERY REPAIR -- read `ready`, not this, to answer "is it /// usable now". `reactivate` clears a wrong `needs_reauth` verdict WITHOUT touching @@ -497,6 +482,50 @@ pub struct StatusResult { /// tracks the MATERIAL; `ready` tracks the VERDICT; a repair can move either alone. #[serde(skip_serializing_if = "Option::is_none")] pub record_version: Option, + + /// A consumer reported the current token refused, so the NEXT `get` must refresh + /// before serving. Present only when a handle resolved. + /// + /// THIS IS A LATENCY PREDICTOR, NOT A HEALTH FIELD, and it exists because the two + /// are not the same question. `ready` answers "would a get succeed" -- and on a + /// stale-marked record the honest answer is YES, because the mark exists precisely + /// so the next get refreshes rather than refusing. What `ready` cannot say is what + /// that get will COST: + /// + /// state=active, stale_pending=false -> local read, sub-millisecond + /// state=active, stale_pending=TRUE -> forces an upstream exchange, seconds + /// state=needs_reauth -> fails fast, no upstream call + /// + /// So a record can read healthy on every other field while the next call is three + /// orders of magnitude slower than the one before it. Measured 2026-08-25 before + /// this field existed: twelve status samples over five minutes, every one + /// `ready: true, last_error_code: null`, with the mark already written to the store + /// and its chain row committed. Nothing on the wire distinguished them. + /// + /// WHY IT IS PUBLISHED NOW rather than earlier: the field was withheld deliberately + /// while no consumer polled this surface, on the grounds that a field added for a + /// hypothetical caller is machinery nobody can test against a real requirement. That + /// condition ended -- the first vault consumer warms a credential cache at startup + /// and needs to SKIP the accounts that would overrun its bound, rather than + /// discovering them by timing out. Skipping requires seeing the mark; the only other + /// way to observe it was `credential.get`, which is the very call whose cost is in + /// question. + /// + /// FREE TO SERVE, which is the whole reason this is a field and not a new verb: + /// `stale_pending` is a PLAINTEXT column, `store::meta()` already selects it, and + /// [`RecordMeta`] already carries it. This path was fetching the value and + /// discarding it before serialization -- exactly the state `record_version` was in + /// before it was published. No decrypt, no master key, no extra query. + /// + /// NO NEW DISCLOSURE: `ready: false` already tells a handle holder that someone + /// observed a failure on this credential. This says only that a repair is pending on + /// one that still works. + /// + /// ABSENT rather than `false` when no handle was presented or the handle did not + /// resolve -- a defaulted `false` would read as "no repair pending" for a revoked + /// handle, which is an assertion this path has no basis to make. + #[serde(skip_serializing_if = "Option::is_none")] + pub stale_pending: Option, } /// The read surface: the engine (for refresh-on-read), the per-connection limiter, @@ -1188,6 +1217,10 @@ impl ReadSurface { last_error_code: None, lease_held, record_version: None, + // No handle means no record, so there is no mark to report. Absent + // rather than false: this is overall daemon readiness, not a claim + // about any credential. + stale_pending: None, }; } Some(h) => h, @@ -1202,6 +1235,9 @@ impl ReadSurface { // A fenced-out daemon is not ready even for an Active credential. ready: !fenced_out && matches!(meta.state, credentials_core::store::RecordState::Active), + // Deliberately NOT folded into `ready`: a stale-marked record is + // still usable, it is merely expensive on the next read. + stale_pending: Some(meta.stale_pending), last_error_code: match meta.state { credentials_core::store::RecordState::NeedsReauth | credentials_core::store::RecordState::Retired => { @@ -1213,18 +1249,24 @@ impl ReadSurface { lease_held, record_version: Some(meta.record_version), }, + // Meta unreadable: absent, not false. Reporting "no repair pending" for + // a record we could not read would be an assertion with no basis. Err(_) => StatusResult { ready: false, last_error_code: Some(ReadError::NotFound), lease_held, record_version: None, + stale_pending: None, }, }, + // Unresolvable handle -- uniform not_found, and no claim about a record that + // may not exist. Err(_) => StatusResult { ready: false, last_error_code: Some(ReadError::NotFound), lease_held, record_version: None, + stale_pending: None, }, } } From 8d3d81c9d53b248d886e9032b52b030a1b4ee598 Mon Sep 17 00:00:00 2001 From: iceteaSA <171169159+iceteaSA@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:40:58 +0200 Subject: [PATCH 2/2] status: pin the error frame shape, not just the class strings The existing golden test pins the four `class` wire strings against the contract's canonical set. It says nothing about the envelope they arrive in. Rename `class` to `error_class`, move the error a nesting level, or drop the field from the body, and that test stays green while every consumer breaks. Found by the first live consumer of this surface. They typed a decoder from the published contract, parsed a real error frame SUCCESSFULLY, and discarded `class` entirely -- serde drops unknown fields without complaint, so a decoder ignoring the field it was told to branch on is indistinguishable from one honouring it. They then branched on `code` through a closed enum with no fallback, which turns the first added code into a parse failure rather than an unknown-code branch, and their untagged wrapper reported it as a malformed local config file. None of that is fixable from this side. What is fixable is guaranteeing the bytes never move under them, so a consumer that gets it right stays right. Serialized through the real producer type rather than a hand-built json!, so it pins what the wire carries; a reconstruction would only pin the reconstruction. The literal is the exact frame captured from a live daemon and handed to that consumer, who pinned the same bytes on their side -- drift now goes red in both directions, which neither test can achieve alone. Assertion order is load-bearing and this is the second version. Written with the equality first, the specific check never ran: assert_eq! panics on any difference, so dropping `class` reported "the frame shape drifted" and left the reader to diff two blobs -- the diagnostic existed only for the case it could not reach. A cheap specific assertion must precede a broad one that subsumes it, or it is decoration. Mutation-checked both ways: renaming the field and skipping it each turn it red with the specific message, and restoring returns it green. Gate: 421 tests, exit 0, nine real-daemon e2e arms executing. --- crates/credentials-module/src/read_surface.rs | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/crates/credentials-module/src/read_surface.rs b/crates/credentials-module/src/read_surface.rs index 1dbeb71..e49b5a4 100644 --- a/crates/credentials-module/src/read_surface.rs +++ b/crates/credentials-module/src/read_surface.rs @@ -1502,6 +1502,59 @@ mod error_class_tests { ); } + /// Golden conformance for the FRAME SHAPE, which the class-string test above does + /// not cover and cannot: it pins the four `class` values while saying nothing about + /// the envelope they arrive in. Rename `class` to `error_class`, move the error a + /// level, or drop `class` from the body entirely, and that test stays green while + /// every consumer breaks. + /// + /// WHY THIS EXISTS AT ALL: a consumer typed a decoder from the published contract, + /// parsed a real error frame SUCCESSFULLY, and silently discarded `class` — serde + /// drops unknown fields without complaint, so a decoder that ignores the field it + /// was told to branch on looks identical to one that honours it. They then branched + /// on `code` through a closed enum, which turns the first added code into a parse + /// failure rather than an unknown-code branch. Neither is reachable from this side; + /// what IS reachable is guaranteeing the bytes never move under them. + /// + /// Serialized through the REAL producer type rather than a hand-built `json!`, so + /// this pins what the wire actually carries. A reconstruction would only pin the + /// reconstruction — the frame could drift and this would still pass. + /// + /// The literal is the exact frame captured from a live daemon and handed to that + /// consumer, who pinned it in their tree. Both directions now go red on drift. + #[test] + fn error_frame_shape_is_pinned() { + let frame = GetOutcome::Err { + error: ErrorBody { + code: ReadError::NotFound, + class: ErrorClass::Permanent, + }, + }; + let got: serde_json::Value = + serde_json::to_value(&frame).expect("serialize the error outcome"); + + // ORDER IS LOAD-BEARING, and this is the second version. Written with the + // equality first, the specific check below never ran: `assert_eq!` panics on any + // difference, so dropping `class` reported "the frame shape drifted" and left the + // reader to diff two blobs. The diagnostic existed only for the case it could not + // reach. A cheap, specific assertion must precede a broad one that subsumes it, + // or it is decoration. + assert!( + got["error"].get("class").is_some(), + "`class` vanished from the error body — the contract's branch-on-class rule \ + becomes unfollowable and consumers silently fall back to branching on `code`" + ); + + let want = serde_json::json!({ + "error": { "code": "not_found", "class": "permanent" } + }); + + assert_eq!( + got, want, + "the error frame shape drifted — consumers branch on these exact keys" + ); + } + /// An UNMAPPED store error degrades to a TRANSIENT code, never a permanent one. /// /// This is the property a cross-repo consumer's destructive behaviour rests on,