feat: contract 1.8 — §16 retry, §17 memo, §18 close(), §19 telemetry (D5) - #45
Merged
Merged
Conversation
…e-sync Re-vendors CONTRACT.md at 1.8 and lands the two foundation modules the rest of D5 builds on. Wiring them into the authz surface, plus §17's decision memo and §18's close(), follow in this branch. §16 — the policy that was being required but never defined. §11.2 rule 5 and §14.2 rule 6 had both been telling SDKs to retry "under the SDK's existing bounded read-only retry policy" while no such policy existed in the contract. This crate's improvisation was backon's ExponentialBuilder defaults with with_max_times(2): no jitter, no Retry-After. Contract 1.8 wrote the table down; src/retry.rs implements it. Hand-rolled rather than reconfigured, for two reasons backon cannot express: - Full jitter. §16.1 requires the wait to be uniform over [0, backoff]. backon's with_jitter() adds a random value in [0, min_delay) on top of the backoff — a much narrower distribution. The difference is the entire point of the clause: partial jitter keeps every client's retries clustered around the same instant, causing the thundering herd retries are meant to prevent. - Retry-After. backon has no seam for a server-supplied floor on the wait. Both non-deterministic inputs are injected behind traits (Jitter, Sleeper), as §16.7 demands — "a test that really waits 200 ms is a test nobody runs". The tests pin the jitter fraction to 0.0 and 1.0 to prove the range really is [0, backoff] and not backoff ± something, and record delays instead of taking them, so the whole file runs in 0.12 s. Two design points worth flagging: - Retry-After is NOT clamped to the 5 s delay cap. My first implementation clamped it and my own test caught the contradiction: that cap governs the computed backoff, while §16.1 makes the hint a floor with no ceiling. Clamping it would retry sooner than the server said it would be ready, which is the one thing the clause forbids. Exposure stays bounded by the attempt cap, and a server that wants to stall a caller can simply not answer. - The hint rides on an internal `Attempt` type rather than on AxiamError. §16 requires the policy to honor Retry-After, not callers to read it, and adding a field to the public non_exhaustive Network variant would have churned 45 construction sites across src/ and tests/ for something no caller asked for. The public error type is unchanged. Only the HTTP delta-seconds form is honored; the HTTP-date form is ignored because it requires the client clock to agree with the server's, and a skewed clock turns the hint into either a no-op or a multi-hour stall. §19 — telemetry hooks. src/telemetry.rs is an optional callback surface so callers can wire OpenTelemetry or Prometheus without this crate depending on either. Two rules are enforced by construction rather than by documentation: a hook is invoked through catch_unwind, so a panicking sink cannot fail an authorization check (§19.2 rule 2); and TelemetryEvent carries a closed field set with no escape hatch, so there is no way to leak a token into a payload destined for a metrics backend (§19.2 rule 3). Path templates, never substituted URLs, so a metric label cannot become a cardinality bomb. §16.5 is why the two modules land together: a retried-then-succeeded operation is otherwise invisible — a slow success with no signal that the server is failing — and that silence is the standing objection to automatic retry. The Retry event is what answers it. 13 unit tests, all passing; cargo fmt and clippy --all-features clean.
Puts the previous commit's modules into effect and adds §18. Without this the retry module was dead code and the old backon policy still shipped. §16 wiring. `check_access`, `can` and `batch_check` now run under `RetryRunner` instead of `backon`, so they get full jitter over [0, backoff] and honor Retry-After as a floor — neither of which the old policy did. The `Retry-After` header is read off the response before the body is consumed, since it typically rides a 429 or 503. `backon` stays in Cargo.toml for now; removing it is a separate cleanup once the other call sites migrate. The retry-eligible set is unchanged and remains exactly §16.2's list: authz reads only. login/verify_mfa/refresh/logout are untouched by the runner, so no mutation became retryable in this commit. §18 close(). `AxiamClient::close()` is async because the token manager's `clear()` is; it sets a `closed` flag with Release ordering and clears the token state. Every public entry point calls `ensure_open()` first, so use-after-close is an `AxiamError::Network` naming the cause rather than a silent reconnect (§18.1 rule 4). Two decisions worth stating: - close() does NOT log out. §18.1 rule 5 — shutting down a client releases local resources and must not reach the network. The server-side session deliberately outlives the client object, which is what lets a process restart and resume; a close() that logged out would silently end every user's session on each deploy. `tests/close_lifecycle_test.rs` asserts this against the *wire* rather than the return value, because that is the only way to catch it: a logout wired into close() succeeds silently. - reqwest exposes no eager pool-shutdown hook — the pool drops with the last Arc clone — so the guarantee close() actually makes is "no further requests", enforced by ensure_open() rather than merely documented. §19 events. `send_authz_post` emits the RequestStart/RequestEnd pair **per attempt**, not per logical call: §19.2 rule 5 requires a caller to be able to count real wire calls from the events, which one pair per operation would hide. The path template is the route constant, never a URL with ids substituted in, so a metric label cannot become a cardinality bomb. Builder gains `retry_enabled(bool)` (§16.6, default on) and `telemetry_hook(sink)` (§19). There is deliberately no knob for the attempt cap, base delay or delay cap: §16.1 forbids raising them, and eleven SDKs agreeing on one table is the point of the section. 4 new §18 conformance tests; the 5 pre-existing authz retry integration tests still pass unchanged against the new policy. 115 lib tests green, clippy --all-features --all-targets clean, fmt clean.
Completes the four D5 sections in this SDK. §17 client-side decision memo — off by default. §11.2 rule 6's ban on caching allow/deny decisions remains the default behaviour; this is the single opt-in exception the section carves out, reached only via `decision_memo_ttl(Duration)`. The default is `Duration::ZERO`, which means disabled — not "cache for zero seconds". The design points that are actually decisions, not mechanics: - TTL clamped to 5 s rather than rejected. The server's equivalent setting (AXIAM__AUTHZ__DECISION_CACHE_TTL_SECS) is an unclamped u64, so an operator can configure a multi-hour staleness window — a known residual. The client has no reason to repeat it, and asking for an hour silently getting 5 s beats failing construction for someone who plainly wants caching. - Allows and denies memoized identically. Caching only one would make the two outcomes take measurably different times, leaking which occurred to anyone who can observe latency, and would surprise every reader who assumed a cache is a cache. - Failures never memoized, enforced structurally: `put_at` is only reachable on the Ok path, so rule 7 cannot be forgotten rather than merely being checked. Memoizing a transport error as a deny would turn a blip into a TTL-long outage. - Cleared on login/verify_mfa/refresh/logout. Entries are keyed by subject, not by session, so a re-authentication as a different principal would otherwise inherit the previous principal's decisions. - The key is all four components with absent distinguished from present. The scope case is the dangerous one: a memo ignoring it answers a narrower question with a broader answer. The staleness bound is the TTL in BOTH directions, and the second one is what surprises people: a grant just added can still read as denied. The README says "reads-your-own-writes is not guaranteed" in those words, because the admin UI that grants a role and immediately re-checks is the case that breaks, and it breaks silently. Tests. 9 unit tests with an injected clock, plus 7 integration tests that assert the **wire-call count** rather than the returned value — a memo that returned the right answer while still making the request would pass a value-only test and deliver none of the point. The most important of them is `off_by_default_every_repeat_check_reaches_the_wire`: a build that quietly enabled this would change authorization staleness for every existing caller without them asking. One test initially failed and the code was right: I expected 3 wire calls after a logout, but `logout()` short-circuits without a request when the client never logged in. The proof the memo was cleared is that the *following* check reached the wire at all — with entries intact the count would have stayed at 1. The assertion now reads relative to the post-logout count and says why. README gains sections for all four: §16 (including the full list of what is never retried and why — state change and single-use credentials, each disqualifying alone), §18 (with close() not logging out called out), §19, and §17 with the reads-your-own-writes warning in a blockquote. Conformance line updated to name §17 and §19; §16 and §18 are MUST-level and so deliberately not named — a MUST is not something an SDK opts into. 124 lib tests, 4 §18 tests, 7 §17 tests, all green; clippy --all-features --all-targets clean; fmt clean.
`decision_memo_ttl`'s doc linked to `crate::memo::MAX_TTL`, but `memo` is a private module, so rustdoc emitted "public documentation links to private item". CI runs `cargo doc` with RUSTDOCFLAGS: -D warnings, so this would have failed the docs job — caught by running that gate locally with the same flag rather than by CI. Replaced with the literal value, which is what a reader of the public API wants anyway: they cannot follow a link into a private module.
Completes D5 in this SDK.
examples/telemetry_hook.rs — a runnable §19 sink aggregating request counts,
latency and retries, plus the exact OpenTelemetry mapping written out
alongside it. The example itself pulls no metrics crate, which is the point of
the section: §19 exists so callers can wire OTel or Prometheus WITHOUT this
crate depending on either, and shipping an `opentelemetry` dev-dependency to
demonstrate that would have undercut it.
Running it against an unreachable host prints:
check_access failure count=3 mean=354ms
retries: check_access 2
which is §16's attempt cap made observable. That output is the argument for
§16.5 in one line: without the hook those three wire calls are a single slow
failure, and a server degrading under retries looks healthy right up until the
retries stop being enough.
The commented OTel adapter uses `http.route` for the path template and notes
why — a metric label carrying a UUID is a cardinality bomb — and repeats the
two rules an adapter author can get wrong: don't block (hooks run on the
calling path), and don't enrich events with data from elsewhere, since the
closed field set is what keeps a token out of a metrics backend.
Removed the direct `backon` dependency: nothing referenced it once §16 landed.
It stays in Cargo.lock as a transitive dependency of `lapin` under the `amqp`
feature, which is unrelated and untouched.
Full sweep from a clean target after reclaiming disk: every test binary green
(0 failures), clippy --all-targets --all-features -D warnings clean, fmt clean,
cargo doc with RUSTDOCFLAGS=-D warnings clean, examples build, leak gate and
TLS-bypass lint clean.
This was referenced Aug 9, 2026
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
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.
Implements all four SDK quality-of-life sections that contract 1.8 added (ilpanich/axiam#283), and re-vendors
CONTRACT.mdat 1.8.openapi.jsonis unchanged — 1.8 is docs-only.This is the first of eleven SDKs. Four decisions below get copied into the other ten, so they are worth disagreeing with now rather than after the fan-out.
src/retry.rs, wired into every authz read, replacingbackonclose()+ensure_open()on every entry pointsrc/telemetry.rs+examples/telemetry_hook.rssrc/memo.rs, opt-in, off by default§16 — the policy that was being required but never defined
§11.2 rule 5 and §14.2 rule 6 had both been telling SDKs to retry "under the SDK's existing bounded read-only retry policy" while no such policy existed in the contract. This crate's improvisation was
backon's defaults withwith_max_times(2)— no jitter, noRetry-After. Contract 1.8 wrote the table down; this implements it.Hand-rolled rather than reconfigured, for two things
backoncannot express:[0, backoff].backon'swith_jitter()adds a value in[0, min_delay)— a much narrower distribution. That difference is the entire clause: partial jitter keeps every client's retries clustered at the same instant, which causes the thundering herd retries are meant to prevent.Retry-After. No seam for a server-supplied floor on the wait.Both non-deterministic inputs are injected behind traits, as §16.7 demands — "a test that really waits 200 ms is a test nobody runs." The tests pin the jitter fraction to
0.0and1.0to prove the range really is[0, backoff]and notbackoff ± something, and record delays instead of taking them. The whole file runs in 0.12 s.Nothing that changes server state is retried. The ineligible list is disqualified twice over: a transient failure after the server committed is indistinguishable at the client from one before it, and their credentials are single-use — an authorization code, a device code at redemption, a rotating refresh token — so a retry replays a spent credential into a hard
invalid_grant. The retry-eligible set is unchanged from before this PR: authz reads only.§17 — opt-in, and the warning is the feature
Off by default, because §11.2 rule 6's ban on caching authorization decisions is still the default behaviour.
AXIAM__AUTHZ__DECISION_CACHE_TTL_SECS) is an unclampedu64, so an operator can configure a multi-hour staleness window — a known residual. The client has no reason to repeat it.put_atis only reachable on theOkpath, so the rule can't be forgotten rather than merely being checked. Caching a transport error as a deny would turn a blip into a TTL-long outage.The README states reads-your-own-writes is not guaranteed in those words, in a blockquote. The admin UI that grants a role and immediately re-checks is the case that breaks, and it breaks silently.
§18 — close() does not log out
Idempotent; use-after-close raises an error naming the cause rather than silently reconnecting.
It never reaches the network. The server-side session deliberately outlives the client object — that is what lets a process restart and resume — so a
close()that logged out would silently end every user's session on each deploy.tests/close_lifecycle_test.rsasserts this against the wire, not the return value, because that is the only way to catch it: alogoutwired intoclose()succeeds silently.reqwestexposes no eager pool-shutdown hook, so the guaranteeclose()actually makes is "no further requests", enforced byensure_open()rather than merely documented.§19 — and why the example matters
A panicking hook cannot fail the operation that fired it (
catch_unwind), andTelemetryEventhas a closed field set with no escape hatch, so there is no way to leak a token into a payload bound for a metrics backend. Path templates, never substituted URLs.One
RequestStart/RequestEndpair per attempt, not per logical call, so callers can count real wire calls. Runningexamples/telemetry_hook.rsagainst an unreachable host prints:That is §16's attempt cap made observable, and the one-line argument for §16.5: without the hook those three wire calls are a single slow failure, and a server degrading under retries looks healthy right up until the retries stop being enough.
The example pulls no metrics crate — §19 exists so callers can wire OTel or Prometheus without this crate depending on either, and shipping an
opentelemetrydev-dependency to demonstrate that would have undercut the point. The exact OTel mapping is written out alongside it.Two things I got wrong, caught locally
Retry-Afterto the 5 s delay cap. My own test caught the contradiction: that cap governs the computed backoff, while §16.1 makes the hint a floor with no ceiling. Clamping means retrying sooner than the server said it would be ready — the one thing the clause forbids. Exposure stays bounded by the attempt cap anyway, and a server that wants to stall a caller can simply not answer.decision_memo_ttllinkedcrate::memo::MAX_TTL;memois private. CI runscargo docwithRUSTDOCFLAGS: -D warnings, so this would have been a red docs job — found by running that gate with the same flag rather than waiting for CI.Decisions worth a second opinion before the fan-out
Retry-Afterrides an internalAttempttype, notAxiamError. §16 requires the policy to honor the hint, not callers to read it, and putting it on the publicnon_exhaustiveNetworkvariant would have churned 45 construction sites for something no caller asked for. The public error type is unchanged. If you'd rather callers could read it, that is cheap now and expensive after ten more SDKs.close()not logging out, and being async becauseTokenManager::clear()is.Verification
Full sweep from a clean
target/:cargo test --all-featurescargo clippy --all-targets --all-features -- -D warningscargo fmt --all --checkcargo doc --all-features --no-depswithRUSTDOCFLAGS=-D warningscargo build --examples --all-featureseyJ-prefixed strings intarget/debug/)New tests: 13 retry unit, 9 memo unit, 4 §18 integration, 7 §17 integration. The 5 pre-existing authz retry integration tests pass unchanged against the new policy.
Notes
backondependency — nothing referenced it once §16 landed. It stays inCargo.lockas a transitive dep oflapinunder theamqpfeature.Generated by Claude Code