diff --git a/CHANGELOG.md b/CHANGELOG.md index 192490b..4178f90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 span (a no-op without a `Tracing` span). Routed to the ADR-0014 Telemetry plane. Adds the `tracing` facade (runtime dep) + `tracing-subscriber` (dev-dep). (ADR-0031 §6, ADR-0014, ADR-0034.) +- `oath-adapter-net-http-api` `stack()` assembly + `HttpConfig` (Slice 2, assembly) — + `stack()` composes the canonical resilience order (ADR-0031 §1) + `Tracing(CircuitBreaker(Retry(RateLimit(Timeout(SetHeaders(Auth(leaf)))))))` over any + leaf, returning `Result`. + It builds the fallible `RateLimit` layer first (running `validate_coverage` + + `validate_concurrency_singleton`), so a coverage/param/singleton failure is a boot + error before the rest is assembled. `HttpConfig` is the non-generic aggregate + (`timeout`, `retry`, `circuit_breaker`, `headers`, `rate_limit_max_wait`); the pacing + map, `auth`, and `timer` are separate arguments. Full-stack ordering invariants + (CircuitBreaker-outside-Retry, RateLimit-inside-Retry, send-Timeout, per-attempt + Auth, Scope fail-closed) are regression-tested over an inline leaf + `MockTimer`. No + new dependency; no existing-layer change. (ADR-0031 §1, ADR-0034.) The hyper leaf + + `build()` land in the following slice. - net-http construction-surface design refinements (ADR-0034 append-only Amendments 2026-07-04, spec updated) — an absent `RateLimit` directive now **fails closed** (not "defaults to `Global`"), closing the last silent diff --git a/crates/adapter/net/http/api/src/lib.rs b/crates/adapter/net/http/api/src/lib.rs index 47511e8..5f9f20a 100644 --- a/crates/adapter/net/http/api/src/lib.rs +++ b/crates/adapter/net/http/api/src/lib.rs @@ -20,6 +20,8 @@ //! `RequestTimeout` per-request override //! - [`trace`] — the `Tracing` layer and its `TracingLayer` factory (outermost; //! one span per request, secret-safe, routed to the ADR-0014 Telemetry plane) +//! - [`stack()`] — `HttpConfig` and the `stack()` assembly composing the canonical +//! resilience order (ADR-0031 §1) over any leaf (Slice 2) //! //! The resilience layers, `stack`/`build` assembly, and backends land in later //! slices. No async runtime, `hyper`, `reqwest`, or `serde` here. @@ -34,6 +36,7 @@ pub mod rate; pub mod rate_limit; pub mod retry; pub mod service; +pub mod stack; pub mod timeout; pub mod trace; @@ -49,5 +52,6 @@ pub use rate::{ pub use rate_limit::{RateLimit, RateLimitLayer, RateScope, Scope}; pub use retry::{Retry, RetryConfig, RetryLayer, Retryable}; pub use service::Service; +pub use stack::{HttpConfig, stack}; pub use timeout::{RequestTimeout, Timeout, TimeoutLayer}; pub use trace::{Tracing, TracingLayer}; diff --git a/crates/adapter/net/http/api/src/stack.rs b/crates/adapter/net/http/api/src/stack.rs new file mode 100644 index 0000000..0018a65 --- /dev/null +++ b/crates/adapter/net/http/api/src/stack.rs @@ -0,0 +1,550 @@ +//! The `stack()` assembly (ADR-0031 §1) + the non-generic `HttpConfig`. +//! +//! [`stack`] composes the canonical resilience order over any leaf: +//! `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( Auth( leaf ) ) ) ) ) ) )`. +//! It builds the one fallible layer ([`RateLimitLayer`], +//! which validates pacing coverage + the concurrency-singleton invariant) first, +//! so a coverage/param error is a [`BuildError`] before the +//! rest is assembled. `Auth`/`SetHeaders` are direct `Service` wrappers (no +//! `Layer` factory), so they pre-wrap the leaf; the five `Layer`-factory layers +//! compose over that via [`ServiceBuilder`] +//! (first `.layer()` = outermost). The composed value satisfies +//! [`HttpClient`] by blanket impl. + +use crate::rate::{BuildError, RateKey, RateLimitConfig}; +use crate::{ + Auth, AuthSource, CircuitBreakerConfig, CircuitBreakerLayer, HttpClient, RateLimitLayer, + RetryConfig, RetryLayer, SetHeaders, TimeoutLayer, TracingLayer, +}; +use oath_adapter_net_api::{ServiceBuilder, Timer}; +use std::fmt; +use std::time::Duration; + +/// Non-generic assembly configuration for [`stack`]. +/// +/// One field per configurable layer plus the static default headers. The +/// `K`-generic pacing map (`RateLimitConfig`), `auth`, and `timer` are +/// separate [`stack`] arguments, so this type carries no type parameter and +/// no `serde` (deserialisation stays in the adapter, ADR-0003). +#[derive(Debug, Clone)] +pub struct HttpConfig { + /// Per-attempt send timeout — bounds the send, **not** the permit wait. + pub timeout: Duration, + /// Retry policy (attempts, backoff schedule). + pub retry: RetryConfig, + /// Circuit-breaker thresholds and cooldowns. + pub circuit_breaker: CircuitBreakerConfig, + /// Static default request headers, stamped by `SetHeaders` (just outside `Auth`). + pub headers: http::HeaderMap, + /// Ceiling on how long an exhausted bucket back-pressures before the request + /// returns [`HttpError::Throttled`](crate::HttpError::Throttled). Distinct from + /// `timeout`: `RateLimit` sits **outside** `Timeout`, so the permit wait is + /// bounded by this — at IBKR's 1/15-min buckets, minutes not seconds. + pub rate_limit_max_wait: Duration, +} + +/// Assemble the canonical resilience stack (ADR-0031 §1) over an arbitrary leaf. +/// +/// Builds the fallible [`RateLimit`](crate::RateLimit) layer **first** — it runs +/// `validate_coverage` + `validate_concurrency_singleton` — so a config that is not +/// total over `K::all()`, carries an out-of-range policy param, or breaches the +/// ≤1-concurrency-permit invariant is a [`BuildError`] before the infallible layers +/// are assembled. Then composes, outermost-first: +/// `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( Auth( leaf ) ) ) ) ) ) )`. +/// `Auth`/`SetHeaders` are direct `Service` wrappers (no `Layer` factory), so they +/// pre-wrap the leaf; the composed value satisfies [`HttpClient`] by blanket impl. +/// +/// # Errors +/// [`BuildError`] propagated from `RateLimitLayer::new` if `rate_limits` is not +/// total over `K::all()`, any policy is out of range, or the concurrency-singleton +/// invariant is breached. +// `rate_limits` is only borrowed internally (`RateLimitLayer::new` takes `&RateLimitConfig`), +// but the public signature takes it by value to match `cfg`'s "config consumed once at boot" +// shape — the caller hands the whole aggregate over and is done with it. +#[allow(clippy::needless_pass_by_value)] +pub fn stack( + leaf: S, + cfg: HttpConfig, + timer: T, + auth: A, + rate_limits: RateLimitConfig, +) -> Result +where + S: HttpClient + Clone + Send + Sync + 'static, + S::Body: Send, + T: Timer + 'static, + A: AuthSource + 'static, + K: RateKey + fmt::Debug, +{ + // Fallible layer first: validates coverage + concurrency-singleton (fail-closed + // at construction — nothing else is built if this errors). + let rate = RateLimitLayer::new(&rate_limits, timer.clone(), cfg.rate_limit_max_wait)?; + // The two innermost layers are direct wrappers, not `Layer` factories. + let inner = SetHeaders::new(Auth::new(leaf, auth), cfg.headers); + let svc = ServiceBuilder::new() + .layer(TracingLayer::new(timer.clone())) // outermost + .layer(CircuitBreakerLayer::new(cfg.circuit_breaker, timer.clone())) + .layer(RetryLayer::new(cfg.retry, timer.clone())) + .layer(rate) + .layer(TimeoutLayer::new(cfg.timeout, timer)) // innermost Layer-factory + .service(inner); + Ok(svc) +} + +#[cfg(test)] +mod tests { + use super::{HttpConfig, stack}; + use crate::rate::{LimitDecl, LimitPolicy, RateLimitConfig}; + use crate::{ + AuthSource, BuildError, CircuitBreakerConfig, HttpError, NoAuth, RateKey, RateScope, + RetryConfig, Retryable, Scope, Service, + }; + use bytes::Bytes; + use http_body::{Body, Frame, SizeHint}; + use http_body_util::BodyExt; + use oath_adapter_net_api::Timer; + use oath_adapter_net_mock::MockTimer; + use std::collections::HashMap; + use std::future::Future; + use std::num::NonZeroU32; + use std::pin::Pin; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{Arc, Mutex}; + use std::task::{Context, Poll}; + use std::time::Duration; + + // ---- test RateKey ---------------------------------------------------- + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] + enum Key { + Snapshot, + History, + } + impl RateKey for Key { + fn all() -> &'static [Self] { + &[Self::Snapshot, Self::History] + } + } + + // ---- canned one-frame body (Data = Bytes, Error = HttpError) ---------- + #[derive(Debug)] + struct StubBody { + data: Option, + } + impl StubBody { + fn new(b: &'static [u8]) -> Self { + Self { + data: Some(Bytes::from_static(b)), + } + } + } + impl Body for StubBody { + type Data = Bytes; + type Error = HttpError; + fn poll_frame( + self: Pin<&mut Self>, + _: &mut Context<'_>, + ) -> Poll, HttpError>>> { + Poll::Ready(self.get_mut().data.take().map(|d| Ok(Frame::data(d)))) + } + fn is_end_stream(&self) -> bool { + self.data.is_none() + } + fn size_hint(&self) -> SizeHint { + self.data.as_ref().map_or_else( + || SizeHint::with_exact(0), + |d| SizeHint::with_exact(d.len() as u64), + ) + } + } + + // ---- scripted, recording, clock-aware inline leaf -------------------- + // One scripted outcome per attempt; repeats the last once exhausted. Records + // the `Authorization` header each call saw (for the Auth re-stamp test) and + // counts calls (for the untouched-leaf assertions). Inline, not `MockClient`, + // to avoid the net-http-mock -> net-http-api dev-dep cycle. + // `Err`/`Hang` drive the Task 2 full-stack tests (circuit-trip, send-timeout). + #[derive(Clone, Copy)] + enum Step { + Status(u16), + Err, // connection error (retryable) + Hang, // sleeps 1h on the shared clock (for the Timeout test) + } + #[derive(Clone)] + struct ScriptLeaf { + steps: Arc>, + calls: Arc, + seen_auth: Arc>>>, + timer: MockTimer, + } + impl ScriptLeaf { + fn new(timer: MockTimer, steps: Vec) -> Self { + Self { + steps: Arc::new(steps), + calls: Arc::new(AtomicUsize::new(0)), + seen_auth: Arc::new(Mutex::new(Vec::new())), + timer, + } + } + // Used by the Task 2 full-stack tests (call-count and Auth re-stamp assertions). + fn calls(&self) -> usize { + self.calls.load(Ordering::Relaxed) + } + fn seen_auth(&self) -> Vec> { + self.seen_auth.lock().unwrap().clone() + } + } + impl Service> for ScriptLeaf { + type Response = http::Response; + type Error = HttpError; + #[allow(clippy::manual_async_fn)] + fn call( + &self, + req: http::Request, + ) -> impl Future> + Send { + let i = self.calls.fetch_add(1, Ordering::Relaxed); + let step = self + .steps + .get(i) + .copied() + .unwrap_or_else(|| *self.steps.last().unwrap()); + let seen = req + .headers() + .get(http::header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + self.seen_auth.lock().unwrap().push(seen); + let timer = self.timer.clone(); + async move { + match step { + Step::Status(code) => { + let mut resp = http::Response::new(StubBody::new(b"ok")); + *resp.status_mut() = http::StatusCode::from_u16(code).unwrap(); + Ok(resp) + }, + Step::Err => Err(HttpError::connection("reset")), + Step::Hang => { + timer.sleep(Duration::from_secs(3600)).await; + Ok(http::Response::new(StubBody::new(b"late"))) + }, + } + } + } + } + + // ---- an AuthSource stamping a monotonically-increasing credential ----- + // Used by the Task 2 Auth-restamp-per-attempt test. + #[derive(Clone)] + struct CounterAuth { + n: Arc, + } + impl CounterAuth { + fn new() -> Self { + Self { + n: Arc::new(AtomicUsize::new(0)), + } + } + } + impl AuthSource for CounterAuth { + fn authorize( + &self, + req: &mut http::Request, + ) -> impl Future> + Send { + let n = self.n.fetch_add(1, Ordering::Relaxed); + let val = http::HeaderValue::from_str(&format!("token-{n}")).unwrap(); + req.headers_mut().insert(http::header::AUTHORIZATION, val); + std::future::ready(Ok(())) + } + } + + // ---- config builders -------------------------------------------------- + // Retry/circuit-breaker knobs tuned so pacing never accidentally interferes: + // a generous global bucket, zero backoff (retries run inline under MockTimer). + fn http_cfg(retry_attempts: u32, timeout: Duration, max_wait: Duration) -> HttpConfig { + HttpConfig { + timeout, + retry: RetryConfig { + max_attempts: NonZeroU32::new(retry_attempts).unwrap(), + base: Duration::ZERO, + cap: Duration::ZERO, + seed: 1, + }, + circuit_breaker: CircuitBreakerConfig { + failure_threshold: NonZeroU32::new(3).unwrap(), + cooldown: Duration::from_secs(30), + throttle_cooldown: Duration::from_secs(900), + half_open_probes: NonZeroU32::new(1).unwrap(), + }, + headers: http::HeaderMap::new(), + rate_limit_max_wait: max_wait, + } + } + // Global effectively unlimited; Snapshot 2/s; History concurrency 1. + fn rate_cfg() -> RateLimitConfig { + RateLimitConfig { + global: LimitPolicy::TokenBucket { + rate: 1000, + per: Duration::from_secs(1), + burst: 1000, + }, + local: HashMap::from([ + ( + Key::Snapshot, + LimitDecl::Policy(LimitPolicy::TokenBucket { + rate: 2, + per: Duration::from_secs(1), + burst: 2, + }), + ), + ( + Key::History, + LimitDecl::Policy(LimitPolicy::Concurrency { max: 1 }), + ), + ]), + } + } + fn req(scope: Scope, key: Option) -> http::Request { + let mut r = http::Request::builder() + .method("GET") + .uri("/x") + .body(Bytes::new()) + .unwrap(); + r.extensions_mut().insert(RateScope { scope, key }); + r.extensions_mut().insert(Retryable); // opt in so Retry engages when max_attempts > 1 + r + } + + // ---- Task 1 tests ----------------------------------------------------- + + #[tokio::test] + async fn plain_request_threads_all_layers_and_body_is_transparent() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(200)]); + let svc = stack( + leaf, + http_cfg(1, Duration::from_secs(30), Duration::ZERO), + timer, + NoAuth, + rate_cfg(), + ) + .expect("total config"); + let resp = svc.call(req(Scope::Global, None)).await.expect("200"); + assert_eq!(resp.status(), http::StatusCode::OK); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"ok")); // through all 7 layers + Guarded, untouched + } + + #[test] + fn missing_key_is_a_build_error_and_constructs_nothing() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(200)]); + let mut rc = rate_cfg(); + rc.local.remove(&Key::History); // no longer total over Key::all() + // `.unwrap_err()` would require `Debug` on the opaque `Ok` type, which the + // return bound (`impl HttpClient + Clone + Send + Sync + 'static`) deliberately + // omits; `let...else` extracts the error without needing it. + let Err(err) = stack( + leaf, + http_cfg(1, Duration::from_secs(30), Duration::ZERO), + timer, + NoAuth, + rc, + ) else { + panic!("expected a BuildError for a non-total rate config"); + }; + assert!(matches!(err, BuildError::UndeclaredKey(ref k) if k.contains("History"))); + } + + // ---- Task 2 tests ----------------------------------------------------- + + // 1. CircuitBreaker OUTSIDE Retry — an open circuit fast-rejects; the leaf is + // untouched and no retry loop spins on the rejection. If CB were INSIDE + // Retry this could not hold: the breaker would be re-consulted per attempt. + #[tokio::test] + async fn circuit_opens_and_fast_rejects_without_touching_the_leaf() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Err]); // always fails + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), // retry ON (3), zero backoff + timer, + NoAuth, + rate_cfg(), + ) + .expect("total config"); + // 3 logical failures (each retried 3x → 9 leaf calls) trip the breaker. + for _ in 0..3 { + let _ = svc.call(req(Scope::Global, None)).await; + } + let calls_after_trip = leaf.calls(); + assert_eq!( + calls_after_trip, 9, + "3 requests x 3 attempts reached the leaf before the trip" + ); + // Next request: circuit is Open → CircuitOpen, leaf untouched, no spin. + // `let...else` avoids needing `Debug` on the opaque `Ok` type (see + // `missing_key_is_a_build_error_and_constructs_nothing` above). + let Err(err) = svc.call(req(Scope::Global, None)).await else { + panic!("expected CircuitOpen from an open breaker"); + }; + assert!(matches!(err, HttpError::CircuitOpen)); + assert_eq!( + leaf.calls(), + 9, + "open circuit fast-rejects; leaf untouched, Retry never spun" + ); + } + + // 2. RateLimit INSIDE Retry — each attempt re-acquires budget. With a burst-1 + // bucket and zero max_wait, the first attempt drains it and the retry + // throttles at the (empty) bucket, so the leaf is hit exactly once. If + // RateLimit were OUTSIDE Retry, the single token would cover the whole + // logical request and the retry would resend to a 200. + #[tokio::test] + async fn rate_limit_is_spent_per_attempt_inside_retry() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(503), Step::Status(200)]); + // Snapshot: burst 1, refill 1/hour → no refill during the test. + let rc = RateLimitConfig { + global: LimitPolicy::TokenBucket { + rate: 1000, + per: Duration::from_secs(1), + burst: 1000, + }, + local: HashMap::from([ + ( + Key::Snapshot, + LimitDecl::Policy(LimitPolicy::TokenBucket { + rate: 1, + per: Duration::from_secs(3600), + burst: 1, + }), + ), + (Key::History, LimitDecl::GlobalOnly), + ]), + }; + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), + timer, + NoAuth, + rc, + ) + .expect("total config"); + // `let...else` avoids needing `Debug` on the opaque `Ok` type. + let Err(err) = svc.call(req(Scope::Local, Some(Key::Snapshot))).await else { + panic!("expected Throttled once the per-attempt bucket is drained"); + }; + assert!( + matches!(err, HttpError::Throttled), + "the retry re-acquired the drained bucket → per-attempt pacing (RateLimit inside Retry)" + ); + assert_eq!( + leaf.calls(), + 1, + "only attempt 1 reached the leaf; the retry throttled at the bucket" + ); + } + + // 3. Timeout bounds the SEND. A hanging leaf, with the clock advanced past the + // send timeout, yields Timeout. (RateLimit sits outside Timeout, so the + // permit wait is bounded separately by rate_limit_max_wait — structural.) + #[tokio::test] + async fn send_timeout_fires_on_a_hanging_leaf() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Hang]); + let svc = stack( + leaf, + http_cfg(1, Duration::from_secs(1), Duration::ZERO), // retry OFF, 1s send timeout + timer.clone(), + NoAuth, + rate_cfg(), + ) + .expect("total config"); + // `tokio::spawn` needs `F::Output: Send`, but the opaque `HttpClient::Body` + // carries no `Send` bound (`HttpClient::Body: http_body::Body<..>` only), + // so a real response body need not be `Send`. `spawn_local` under a + // `LocalSet` drives the same concurrent interleaving (task registers the + // sleep, then the timer fires it) without that bound. + let local = tokio::task::LocalSet::new(); + local + .run_until(async move { + let waiter = + tokio::task::spawn_local( + async move { svc.call(req(Scope::Global, None)).await }, + ); + tokio::task::yield_now().await; // task registers the inner sleep + the 1s deadline + timer.advance(Duration::from_secs(1)); // fire the send-timeout deadline + // `let...else` avoids needing `Debug` on the opaque `Ok` type. + let Err(err) = waiter.await.unwrap() else { + panic!("expected Timeout once the send deadline fires"); + }; + assert!(matches!(err, HttpError::Timeout)); + }) + .await; + } + + // 4. Auth re-stamps per attempt — inside Retry, so each of the N attempts + // carries a fresh credential. + #[tokio::test] + async fn auth_restamps_a_fresh_credential_on_every_attempt() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Err, Step::Err, Step::Status(200)]); + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), + timer, + CounterAuth::new(), + rate_cfg(), + ) + .expect("total config"); + let resp = svc + .call(req(Scope::Global, None)) + .await + .expect("third attempt is 200"); + assert_eq!(resp.status(), http::StatusCode::OK); + assert_eq!( + leaf.seen_auth(), + vec![ + Some("token-0".to_owned()), + Some("token-1".to_owned()), + Some("token-2".to_owned()) + ], + "Auth ran once per attempt (inside Retry), re-stamping a fresh credential each time" + ); + } + + // 5. Scope fail-closed end-to-end — a request with no RateScope extension is + // rejected before the leaf, and the fail-closed path survives composition. + #[tokio::test] + async fn absent_scope_is_rejected_fail_closed_through_the_stack() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(200)]); + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), + timer, + NoAuth, + rate_cfg(), + ) + .expect("total config"); + // A request with neither RateScope nor Retryable — the forgotten-stamp case. + let bare = http::Request::builder() + .method("GET") + .uri("/x") + .body(Bytes::new()) + .unwrap(); + // `let...else` avoids needing `Debug` on the opaque `Ok` type. + let Err(err) = svc.call(bare).await else { + panic!("expected fail-closed Throttled for a request with no RateScope"); + }; + assert!( + matches!(err, HttpError::Throttled), + "no RateScope → fail-closed Throttled" + ); + assert_eq!( + leaf.calls(), + 0, + "the fail-closed request never reached the leaf" + ); + } +} diff --git a/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md b/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md index 6fe794b..3ea5a88 100644 --- a/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md +++ b/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md @@ -260,3 +260,32 @@ carries the full reasoning. such span/field is active). Adds the `tracing` facade (runtime dep, zero executor) + `tracing-subscriber` (dev-dep). The module is named `trace` to avoid shadowing the `tracing` crate; the public types are `Tracing`/`TracingLayer`. +11. **`stack()` assembly + `HttpConfig` (Slice 2, assembly).** `stack()` + (`net-http-api`) assembles the canonical resilience order (ADR-0031 §1) over an + arbitrary leaf: `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( + Auth( leaf ) ) ) ) ) ) )`. It builds the one fallible layer (`RateLimitLayer::new`, + which runs `validate_coverage` + `validate_concurrency_singleton`) **first**, so a + coverage/param/singleton failure is a `BuildError` before the infallible layers are + assembled — `stack()` does **not** call `validate_coverage` separately. `Auth`/ + `SetHeaders` are direct `Service` wrappers (no `Layer` factory), so they pre-wrap + the leaf; the five `Layer`-factory layers compose over that via the kernel's + `ServiceBuilder`. The return bound is the full `impl HttpClient + Clone + Send + + Sync + 'static` (not bare `impl HttpClient`), so a `Send`/`Clone`/`'static` + regression in any layer is a compile error *at `stack()`*; `build()` (the following + hyper-backend slice) reuses this bound over the hyper leaf. `HttpConfig` is + non-generic plain data — `timeout`, `retry`, `circuit_breaker`, `headers`, and + `rate_limit_max_wait` (the permit-wait ceiling feeding `RateLimitLayer::new`, + distinct from the send `timeout` because `RateLimit` sits outside `Timeout`) — with + no type parameter and no `serde` (deserialisation stays in the adapter, ADR-0003). + The one generic pacing arg (`RateLimitConfig`), `auth`, and `timer` are separate + `stack()` parameters. **Bound refinement:** the spec sketch's `T: Timer, A: + AuthSource, K: RateKey` becomes `T: Timer + 'static, A: AuthSource + 'static, K: + RateKey + Debug` in the implementation, plus the leaf's `S::Body: Send` + (transitively required by `Retry`/`RateLimit`'s existing `B: Send` bound) — the + composed value is returned `'static`; coverage validation renders the offending + key. `BufferOrStream` is **not** a + layer here — buffering is a leaf-side body-construction concern, so the innermost + leaf already satisfies "inside `Retry`". Full-stack ordering invariants are + regression-tested over an inline recording leaf + `MockTimer` (not `MockClient`, + which would close the net-http-mock → net-http-api dev-dep cycle and cannot script + sequences). No new dependency; no existing-layer change. diff --git a/docs/superpowers/plans/2026-07-04-net-http-rate-coverage.md b/docs/superpowers/plans/2026-07-04-net-http-rate-coverage.md new file mode 100644 index 0000000..6f80817 --- /dev/null +++ b/docs/superpowers/plans/2026-07-04-net-http-rate-coverage.md @@ -0,0 +1,524 @@ +# net-http RateKey + RateLimitConfig + Boot-Time Coverage (Slice 0, PR 4) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Land the boot-time pacing-coverage contract in `oath-adapter-net-http-api` — the `RateKey` trait, the `LimitPolicy`/`LimitDecl` vocabulary, the total `RateLimitConfig` map, `BuildError`, and the standalone `validate_coverage` validator — so a missing or ill-configured pacing bucket is a **boot failure**, not a first-live-order 429 → IBKR penalty box. This closes Slice 0. + +**Architecture:** `RateKey` is a trait whose implementors (an adapter's endpoint enum) expose a **finite universe** via `fn all() -> &'static [Self]`. `RateLimitConfig` is a **total** map: every `K::all()` variant must be *explicitly classified* (`LimitDecl::Policy` or `LimitDecl::GlobalOnly` — never "absent"), plus a required `global` policy. `validate_coverage` is the pure construction-time check (totality + param sanity) returning `Result<(), BuildError>`; Slice 2's `stack()`/`build()` will call it, but it is complete and unit-tested standalone here. No layer, no `RateLimit` runtime, no `Scope` request-surface enforcement (those are Slice 1). Pure data + one validator — no async, no new dependency. + +**Tech Stack:** Rust (edition 2024, MSRV 1.90), `just`, `std::collections::HashMap`, `thiserror` (already a crate dependency). **No new dependency** — `net-http-api` stays runtime-free (no `tokio`/`hyper`/`reqwest`/`serde`). + +**Source spec:** [docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md](../specs/2026-06-30-net-http-construction-surface-design.md) §"`RateKey` — a typed enum with a finite universe" / §"Boot-time total pacing coverage" (lines ~298–363), recorded in [ADR-0034 §3](../../adr/0034-http-construction-surface-auth-guarded-boot-coverage.md). Roadmapped as **PR 4** in [2026-06-30-net-http-foundation.md](2026-06-30-net-http-foundation.md) (lines 435–441). + +**Depends on PR 3 having merged** (it has — #66): PR 4 adds a new module to the same crate; it consumes only `thiserror` and `std`. It does not consume PR 3's `AuthSource`/`Guarded`. + +## Decisions locked by this plan + +1. **Scope = config + validator only.** `RateKey`, `LimitPolicy`, `LimitDecl`, `RateLimitConfig`, `BuildError`, `validate_coverage`. **Deferred, correctly absent here:** the `RateLimit` *layer*, the per-request `Scope`/`RateLimit` extension and its call-site fail-closed (spec lines 342–357), `HttpConfig`, `stack()`/`build()` wiring — all Slice 1/Slice 2. This PR delivers the validator those slices call. + +2. **`BuildError` is non-generic with two variants — `UndeclaredKey(String)` and `InvalidPolicy(String)`.** The spec sketch (line 362) also lists "missing global", but `RateLimitConfig.global` is a **required, non-optional field** — "missing global" is structurally unrepresentable, so no `MissingGlobal` variant is added (a dead, unconstructible variant would be worse design and a `missing_docs`/clippy liability). The global policy's *params* are still validated, via the same `InvalidPolicy` path as any local policy. `BuildError` must be non-generic because Slice 2's `stack()`/`build()` return `Result<_, BuildError>` (spec lines 267/273), so the offending key is rendered to a `String`. + +3. **`RateKey`'s supertraits stay exactly as the spec defines them** — `Hash + Eq + Clone + Send + Sync + 'static` (no `Debug` added to the trait). Only `validate_coverage` needs to *render* an undeclared key, so it — not the trait — carries the `K: fmt::Debug` bound. `RateLimitConfig` derives `Debug` conditionally (`impl Debug`), which satisfies the workspace `missing_debug_implementations` lint without forcing `Debug` onto every `RateKey`. + +4. **`validate_coverage` is `pub`** — it is part of the boot-time contract (an adapter may pre-validate its pacing table), and Slice 2's `stack()`/`build()` (same crate) call it. + +5. **Drift-proofing lives in the *test* `RateKey`, not the trait.** Per the spec (lines 316–322) the production trait stays dependency-free; the *adapter* owns `all()` exhaustiveness (via `strum::VariantArray` or an exhaustive-`match` test). PR 4's test key uses the dependency-free exhaustive-`match` guard so the pattern is demonstrated and pinned. + +## Global Constraints + +Every task implicitly includes these: + +- **Edition 2024, MSRV 1.90.** No `unsafe` (`unsafe_code = "deny"`; the crate is `#![forbid(unsafe_code)]`). +- **No `unwrap`/`expect`/indexing/panic in non-test code** — return `Result`. Test code is exempt for `unwrap`/`expect`/indexing only. +- **`just lint` runs clippy with `-D warnings` and promotes `pedantic`/`nursery`** — all code including tests must be pedantic-clean: `#[must_use]` where clippy asks, document all public items (`missing_docs`), `Debug` on all public types (`missing_debug_implementations`), no unreachable `pub`, `const fn` where nursery's `missing_const_for_fn` asks. Compare unsigned to `== 0`, not `< 1` (clippy). +- **`net-http-api` charter:** free of any async *runtime* — no `tokio`/`hyper`/`reqwest`/`serde`. **This PR adds no dependency** (`thiserror` and `std` only), so `cargo-machete` and `cargo-deny` are unaffected. +- **DoD per PR:** `just ci` green (fmt, lint, test + doctests, doc, deny, typos, machete). Update `CHANGELOG.md` `[Unreleased]`. One issue → one branch → worktree under `.claude/worktrees/` → one PR (`Closes #`). + +--- + +## File Structure + +- `crates/adapter/net/http/api/src/rate.rs` — **new.** `RateKey`, `LimitPolicy`, `LimitDecl`, `RateLimitConfig`, `BuildError`, `validate_coverage`, and the unit tests. +- `crates/adapter/net/http/api/src/lib.rs` — **modify.** `pub mod rate;` + re-exports + module-doc bullet. +- `CHANGELOG.md` — **modify.** `[Unreleased] → Added`. + +No `Cargo.toml` change — no new dependency. Each task is one commit; the tasks together are one PR/issue. + +--- + +## Setup: issue + worktree + +- [ ] **Create the issue and the isolated worktree** + +```bash +gh issue create \ + --title "feat(net): RateKey + RateLimitConfig + boot-time coverage (Slice 0, PR 4)" \ + --label enhancement \ + --body "Slice 0 PR 4 of the net-http construction surface (spec: docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md, plan: docs/superpowers/plans/2026-07-04-net-http-rate-coverage.md), closing Slice 0: + +- \`RateKey\` trait (finite universe via \`all()\`) + \`LimitPolicy\` + \`LimitDecl\` + total \`RateLimitConfig\` +- \`BuildError\` + \`validate_coverage\`: a config missing a \`K\` variant, or with a bad policy param, is a boot failure (ADR-0034 §3) +- No new dependency; no runtime; validator is unit-tested standalone for Slice 2's \`stack()\`/\`build()\` to call" +``` + +Note the issue number `#` for the PR body. + +```bash +git worktree add .claude/worktrees/net-http-rate-coverage -b feat/net-http-rate-coverage main +cd .claude/worktrees/net-http-rate-coverage +``` + +All subsequent tasks run inside this worktree. + +--- + +## Task 4.1: `RateKey` + `LimitPolicy` + `LimitDecl` + `RateLimitConfig` — the vocabulary + +**Files:** +- Create: `crates/adapter/net/http/api/src/rate.rs` +- Modify: `crates/adapter/net/http/api/src/lib.rs` + +**Interfaces:** +- Consumes: `std::collections::HashMap`, `std::hash::Hash`. +- Produces: + - `oath_adapter_net_http_api::RateKey` — `trait RateKey: Hash + Eq + Clone + Send + Sync + 'static { fn all() -> &'static [Self] where Self: Sized; }`. + - `oath_adapter_net_http_api::LimitPolicy` — `#[non_exhaustive] enum { TokenBucket { rate: u32, burst: u32 }, Concurrency { max: u32 } }` (`Copy`). + - `oath_adapter_net_http_api::LimitDecl` — `#[non_exhaustive] enum { Policy(LimitPolicy), GlobalOnly }` (`Copy`). + - `oath_adapter_net_http_api::RateLimitConfig` — `struct { pub global: LimitPolicy, pub local: HashMap }`. + - Task 4.2 adds `BuildError` + `validate_coverage` to this module and consumes exactly these types. + +- [ ] **Step 1: Write the failing test** + +Create `crates/adapter/net/http/api/src/rate.rs` with the module doc and only the tests, and add `pub mod rate;` + `pub use rate::{LimitDecl, LimitPolicy, RateKey, RateLimitConfig};` + the module-doc bullet (Step 3) to `lib.rs`: + +```rust +//! Boot-time pacing coverage: the `RateKey` universe, the `LimitPolicy`/ +//! `LimitDecl` classification vocabulary, the total `RateLimitConfig` map, +//! and the `validate_coverage` construction-time check (ADR-0034 §3). +//! +//! A `RateLimitConfig` is **total**: every `K::all()` variant must be +//! explicitly classified — `LimitDecl::Policy` or `LimitDecl::GlobalOnly`, +//! never "absent". A missing or ill-configured bucket is caught at +//! construction ([`validate_coverage`]), so it is a boot failure rather than a +//! first-live-order 429 → 15-minute IBKR penalty box. This module is pure data +//! + one validator; the `RateLimit` layer that consumes it lands in Slice 1. + +#[cfg(test)] +mod tests { + use super::{LimitDecl, LimitPolicy, RateKey, RateLimitConfig}; + use std::collections::HashMap; + + /// A stand-in endpoint key for the tests — the shape an adapter provides. + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] + enum TestKey { + PlaceOrder, + Snapshot, + History, + } + + impl RateKey for TestKey { + fn all() -> &'static [Self] { + &[Self::PlaceOrder, Self::Snapshot, Self::History] + } + } + + #[test] + fn rate_key_all_is_drift_proof() { + // Exhaustive `match` with no wildcard arm: adding a `TestKey` variant + // fails to compile HERE, forcing whoever adds it to also list it in + // `all()`; the length assertion catches a variant added to the enum + // but dropped from `all()`. + fn is_listed(k: TestKey) -> bool { + match k { + TestKey::PlaceOrder | TestKey::Snapshot | TestKey::History => true, + } + } + assert!(TestKey::all().iter().copied().all(is_listed)); + assert_eq!(TestKey::all().len(), 3); + } + + #[test] + fn config_classifies_every_key_explicitly() { + let cfg = RateLimitConfig { + global: LimitPolicy::TokenBucket { rate: 10, burst: 20 }, + local: HashMap::from([ + (TestKey::PlaceOrder, LimitDecl::Policy(LimitPolicy::Concurrency { max: 1 })), + (TestKey::Snapshot, LimitDecl::Policy(LimitPolicy::TokenBucket { rate: 5, burst: 5 })), + (TestKey::History, LimitDecl::GlobalOnly), + ]), + }; + assert_eq!(cfg.local.len(), 3); + assert_eq!(cfg.global, LimitPolicy::TokenBucket { rate: 10, burst: 20 }); + assert_eq!(cfg.local[&TestKey::History], LimitDecl::GlobalOnly); + } +} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `just check` +Expected: FAIL — `cannot find type RateKey` / `LimitPolicy` / `LimitDecl` / `RateLimitConfig`. + +- [ ] **Step 3: Implement the vocabulary** + +Insert between the module doc and the tests in `rate.rs`: + +```rust +use std::collections::HashMap; +use std::hash::Hash; + +/// An adapter's rate-limit key with a **finite universe** — the enumeration +/// that makes the boot-time coverage check possible (ADR-0034 §3). +/// +/// `Clone` is doubly-earned: `http::Extensions::insert` demands it, and `Retry` +/// clones the request per attempt (Slice 1), so a stamped key survives replay. +/// The universe is kept generic (not erased to `u32`/`&str`) precisely so +/// [`validate_coverage`] can iterate every variant. +pub trait RateKey: Hash + Eq + Clone + Send + Sync + 'static { + /// Every key in the universe. Its exhaustiveness is what the coverage check + /// trusts; an adapter keeps it drift-proof (`strum::VariantArray` or an + /// exhaustive-`match` test), keeping this trait dependency-free. + fn all() -> &'static [Self] + where + Self: Sized; +} + +/// A single pacing policy applied to one scope. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum LimitPolicy { + /// A refilling token bucket: `rate` tokens/second, up to `burst` in hand. + TokenBucket { + /// Steady-state tokens per second (must be `>= 1`). + rate: u32, + /// Maximum tokens available at once (must be `>= 1`). + burst: u32, + }, + /// A concurrency cap: at most `max` in-flight requests in this scope. + Concurrency { + /// Maximum concurrent requests (must be `>= 1`). + max: u32, + }, +} + +/// How one endpoint is paced — an **explicit** classification. There is no +/// "absent" arm: totality (every [`RateKey`] variant classified) is what the +/// boot check enforces. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum LimitDecl { + /// This endpoint has its own local policy (in addition to the global one). + Policy(LimitPolicy), + /// This endpoint is paced by the global policy only — declared on purpose. + GlobalOnly, +} + +/// A **total** pacing configuration: a required `global` policy plus a +/// per-endpoint classification for every key in the [`RateKey`] universe. +/// +/// [`validate_coverage`] rejects a `local` map that is not total over +/// `K::all()`, so forgetting to pace a new endpoint is a boot failure. +#[derive(Debug, Clone)] +pub struct RateLimitConfig { + /// The account-wide policy every request is subject to. + pub global: LimitPolicy, + /// The per-endpoint classification. Must be total over `K::all()`. + pub local: HashMap, +} +``` + +In `lib.rs`, add the module and re-export (keep the existing alphabetical `pub mod` / `pub use` ordering — `rate` sits between `error` and `service`), and extend the module-doc list: + +```rust +//! - [`rate`] — `RateKey`, the `LimitPolicy`/`LimitDecl` vocabulary, the total +//! `RateLimitConfig`, and the boot-time `validate_coverage` check +``` + +```rust +pub mod rate; +``` + +```rust +pub use rate::{LimitDecl, LimitPolicy, RateKey, RateLimitConfig}; +``` + +(Task 4.2 extends this re-export to `pub use rate::{BuildError, LimitDecl, LimitPolicy, RateKey, RateLimitConfig, validate_coverage};`.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `just check && cargo test -p oath-adapter-net-http-api rate && just lint` +Expected: PASS, warning-free. + +- [ ] **Step 5: Commit** + +```bash +git add crates/adapter/net/http/api/src/rate.rs crates/adapter/net/http/api/src/lib.rs +git commit -m "feat(net): RateKey + LimitPolicy/LimitDecl + total RateLimitConfig" +``` + +--- + +## Task 4.2: `BuildError` + `validate_coverage` — the boot-time check + +**Files:** +- Modify: `crates/adapter/net/http/api/src/rate.rs`, `crates/adapter/net/http/api/src/lib.rs` + +**Interfaces:** +- Consumes: Task 4.1's `RateKey`, `LimitPolicy`, `LimitDecl`, `RateLimitConfig`; `std::fmt`. +- Produces: + - `oath_adapter_net_http_api::BuildError` — `#[non_exhaustive] enum { UndeclaredKey(String), InvalidPolicy(String) }` (`Debug`, `thiserror::Error`, `PartialEq`, `Eq`). + - `oath_adapter_net_http_api::validate_coverage` — `pub fn validate_coverage(cfg: &RateLimitConfig) -> Result<(), BuildError>`. Slice 2's `stack()`/`build()` call it before assembling layers. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `rate.rs` (extend the `use super::…` line to `use super::{BuildError, LimitDecl, LimitPolicy, RateKey, RateLimitConfig, validate_coverage};`): + +```rust + /// A total, param-sane config over `TestKey` — the baseline the negative + /// tests mutate. + fn total_config() -> RateLimitConfig { + RateLimitConfig { + global: LimitPolicy::TokenBucket { rate: 10, burst: 20 }, + local: HashMap::from([ + (TestKey::PlaceOrder, LimitDecl::Policy(LimitPolicy::Concurrency { max: 1 })), + (TestKey::Snapshot, LimitDecl::Policy(LimitPolicy::TokenBucket { rate: 5, burst: 5 })), + (TestKey::History, LimitDecl::GlobalOnly), + ]), + } + } + + #[test] + fn total_config_validates() { + assert_eq!(validate_coverage(&total_config()), Ok(())); + } + + #[test] + fn missing_key_is_undeclared() { + let mut cfg = total_config(); + cfg.local.remove(&TestKey::History); + let err = validate_coverage(&cfg).unwrap_err(); + assert!(matches!(err, BuildError::UndeclaredKey(ref k) if k.contains("History"))); + } + + #[test] + fn zero_rate_token_bucket_is_invalid() { + let mut cfg = total_config(); + cfg.local.insert( + TestKey::Snapshot, + LimitDecl::Policy(LimitPolicy::TokenBucket { rate: 0, burst: 5 }), + ); + assert!(matches!(validate_coverage(&cfg), Err(BuildError::InvalidPolicy(_)))); + } + + #[test] + fn zero_burst_token_bucket_is_invalid() { + let mut cfg = total_config(); + cfg.local.insert( + TestKey::Snapshot, + LimitDecl::Policy(LimitPolicy::TokenBucket { rate: 5, burst: 0 }), + ); + assert!(matches!(validate_coverage(&cfg), Err(BuildError::InvalidPolicy(_)))); + } + + #[test] + fn zero_concurrency_max_is_invalid() { + let mut cfg = total_config(); + cfg.local.insert( + TestKey::PlaceOrder, + LimitDecl::Policy(LimitPolicy::Concurrency { max: 0 }), + ); + assert!(matches!(validate_coverage(&cfg), Err(BuildError::InvalidPolicy(_)))); + } + + #[test] + fn bad_global_policy_is_invalid() { + let mut cfg = total_config(); + cfg.global = LimitPolicy::TokenBucket { rate: 0, burst: 1 }; + assert!(matches!(validate_coverage(&cfg), Err(BuildError::InvalidPolicy(_)))); + } + + #[test] + fn global_only_endpoints_need_no_local_params() { + // A `GlobalOnly` decl carries no policy, so it is always coverage-valid + // (it is paced by the already-validated global). + let cfg = RateLimitConfig { + global: LimitPolicy::Concurrency { max: 2 }, + local: HashMap::from([ + (TestKey::PlaceOrder, LimitDecl::GlobalOnly), + (TestKey::Snapshot, LimitDecl::GlobalOnly), + (TestKey::History, LimitDecl::GlobalOnly), + ]), + }; + assert_eq!(validate_coverage(&cfg), Ok(())); + } +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `just check` +Expected: FAIL — `cannot find type BuildError` / `cannot find function validate_coverage`. + +- [ ] **Step 3: Implement `BuildError`, `LimitPolicy::validate`, and `validate_coverage`** + +Add `use std::fmt;` to the imports at the top of `rate.rs`. Add the `validate` method in an `impl LimitPolicy` block below the enum, and append `BuildError` + `validate_coverage` after `RateLimitConfig`: + +```rust +impl LimitPolicy { + /// Reject non-sensical policy parameters (ADR-0034 §3 / spec: `rate == 0`, + /// `burst == 0`, `max == 0`). + fn validate(self) -> Result<(), BuildError> { + match self { + Self::TokenBucket { rate, burst } => { + if rate == 0 { + return Err(BuildError::InvalidPolicy(format!( + "token-bucket rate must be >= 1, got {rate}" + ))); + } + if burst == 0 { + return Err(BuildError::InvalidPolicy(format!( + "token-bucket burst must be >= 1, got {burst}" + ))); + } + Ok(()) + } + Self::Concurrency { max } => { + if max == 0 { + return Err(BuildError::InvalidPolicy(format!( + "concurrency max must be >= 1, got {max}" + ))); + } + Ok(()) + } + } + } +} + +/// A construction-time pacing-config failure — the boot-time guard that turns a +/// missing or nonsensical bucket into a startup error instead of a live 429 +/// (ADR-0034 §3). Non-generic: the offending key is rendered to a `String` so +/// `stack()`/`build()` can return `Result<_, BuildError>` regardless of `K`. +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[non_exhaustive] +pub enum BuildError { + /// A [`RateKey`] variant is not classified in `local` — the map is not + /// total over `K::all()`. + #[error("rate-limit key `{0}` is not classified in the config (every RateKey::all() variant must be declared)")] + UndeclaredKey(String), + /// A policy carries out-of-range parameters (`rate`/`burst`/`max` of 0). + #[error("invalid rate-limit policy: {0}")] + InvalidPolicy(String), +} + +/// Validate that `cfg` is a **total**, param-sane pacing configuration: the +/// `global` policy is valid, and every [`RateKey`] variant is classified with a +/// valid policy (ADR-0034 §3). Slice 2's `stack()`/`build()` call this before +/// assembling the stack, so a coverage gap is a boot failure. +/// +/// # Errors +/// [`BuildError::UndeclaredKey`] if a `K::all()` variant is absent from +/// `cfg.local`; [`BuildError::InvalidPolicy`] if the global or any local policy +/// has an out-of-range parameter. +pub fn validate_coverage(cfg: &RateLimitConfig) -> Result<(), BuildError> +where + K: RateKey + fmt::Debug, +{ + cfg.global.validate()?; + for key in K::all() { + match cfg.local.get(key) { + None => return Err(BuildError::UndeclaredKey(format!("{key:?}"))), + Some(LimitDecl::Policy(policy)) => policy.validate()?, + Some(LimitDecl::GlobalOnly) => {} + } + } + Ok(()) +} +``` + +Extend the `lib.rs` re-export to `pub use rate::{BuildError, LimitDecl, LimitPolicy, RateKey, RateLimitConfig, validate_coverage};`. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `just check && cargo test -p oath-adapter-net-http-api rate && just lint` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add crates/adapter/net/http/api/src/rate.rs crates/adapter/net/http/api/src/lib.rs +git commit -m "feat(net): BuildError + validate_coverage — boot-time pacing coverage" +``` + +--- + +## Task 4.3: CHANGELOG, full gate, PR + +**Files:** +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: CHANGELOG** + +Add to `CHANGELOG.md` `[Unreleased] → Added` (after the PR 3 construction-seams entry): + +```markdown +- `oath-adapter-net-http-api` boot-time pacing coverage — the `RateKey` trait + (finite universe via `all()`), the `LimitPolicy`/`LimitDecl` classification + vocabulary, the total `RateLimitConfig` map, `BuildError`, and the + standalone `validate_coverage` check: an unclassified endpoint or an + out-of-range policy param is a boot failure, not a first-live-order 429 + (ADR-0034 §3). Closes Slice 0 of the net-http construction surface. +``` + +- [ ] **Step 2: Full local gate** + +Run: `just ci` +Expected: green (fmt, lint, test + doctests, doc, deny, typos, machete — no new dep, so `deny`/`machete` are unaffected). + +- [ ] **Step 3: Commit, push, PR** + +```bash +git add CHANGELOG.md +git commit -m "docs(changelog): net-http boot-time pacing coverage (Slice 0 PR 4)" +git push -u origin feat/net-http-rate-coverage +gh pr create \ + --title "feat(net): RateKey + RateLimitConfig + boot-time coverage (Slice 0 PR 4)" \ + --body "Closes # + +Slice 0 **PR 4** of the net-http construction surface (spec: docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md) — **closes Slice 0**. + +- **\`RateKey\`** — the adapter's endpoint key with a finite universe (\`fn all() -> &'static [Self]\`), kept generic so the coverage check can iterate every variant. +- **\`LimitPolicy\`** (\`TokenBucket { rate, burst }\` / \`Concurrency { max }\`) + **\`LimitDecl\`** (\`Policy\` / \`GlobalOnly\`) — explicit per-endpoint classification; there is no \"absent\" arm. +- **\`RateLimitConfig\`** — a total map (required \`global\` + \`local\` over \`K::all()\`). +- **\`BuildError\`** + **\`validate_coverage\`** — the pure construction-time check: an unclassified key → \`UndeclaredKey\`; an out-of-range \`rate\`/\`burst\`/\`max\` → \`InvalidPolicy\`. A missing bucket is a boot failure, not a 15-minute IBKR penalty box (ADR-0034 §3). + +No new dependency; no runtime; no layer. Slice 2's \`stack()\`/\`build()\` will call \`validate_coverage\` before assembling the stack. + +Next: **Slice 1** — the resilience layers (\`Retry\`, \`RateLimit\` (consumes this config + constructs \`Guarded\`), \`CircuitBreaker\`, \`Tracing\`), needing \`MockTimer\`-driven timing tests and the per-request \`Scope\`/\`RateLimit\` extension. + +🤖 Generated with [Claude Code](https://claude.com/claude-code)" +``` + +Expected: PR open, GitHub Actions CI green (same `just ci` + MSRV job). + +--- + +## Self-Review + +**Spec coverage (PR 4 roadmap, foundation plan lines 435–441 + spec lines 298–363):** +- `RateKey` trait, exact spec signature (`Hash + Eq + Clone + Send + Sync + 'static`, `fn all() -> &'static [Self] where Self: Sized`) — Task 4.1. ✅ +- `LimitPolicy { TokenBucket { rate, burst }, Concurrency { max } }` — Task 4.1. ✅ +- `LimitDecl { Policy(LimitPolicy), GlobalOnly }` — Task 4.1. ✅ +- `RateLimitConfig { global: LimitPolicy, local: HashMap }` — Task 4.1. ✅ +- `BuildError` (`thiserror`; `UndeclaredKey`, bad-policy-params) — Task 4.2. "missing-global" intentionally omitted (Decision 2: `global` is a required field, structurally unrepresentable). ✅ +- `validate_coverage`: `local` total over `K::all()`, `global` present, param sanity (`rate/burst/max >= 1`) → `Result<(), BuildError>` — Task 4.2. ✅ +- Tests: missing `K` → `Err(UndeclaredKey)`; total → `Ok`; bad params (local + global) → `Err`; test `RateKey` with an exhaustive-`match` drift guard — Tasks 4.1/4.2. ✅ +- Deferred (correctly absent): the `RateLimit` layer, per-request `Scope`/`RateLimit` extension + call-site fail-closed (spec lines 342–357), `HttpConfig`, `stack()`/`build()` — Slice 1/Slice 2 (Decision 1). ✅ + +**Placeholder scan:** none — every code step carries the actual code, every run step the actual command. + +**Type consistency:** `RateKey::all()` signature identical in the trait def (Task 4.1) and the `TestKey` impl (Task 4.1 tests); `LimitPolicy`/`LimitDecl` variant names identical across the definitions, `total_config()` (Task 4.2), and every test; `RateLimitConfig` field names (`global`, `local`) consistent between the struct, both tasks' tests, and `validate_coverage`; `validate_coverage(&RateLimitConfig) -> Result<(), BuildError>` matches the `Interfaces` block and all call sites; `BuildError::{UndeclaredKey, InvalidPolicy}` consistent between the enum, `LimitPolicy::validate`, `validate_coverage`, and the tests; `lib.rs` re-exports accumulate to `pub use rate::{BuildError, LimitDecl, LimitPolicy, RateKey, RateLimitConfig, validate_coverage};`. + +**Known risks to watch during impl:** +- Clippy nursery `missing_const_for_fn` may ask `LimitPolicy::validate` to be `const` — it can't (`format!` + `String`), so no action; if clippy asks for `const` on a genuinely-const fn elsewhere, add it. +- `#[non_exhaustive]` + `#[derive(PartialEq, Eq)]` on `BuildError` is fine within-crate (tests `matches!`/`assert_eq!` on it); external exhaustive matching is intentionally blocked. +- `format!("{key:?}")` requires `K: fmt::Debug` — bounded on `validate_coverage` only (Decision 3), so the `RateKey` trait stays exactly as the spec defines it. +- `RateLimitConfig` derives `Debug` conditionally; the workspace `missing_debug_implementations` lint is satisfied by the derive without forcing `Debug` onto `RateKey`. diff --git a/docs/superpowers/plans/2026-07-05-net-http-stack-assembly.md b/docs/superpowers/plans/2026-07-05-net-http-stack-assembly.md new file mode 100644 index 0000000..4ce542f --- /dev/null +++ b/docs/superpowers/plans/2026-07-05-net-http-stack-assembly.md @@ -0,0 +1,734 @@ +# net-http `stack()` assembly + `HttpConfig` (Slice 2) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Build the runtime-free assembly for the net-http resilience stack — the non-generic `HttpConfig` aggregate and the `stack()` function that validates pacing coverage and composes the ADR-0031 canonical layer order over any leaf — plus the full-stack ordering-invariant tests that only an assembly makes possible. + +**Architecture:** `stack()` builds the one fallible layer (`RateLimit`, which validates coverage + concurrency-singleton) first, pre-wraps the leaf with the two direct `Service` wrappers (`Auth`, `SetHeaders`), then composes the five `Layer`-factory layers over that via the kernel's `ServiceBuilder` (first `.layer()` = outermost). The composed value auto-satisfies `HttpClient` via blanket impl; the return bound `impl HttpClient + Clone + Send + Sync + 'static` turns any layer regression into a compile error *at `stack()`*. No new dependency; no existing layer changes. + +**Tech Stack:** Rust (edition 2024, MSRV 1.90), `just`, the kernel's `ServiceBuilder`/`Layer`/`Timer` (`oath-adapter-net-api`), the already-shipped layers/config of `oath-adapter-net-http-api`. Tests use inline service doubles + `MockTimer` (`oath-adapter-net-mock`), driven on `tokio` (dev-only). + +## Global Constraints + +Every task implicitly includes these: + +- **Edition 2024, MSRV 1.90.** No `unsafe` — the crate is `#![forbid(unsafe_code)]`. +- **No `unwrap`/`expect`/indexing/panic and no truncating `as` casts in non-test code** — return `Result`. Test code is exempt for `unwrap`/`expect`/indexing. +- **`just lint` = clippy `-D warnings` + `pedantic`/`nursery`** — `#[must_use]` where asked, document all public items (`missing_docs`), `Debug` on all **public** types (`missing_debug_implementations`), `const fn` where `missing_const_for_fn` asks. +- **`just doc` per task** — `just check`/`lint`/`test` do **not** catch broken rustdoc intra-doc links; every task's verify step runs `just doc`. +- **`net-http-api` charter:** no async *runtime* — no `tokio`/`hyper`/`reqwest`/`serde` in non-dev deps. **This slice adds no dependency at all** (prod or dev), so `deny`/`machete` are unaffected. +- **net-http-api tests must NOT dev-depend on `oath-adapter-net-http-mock` (`MockClient`)** — it normal-depends on this crate, so a dev-dep closes a cycle that recompiles a second, non-unifying copy. Use **inline** service doubles + `oath-adapter-net-mock`'s `MockTimer`, exactly as `retry.rs`/`circuit_breaker.rs`/`rate_limit.rs` do. +- **DoD per PR:** `just ci` green (fmt, lint, test + doctests, doc, deny, typos, machete). Update `CHANGELOG.md` `[Unreleased]`. One issue → one branch → worktree → one PR (`Closes #`). + +## Source spec + +[docs/superpowers/specs/2026-07-05-net-http-stack-assembly-design.md](../specs/2026-07-05-net-http-stack-assembly-design.md), governed by [ADR-0031 §1](../../adr/0031-http-resilience-venue-pacing.md) (canonical layer order), the [construction-surface spec](../specs/2026-06-30-net-http-construction-surface-design.md) Seam #3, and [ADR-0034](../../adr/0034-http-construction-surface-auth-guarded-boot-coverage.md). This is **Slice 2 (assembly)**, runtime-free half; the hyper leaf + `build()` + `TokioTimer` are the following slice. Everything `stack()` composes already ships: all five layers (#76/#78/#82/#85/#86), `Auth`/`SetHeaders`/`Guarded` (#66), `RateScope`/`Scope` (#76), and `RateKey`/`RateLimitConfig`/`validate_coverage` (#72). + +## File Structure + +- `crates/adapter/net/http/api/src/stack.rs` — **new** (Tasks 1–2). `HttpConfig`, `stack()`, and the full test module (inline leaf + `CounterAuth` + test `RateKey` + all seven tests). +- `crates/adapter/net/http/api/src/lib.rs` — **modify** (Task 1). `pub mod stack;` + re-exports + module-doc bullet. +- `docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md`, `CHANGELOG.md` — **modify** (Task 3). + +No `Cargo.toml` change — no new dependency. Each task is one or more commits; the tasks together are one PR/issue. + +--- + +## Setup: issue (worktree already exists) + +> The isolated worktree **already exists** at `.claude/worktrees/net-http-stack-assembly` (branch `feat/net-http-stack-assembly`, branched off `main`), and already holds the design spec + this plan (commit `docs(net): Slice 2 …`). All tasks run inside it. Only the GitHub issue remains. + +- [ ] **Create the issue** + +```bash +gh issue create \ + --title "feat(net): stack() assembly + HttpConfig (Slice 2)" \ + --label enhancement \ + --body "Slice 2 (assembly, runtime-free) of the net-http construction surface (spec: docs/superpowers/specs/2026-07-05-net-http-stack-assembly-design.md; ADR-0031 §1, ADR-0034). + +- \`HttpConfig\` — non-generic aggregate (timeout, retry, circuit_breaker, headers, rate_limit_max_wait). +- \`stack()\` — validates pacing coverage (via \`RateLimitLayer::new\`), then composes the canonical order \`Tracing(CircuitBreaker(Retry(RateLimit(Timeout(SetHeaders(Auth(leaf)))))))\` over any leaf; returns \`Result\`. +- Full-stack ordering-invariant / boot-coverage / fail-closed tests over an inline recording leaf + \`MockTimer\` (no \`MockClient\` — dev-dep cycle). + +No new dependency; no runtime; no existing-layer changes. The hyper leaf + \`build()\` are the following slice." +``` + +Note the issue number `#` for the PR body. + +--- + +## Task 1: `HttpConfig` + `stack()` — compose, validate, and prove it type-checks + +**Files:** +- Create: `crates/adapter/net/http/api/src/stack.rs` +- Modify: `crates/adapter/net/http/api/src/lib.rs` + +**Interfaces:** +- Consumes (all crate-local, already shipped): `HttpClient` (`client`); `Auth`, `SetHeaders`, `AuthSource`, `NoAuth` (`auth`); `TracingLayer` (`trace`); `CircuitBreakerLayer`, `CircuitBreakerConfig` (`circuit_breaker`); `RetryLayer`, `RetryConfig` (`retry`); `RateLimitLayer` (`rate_limit`); `TimeoutLayer` (`timeout`); `RateLimitConfig`, `RateKey`, `BuildError` (`rate`); and `ServiceBuilder`, `Layer`, `Timer` (`oath_adapter_net_api`). +- Produces: + - `oath_adapter_net_http_api::HttpConfig` — `#[derive(Debug, Clone)] struct { pub timeout: Duration, pub retry: RetryConfig, pub circuit_breaker: CircuitBreakerConfig, pub headers: http::HeaderMap, pub rate_limit_max_wait: Duration }`. + - `oath_adapter_net_http_api::stack` — `pub fn stack(leaf: S, cfg: HttpConfig, timer: T, auth: A, rate_limits: RateLimitConfig) -> Result where S: HttpClient + Clone + Send + Sync + 'static, T: Timer + 'static, A: AuthSource + 'static, K: RateKey + std::fmt::Debug`. + +> **Bound note (deviation from the spec sketch).** The spec sketch wrote `T: Timer, A: AuthSource, K: RateKey`. The real bounds add `+ 'static` on `T`/`A` (the composed value is returned as `'static`, and `Timer`/`AuthSource` are not `'static` by default) and `+ std::fmt::Debug` on `K` (`RateLimitLayer::new` renders the offending key when coverage fails). These are necessary, mechanical additions — record them in the ADR amendment (Task 3). + +- [ ] **Step 1: Write the failing tests (module scaffolding + smoke + boot-coverage)** + +Create `crates/adapter/net/http/api/src/stack.rs` with the module doc and **only** the test module below (the `HttpConfig`/`stack` items land in Step 4, so this compiles to a failure until then): + +```rust +//! The `stack()` assembly (ADR-0031 §1) + the non-generic `HttpConfig`. +//! +//! [`stack`] composes the canonical resilience order over any leaf: +//! `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( Auth( leaf ) ) ) ) ) ) )`. +//! It builds the one fallible layer ([`RateLimitLayer`](crate::RateLimitLayer), +//! which validates pacing coverage + the concurrency-singleton invariant) first, +//! so a coverage/param error is a [`BuildError`](crate::BuildError) before the +//! rest is assembled. `Auth`/`SetHeaders` are direct `Service` wrappers (no +//! `Layer` factory), so they pre-wrap the leaf; the five `Layer`-factory layers +//! compose over that via [`ServiceBuilder`](oath_adapter_net_api::ServiceBuilder) +//! (first `.layer()` = outermost). The composed value satisfies +//! [`HttpClient`](crate::HttpClient) by blanket impl. + +#[cfg(test)] +mod tests { + use super::{stack, HttpConfig}; + use crate::rate::{LimitDecl, LimitPolicy, RateLimitConfig}; + use crate::{ + AuthSource, BuildError, CircuitBreakerConfig, HttpError, NoAuth, RateKey, RateScope, + Retryable, RetryConfig, Scope, Service, + }; + use bytes::Bytes; + use http_body::{Body, Frame, SizeHint}; + use http_body_util::BodyExt; + use oath_adapter_net_mock::MockTimer; + use std::collections::HashMap; + use std::future::Future; + use std::num::NonZeroU32; + use std::pin::Pin; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{Arc, Mutex}; + use std::task::{Context, Poll}; + use std::time::Duration; + + // ---- test RateKey ---------------------------------------------------- + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] + enum Key { + Snapshot, + History, + } + impl RateKey for Key { + fn all() -> &'static [Self] { + &[Self::Snapshot, Self::History] + } + } + + // ---- canned one-frame body (Data = Bytes, Error = HttpError) ---------- + #[derive(Debug)] + struct StubBody { + data: Option, + } + impl StubBody { + fn new(b: &'static [u8]) -> Self { + Self { data: Some(Bytes::from_static(b)) } + } + } + impl Body for StubBody { + type Data = Bytes; + type Error = HttpError; + fn poll_frame( + self: Pin<&mut Self>, + _: &mut Context<'_>, + ) -> Poll, HttpError>>> { + Poll::Ready(self.get_mut().data.take().map(|d| Ok(Frame::data(d)))) + } + fn is_end_stream(&self) -> bool { + self.data.is_none() + } + fn size_hint(&self) -> SizeHint { + self.data + .as_ref() + .map_or_else(|| SizeHint::with_exact(0), |d| SizeHint::with_exact(d.len() as u64)) + } + } + + // ---- scripted, recording, clock-aware inline leaf -------------------- + // One scripted outcome per attempt; repeats the last once exhausted. Records + // the `Authorization` header each call saw (for the Auth re-stamp test) and + // counts calls (for the untouched-leaf assertions). Inline, not `MockClient`, + // to avoid the net-http-mock -> net-http-api dev-dep cycle. + #[derive(Clone, Copy)] + enum Step { + Status(u16), + Err, // connection error (retryable) + Hang, // sleeps 1h on the shared clock (for the Timeout test) + } + #[derive(Clone)] + struct ScriptLeaf { + steps: Arc>, + calls: Arc, + seen_auth: Arc>>>, + timer: MockTimer, + } + impl ScriptLeaf { + fn new(timer: MockTimer, steps: Vec) -> Self { + Self { + steps: Arc::new(steps), + calls: Arc::new(AtomicUsize::new(0)), + seen_auth: Arc::new(Mutex::new(Vec::new())), + timer, + } + } + fn calls(&self) -> usize { + self.calls.load(Ordering::Relaxed) + } + fn seen_auth(&self) -> Vec> { + self.seen_auth.lock().unwrap().clone() + } + } + impl Service> for ScriptLeaf { + type Response = http::Response; + type Error = HttpError; + #[allow(clippy::manual_async_fn)] + fn call( + &self, + req: http::Request, + ) -> impl Future> + Send { + let i = self.calls.fetch_add(1, Ordering::Relaxed); + let step = self.steps.get(i).copied().unwrap_or_else(|| *self.steps.last().unwrap()); + let seen = req + .headers() + .get(http::header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + self.seen_auth.lock().unwrap().push(seen); + let timer = self.timer.clone(); + async move { + match step { + Step::Status(code) => { + let mut resp = http::Response::new(StubBody::new(b"ok")); + *resp.status_mut() = http::StatusCode::from_u16(code).unwrap(); + Ok(resp) + } + Step::Err => Err(HttpError::connection("reset")), + Step::Hang => { + timer.sleep(Duration::from_secs(3600)).await; + Ok(http::Response::new(StubBody::new(b"late"))) + } + } + } + } + } + + // ---- an AuthSource stamping a monotonically-increasing credential ----- + #[derive(Clone)] + struct CounterAuth { + n: Arc, + } + impl CounterAuth { + fn new() -> Self { + Self { n: Arc::new(AtomicUsize::new(0)) } + } + } + impl AuthSource for CounterAuth { + fn authorize( + &self, + req: &mut http::Request, + ) -> impl Future> + Send { + let n = self.n.fetch_add(1, Ordering::Relaxed); + let val = http::HeaderValue::from_str(&format!("token-{n}")).unwrap(); + req.headers_mut().insert(http::header::AUTHORIZATION, val); + std::future::ready(Ok(())) + } + } + + // ---- config builders -------------------------------------------------- + // Retry/circuit-breaker knobs tuned so pacing never accidentally interferes: + // a generous global bucket, zero backoff (retries run inline under MockTimer). + fn http_cfg(retry_attempts: u32, timeout: Duration, max_wait: Duration) -> HttpConfig { + HttpConfig { + timeout, + retry: RetryConfig { + max_attempts: NonZeroU32::new(retry_attempts).unwrap(), + base: Duration::ZERO, + cap: Duration::ZERO, + seed: 1, + }, + circuit_breaker: CircuitBreakerConfig { + failure_threshold: NonZeroU32::new(3).unwrap(), + cooldown: Duration::from_secs(30), + throttle_cooldown: Duration::from_secs(900), + half_open_probes: NonZeroU32::new(1).unwrap(), + }, + headers: http::HeaderMap::new(), + rate_limit_max_wait: max_wait, + } + } + // Global effectively unlimited; Snapshot 2/s; History concurrency 1. + fn rate_cfg() -> RateLimitConfig { + RateLimitConfig { + global: LimitPolicy::TokenBucket { rate: 1000, per: Duration::from_secs(1), burst: 1000 }, + local: HashMap::from([ + ( + Key::Snapshot, + LimitDecl::Policy(LimitPolicy::TokenBucket { + rate: 2, + per: Duration::from_secs(1), + burst: 2, + }), + ), + (Key::History, LimitDecl::Policy(LimitPolicy::Concurrency { max: 1 })), + ]), + } + } + fn req(scope: Scope, key: Option) -> http::Request { + let mut r = http::Request::builder().method("GET").uri("/x").body(Bytes::new()).unwrap(); + r.extensions_mut().insert(RateScope { scope, key }); + r.extensions_mut().insert(Retryable); // opt in so Retry engages when max_attempts > 1 + r + } + + // ---- Task 1 tests ----------------------------------------------------- + + #[tokio::test] + async fn plain_request_threads_all_layers_and_body_is_transparent() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(200)]); + let svc = stack(leaf, http_cfg(1, Duration::from_secs(30), Duration::ZERO), timer, NoAuth, rate_cfg()) + .expect("total config"); + let resp = svc.call(req(Scope::Global, None)).await.expect("200"); + assert_eq!(resp.status(), http::StatusCode::OK); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"ok")); // through all 7 layers + Guarded, untouched + } + + #[test] + fn missing_key_is_a_build_error_and_constructs_nothing() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(200)]); + let mut rc = rate_cfg(); + rc.local.remove(&Key::History); // no longer total over Key::all() + let err = stack(leaf, http_cfg(1, Duration::from_secs(30), Duration::ZERO), timer, NoAuth, rc) + .unwrap_err(); + assert!(matches!(err, BuildError::UndeclaredKey(ref k) if k.contains("History"))); + } +} +``` + +Then wire `lib.rs`. Add the module-doc bullet after the `trace` bullet (the `//! - [\`trace\`] …` block): + +```rust +//! - [`stack`] — `HttpConfig` and the `stack()` assembly composing the canonical +//! resilience order (ADR-0031 §1) over any leaf (Slice 2) +``` + +Add the module declaration alphabetically, between `pub mod service;` and `pub mod timeout;`: + +```rust +pub mod stack; +``` + +Add the re-export alphabetically, between `pub use service::Service;` and `pub use timeout::{…};`: + +```rust +pub use stack::{stack, HttpConfig}; +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `just check` +Expected: FAIL — `cannot find function stack` / `cannot find type HttpConfig` in module `stack` (only the test module exists). + +- [ ] **Step 3: (skipped — no separate red step)** + +The failing compile in Step 2 is the red. Proceed to implement. + +- [ ] **Step 4: Implement `HttpConfig` + `stack()`** + +Insert between the module doc and the `#[cfg(test)] mod tests` in `stack.rs`: + +```rust +use crate::rate::{BuildError, RateKey, RateLimitConfig}; +use crate::{ + Auth, AuthSource, CircuitBreakerConfig, CircuitBreakerLayer, HttpClient, RateLimitLayer, + RetryConfig, RetryLayer, SetHeaders, TimeoutLayer, TracingLayer, +}; +use oath_adapter_net_api::{Layer, ServiceBuilder, Timer}; +use std::fmt; +use std::time::Duration; + +/// Non-generic assembly configuration: one field per configurable layer plus the +/// static default headers. The `K`-generic pacing map (`RateLimitConfig`), +/// `auth`, and `timer` are separate [`stack`] arguments, so this type carries no +/// type parameter and no `serde` (deserialisation stays in the adapter, ADR-0003). +#[derive(Debug, Clone)] +pub struct HttpConfig { + /// Per-attempt send timeout — bounds the send, **not** the permit wait. + pub timeout: Duration, + /// Retry policy (attempts, backoff schedule). + pub retry: RetryConfig, + /// Circuit-breaker thresholds and cooldowns. + pub circuit_breaker: CircuitBreakerConfig, + /// Static default request headers, stamped by `SetHeaders` (just outside `Auth`). + pub headers: http::HeaderMap, + /// Ceiling on how long an exhausted bucket back-pressures before the request + /// returns [`HttpError::Throttled`](crate::HttpError::Throttled). Distinct from + /// `timeout`: `RateLimit` sits **outside** `Timeout`, so the permit wait is + /// bounded by this — at IBKR's 1/15-min buckets, minutes not seconds. + pub rate_limit_max_wait: Duration, +} + +/// Assemble the canonical resilience stack (ADR-0031 §1) over an arbitrary leaf. +/// +/// Builds the fallible [`RateLimit`](crate::RateLimit) layer **first** — it runs +/// `validate_coverage` + `validate_concurrency_singleton` — so a config that is not +/// total over `K::all()`, carries an out-of-range policy param, or breaches the +/// ≤1-concurrency-permit invariant is a [`BuildError`] before the infallible layers +/// are assembled. Then composes, outermost-first: +/// `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( Auth( leaf ) ) ) ) ) ) )`. +/// `Auth`/`SetHeaders` are direct `Service` wrappers (no `Layer` factory), so they +/// pre-wrap the leaf; the composed value satisfies [`HttpClient`] by blanket impl. +/// +/// # Errors +/// [`BuildError`] propagated from `RateLimitLayer::new` if `rate_limits` is not +/// total over `K::all()`, any policy is out of range, or the concurrency-singleton +/// invariant is breached. +pub fn stack( + leaf: S, + cfg: HttpConfig, + timer: T, + auth: A, + rate_limits: RateLimitConfig, +) -> Result +where + S: HttpClient + Clone + Send + Sync + 'static, + T: Timer + 'static, + A: AuthSource + 'static, + K: RateKey + fmt::Debug, +{ + // Fallible layer first: validates coverage + concurrency-singleton (fail-closed + // at construction — nothing else is built if this errors). + let rate = RateLimitLayer::new(&rate_limits, timer.clone(), cfg.rate_limit_max_wait)?; + // The two innermost layers are direct wrappers, not `Layer` factories. + let inner = SetHeaders::new(Auth::new(leaf, auth), cfg.headers); + let svc = ServiceBuilder::new() + .layer(TracingLayer::new(timer.clone())) // outermost + .layer(CircuitBreakerLayer::new(cfg.circuit_breaker, timer.clone())) + .layer(RetryLayer::new(cfg.retry, timer.clone())) + .layer(rate) + .layer(TimeoutLayer::new(cfg.timeout, timer)) // innermost Layer-factory + .service(inner); + Ok(svc) +} +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `just check && cargo test -p oath-adapter-net-http-api stack && just lint && just doc` +Expected: PASS, warning-free, docs clean. The smoke test proves the composed type type-checks against the `impl HttpClient + Clone + Send + Sync + 'static` bound and is body-transparent; the boot-coverage test proves construction fails closed. + +> Known risks (address in place if hit): +> - **Return-bound compile error.** If `stack()` fails to satisfy `Send`/`Sync`/`'static`, the failing layer is named in the error. This is the bound doing its job — the fix is on the layer, not `stack()`; but for this slice every shipped layer already holds `Arc` state + `Clone` config, so a failure most likely means a missing `+ 'static` on `T`/`A` (already in the `where` clause above) or a non-`Sync` test leaf (the `ScriptLeaf` is `Sync` via its `Arc` fields). +> - **`ServiceBuilder` import.** `.layer()`/`.service()` are inherent methods; the `Layer` trait is imported only to satisfy the `where L: Layer` bound resolution. If clippy flags `Layer` as unused, drop it from the `use`. +> - **`missing_const_for_fn` on `stack`.** It calls fallible `RateLimitLayer::new`, so it cannot be `const` — no action. +> - **`LimitPolicy::TokenBucket` fields.** It has `{ rate, per, burst }` (three fields). The `rate_cfg()` builder uses all three — do not drop `per`. + +- [ ] **Step 6: Commit** + +```bash +git add crates/adapter/net/http/api/src/stack.rs crates/adapter/net/http/api/src/lib.rs +git commit -m "feat(net): HttpConfig + stack() — validate-then-compose the canonical order" +``` + +--- + +## Task 2: Full-stack ordering-invariant, fail-closed, and Auth-per-attempt tests + +**Files:** +- Modify: `crates/adapter/net/http/api/src/stack.rs` (test module only) + +**Interfaces:** +- Consumes: the Task 1 test harness (`ScriptLeaf`, `CounterAuth`, `Key`, `StubBody`, `http_cfg`, `rate_cfg`, `req`) and `stack`/`HttpConfig` (Task 1). +- Produces: no new public items — five regression tests that lock the composition order. These pass immediately against a **correct** `stack()`; if any fails, `stack.rs`'s layer order is wrong and must be fixed there (that is the point of full-stack tests — per-layer tests cannot catch a reorder). + +> These are characterization tests over already-built behaviour, so the TDD "red" is structural: run each once and confirm it passes (a correct order), and reason about *why the wrong order would fail* (noted per test). If one is red, the composition is genuinely wrong — fix `stack()`. + +- [ ] **Step 1: Add the five tests** + +Append inside the same `tests` module in `stack.rs`, after the Task 1 tests. **No new imports** — these tests use only names already in scope from Task 1 (`ScriptLeaf`, `Step`, `CounterAuth`, `Key`, `stack`, `http_cfg`, `rate_cfg`, `req`, `RateLimitConfig`, `LimitDecl`, `LimitPolicy`, `HttpError`, `NoAuth`, `Scope`, `MockTimer`, `Duration`, `HashMap`). Add the tests: + +```rust + // 1. CircuitBreaker OUTSIDE Retry — an open circuit fast-rejects; the leaf is + // untouched and no retry loop spins on the rejection. If CB were INSIDE + // Retry this could not hold: the breaker would be re-consulted per attempt. + #[tokio::test] + async fn circuit_opens_and_fast_rejects_without_touching_the_leaf() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Err]); // always fails + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), // retry ON (3), zero backoff + timer, + NoAuth, + rate_cfg(), + ) + .expect("total config"); + // 3 logical failures (each retried 3x → 9 leaf calls) trip the breaker. + for _ in 0..3 { + let _ = svc.call(req(Scope::Global, None)).await; + } + let calls_after_trip = leaf.calls(); + assert_eq!(calls_after_trip, 9, "3 requests x 3 attempts reached the leaf before the trip"); + // Next request: circuit is Open → CircuitOpen, leaf untouched, no spin. + let err = svc.call(req(Scope::Global, None)).await.unwrap_err(); + assert!(matches!(err, HttpError::CircuitOpen)); + assert_eq!(leaf.calls(), 9, "open circuit fast-rejects; leaf untouched, Retry never spun"); + } + + // 2. RateLimit INSIDE Retry — each attempt re-acquires budget. With a burst-1 + // bucket and zero max_wait, the first attempt drains it and the retry + // throttles at the (empty) bucket, so the leaf is hit exactly once. If + // RateLimit were OUTSIDE Retry, the single token would cover the whole + // logical request and the retry would resend to a 200. + #[tokio::test] + async fn rate_limit_is_spent_per_attempt_inside_retry() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(503), Step::Status(200)]); + // Snapshot: burst 1, refill 1/hour → no refill during the test. + let rc = RateLimitConfig { + global: LimitPolicy::TokenBucket { rate: 1000, per: Duration::from_secs(1), burst: 1000 }, + local: HashMap::from([ + ( + Key::Snapshot, + LimitDecl::Policy(LimitPolicy::TokenBucket { + rate: 1, + per: Duration::from_secs(3600), + burst: 1, + }), + ), + (Key::History, LimitDecl::GlobalOnly), + ]), + }; + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), + timer, + NoAuth, + rc, + ) + .expect("total config"); + let err = svc.call(req(Scope::Local, Some(Key::Snapshot))).await.unwrap_err(); + assert!( + matches!(err, HttpError::Throttled), + "the retry re-acquired the drained bucket → per-attempt pacing (RateLimit inside Retry)" + ); + assert_eq!(leaf.calls(), 1, "only attempt 1 reached the leaf; the retry throttled at the bucket"); + } + + // 3. Timeout bounds the SEND. A hanging leaf, with the clock advanced past the + // send timeout, yields Timeout. (RateLimit sits outside Timeout, so the + // permit wait is bounded separately by rate_limit_max_wait — structural.) + #[tokio::test] + async fn send_timeout_fires_on_a_hanging_leaf() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Hang]); + let svc = stack( + leaf, + http_cfg(1, Duration::from_secs(1), Duration::ZERO), // retry OFF, 1s send timeout + timer.clone(), + NoAuth, + rate_cfg(), + ) + .expect("total config"); + let waiter = tokio::spawn(async move { svc.call(req(Scope::Global, None)).await }); + tokio::task::yield_now().await; // task registers the inner sleep + the 1s deadline + timer.advance(Duration::from_secs(1)); // fire the send-timeout deadline + let err = waiter.await.unwrap().unwrap_err(); + assert!(matches!(err, HttpError::Timeout)); + } + + // 4. Auth re-stamps per attempt — inside Retry, so each of the N attempts + // carries a fresh credential. + #[tokio::test] + async fn auth_restamps_a_fresh_credential_on_every_attempt() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Err, Step::Err, Step::Status(200)]); + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), + timer, + CounterAuth::new(), + rate_cfg(), + ) + .expect("total config"); + let resp = svc.call(req(Scope::Global, None)).await.expect("third attempt is 200"); + assert_eq!(resp.status(), http::StatusCode::OK); + assert_eq!( + leaf.seen_auth(), + vec![Some("token-0".to_owned()), Some("token-1".to_owned()), Some("token-2".to_owned())], + "Auth ran once per attempt (inside Retry), re-stamping a fresh credential each time" + ); + } + + // 5. Scope fail-closed end-to-end — a request with no RateScope extension is + // rejected before the leaf, and the fail-closed path survives composition. + #[tokio::test] + async fn absent_scope_is_rejected_fail_closed_through_the_stack() { + let timer = MockTimer::new(); + let leaf = ScriptLeaf::new(timer.clone(), vec![Step::Status(200)]); + let svc = stack( + leaf.clone(), + http_cfg(3, Duration::from_secs(30), Duration::ZERO), + timer, + NoAuth, + rate_cfg(), + ) + .expect("total config"); + // A request with neither RateScope nor Retryable — the forgotten-stamp case. + let bare = http::Request::builder().method("GET").uri("/x").body(Bytes::new()).unwrap(); + let err = svc.call(bare).await.unwrap_err(); + assert!(matches!(err, HttpError::Throttled), "no RateScope → fail-closed Throttled"); + assert_eq!(leaf.calls(), 0, "the fail-closed request never reached the leaf"); + } +``` + +- [ ] **Step 2: Run the tests to verify they pass** + +Run: `cargo test -p oath-adapter-net-http-api stack -- --nocapture` +Expected: PASS — all seven `stack` tests green. A red test #1–#5 means the composition order in `stack()` is wrong (or a layer's classification changed); fix `stack.rs` (do not weaken the test). + +- [ ] **Step 3: Lint + doc** + +Run: `just lint && just doc` +Expected: PASS, warning-free, docs clean. (The `_AtomicUsizeInUse` scaffolding line from Step 1, if you added it, is unnecessary — remove it; all names are already imported by Task 1.) + +- [ ] **Step 4: Commit** + +```bash +git add crates/adapter/net/http/api/src/stack.rs +git commit -m "test(net): full-stack ordering-invariant, fail-closed, and per-attempt Auth tests" +``` + +--- + +## Task 3: ADR amendment, CHANGELOG, full gate, PR + +**Files:** +- Modify: `docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md` +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: ADR-0034 append-only amendment** + +Append to ADR-0034's **Amendments** numbered list, after item **10** (the Tracing note), a new item **11**: + +```markdown +11. **`stack()` assembly + `HttpConfig` (Slice 2, assembly).** `stack()` + (`net-http-api`) assembles the canonical resilience order (ADR-0031 §1) over an + arbitrary leaf: `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( + Auth( leaf ) ) ) ) ) ) )`. It builds the one fallible layer (`RateLimitLayer::new`, + which runs `validate_coverage` + `validate_concurrency_singleton`) **first**, so a + coverage/param/singleton failure is a `BuildError` before the infallible layers are + assembled — `stack()` does **not** call `validate_coverage` separately. `Auth`/ + `SetHeaders` are direct `Service` wrappers (no `Layer` factory), so they pre-wrap + the leaf; the five `Layer`-factory layers compose over that via the kernel's + `ServiceBuilder`. The return bound is the full `impl HttpClient + Clone + Send + + Sync + 'static` (not bare `impl HttpClient`), so a `Send`/`Clone`/`'static` + regression in any layer is a compile error *at `stack()`*; `build()` (the following + hyper-backend slice) reuses this bound over the hyper leaf. `HttpConfig` is + non-generic plain data — `timeout`, `retry`, `circuit_breaker`, `headers`, and + `rate_limit_max_wait` (the permit-wait ceiling feeding `RateLimitLayer::new`, + distinct from the send `timeout` because `RateLimit` sits outside `Timeout`) — with + no type parameter and no `serde` (deserialisation stays in the adapter, ADR-0003). + The one generic pacing arg (`RateLimitConfig`), `auth`, and `timer` are separate + `stack()` parameters. **Bound refinement:** the spec sketch's `T: Timer, A: + AuthSource, K: RateKey` becomes `T: Timer + 'static, A: AuthSource + 'static, K: + RateKey + Debug` in the implementation (the composed value is returned `'static`; + coverage validation renders the offending key). `BufferOrStream` is **not** a + layer here — buffering is a leaf-side body-construction concern, so the innermost + leaf already satisfies "inside `Retry`". Full-stack ordering invariants are + regression-tested over an inline recording leaf + `MockTimer` (not `MockClient`, + which would close the net-http-mock → net-http-api dev-dep cycle and cannot script + sequences). No new dependency; no existing-layer change. +``` + +> **Numbering caveat:** if another PR landed an amendment concurrently and took **#11**, renumber this one to the next free integer during rebase (mechanical, not a design change). + +- [ ] **Step 2: CHANGELOG** + +Add to `CHANGELOG.md` `[Unreleased] → Added`, at the **end of the list** (after the `Tracing` resilience-layer entry): + +```markdown +- `oath-adapter-net-http-api` `stack()` assembly + `HttpConfig` (Slice 2, assembly) — + `stack()` composes the canonical resilience order (ADR-0031 §1) + `Tracing(CircuitBreaker(Retry(RateLimit(Timeout(SetHeaders(Auth(leaf)))))))` over any + leaf, returning `Result`. + It builds the fallible `RateLimit` layer first (running `validate_coverage` + + `validate_concurrency_singleton`), so a coverage/param/singleton failure is a boot + error before the rest is assembled. `HttpConfig` is the non-generic aggregate + (`timeout`, `retry`, `circuit_breaker`, `headers`, `rate_limit_max_wait`); the pacing + map, `auth`, and `timer` are separate arguments. Full-stack ordering invariants + (CircuitBreaker-outside-Retry, RateLimit-inside-Retry, send-Timeout, per-attempt + Auth, Scope fail-closed) are regression-tested over an inline leaf + `MockTimer`. No + new dependency; no existing-layer change. (ADR-0031 §1, ADR-0034.) The hyper leaf + + `build()` land in the following slice. +``` + +- [ ] **Step 3: Full local gate** + +Run: `just ci` +Expected: green — fmt, lint, test + doctests, doc, deny, typos, machete. No new dependency, so `deny`/`machete` see no change. + +- [ ] **Step 4: Commit, push, PR** + +```bash +git add docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md CHANGELOG.md +git commit -m "docs(net): record stack()/HttpConfig assembly amendment (ADR-0034) + changelog" +git push -u origin feat/net-http-stack-assembly +gh pr create \ + --title "feat(net): stack() assembly + HttpConfig (Slice 2)" \ + --body "Closes # + +Slice 2 (assembly, runtime-free) of the net-http construction surface (spec: docs/superpowers/specs/2026-07-05-net-http-stack-assembly-design.md; ADR-0031 §1, ADR-0034). + +- **\`HttpConfig\`** — non-generic aggregate: \`timeout\`, \`retry\`, \`circuit_breaker\`, \`headers\`, \`rate_limit_max_wait\`. +- **\`stack()\`** — builds the fallible \`RateLimit\` layer first (validates coverage + concurrency-singleton), then composes \`Tracing(CircuitBreaker(Retry(RateLimit(Timeout(SetHeaders(Auth(leaf)))))))\` over any leaf. Returns \`Result\`, so a layer regression is a compile error at \`stack()\`. +- **Full-stack tests** over an inline recording leaf + \`MockTimer\` (no \`MockClient\` — dev-dep cycle): CircuitBreaker-outside-Retry, RateLimit-inside-Retry, send-Timeout, per-attempt Auth re-stamp, Scope fail-closed, plus a transparency smoke and a boot-coverage \`BuildError\`. + +No new dependency; no runtime; no existing-layer change. \`BufferOrStream\` is leaf-side (not a layer). Next: the hyper-backend slice — \`build()\` + \`hyper_leaf()\` + \`TokioTimer\`, delegating to this \`stack()\`. + +🤖 Generated with [Claude Code](https://claude.com/claude-code)" +``` + +Expected: PR open, GitHub Actions CI green (same `just ci` + MSRV job). + +--- + +## Self-Review + +**Spec coverage (design doc §Scope + Decisions):** +- `HttpConfig` five-field non-generic aggregate (`timeout`, `retry`, `circuit_breaker`, `headers`, `rate_limit_max_wait`) — Task 1 Step 4. ✅ +- `stack()`, validate-via-RateLimit-first, canonical order, full return bound — Task 1 Step 4 + smoke test. ✅ +- `Auth`/`SetHeaders` as direct wrappers pre-wrapping the leaf; five `Layer` layers via `ServiceBuilder` — Task 1 Step 4. ✅ +- Boot coverage → `BuildError::UndeclaredKey`, constructs nothing — Task 1 `missing_key_is_a_build_error_and_constructs_nothing`. ✅ +- CircuitBreaker outside Retry — Task 2 test 1. ✅ +- RateLimit inside Retry (per-attempt budget) — Task 2 test 2. ✅ +- Timeout bounds the send — Task 2 test 3. ✅ +- Auth re-stamps per attempt — Task 2 test 4. ✅ +- Scope fail-closed end-to-end — Task 2 test 5. ✅ +- Transparency smoke (all seven layers) — Task 1 `plain_request_threads_all_layers_and_body_is_transparent`. ✅ +- Inline leaf + `MockTimer`, no `MockClient` — Tasks 1–2 harness. ✅ +- ADR-0034 amendment #11 + CHANGELOG — Task 3. ✅ +- Deferred (correctly absent): `build()`, `hyper_leaf`, `ConnConfig`, `TokioTimer`, `BufferOrStream` layer, `serde` on `HttpConfig` — noted, not built. ✅ + +**Placeholder scan:** none — every code step carries complete code; every command step an expected result. `#` is the real issue number captured in Setup. (The `_AtomicUsizeInUse` line in Task 2 Step 1 is an explicit no-op guard removed in Step 3, not a placeholder.) + +**Type consistency:** +- `HttpConfig { timeout, retry, circuit_breaker, headers, rate_limit_max_wait }` — identical in the struct (Task 1 Step 4), `http_cfg()` (Task 1 Step 1), and the ADR/CHANGELOG copy. +- `stack(leaf, cfg, timer, auth, rate_limits) -> Result` with `S: HttpClient + Clone + Send + Sync + 'static, T: Timer + 'static, A: AuthSource + 'static, K: RateKey + fmt::Debug` — identical in the Interfaces block, Task 1 Step 4, and every test call site. +- `RateLimitLayer::new(&cfg, timer, max_wait) -> Result<_, BuildError>`, `Auth::new(inner, auth)`, `SetHeaders::new(inner, headers)`, `TracingLayer::new(timer)`, `CircuitBreakerLayer::new(cfg, timer)`, `RetryLayer::new(cfg, timer)`, `TimeoutLayer::new(default, timer)` — all match the shipped signatures read from the crate. +- `LimitPolicy::TokenBucket { rate, per, burst }` / `Concurrency { max }`, `LimitDecl::{Policy, GlobalOnly}`, `RateLimitConfig { global, local }` — match the shipped `rate.rs`/`rate_limit.rs`. +- `RetryConfig { max_attempts: NonZeroU32, base, cap, seed }`, `CircuitBreakerConfig { failure_threshold, cooldown, throttle_cooldown, half_open_probes }` (all `NonZeroU32`/`Duration`) — match the shipped structs. +- `Scope::{None, Global, Local, Both}`, `RateScope { scope, key: Option }`, `Retryable`, `NoAuth`, `HttpError::{Throttled, Timeout, CircuitOpen, connection()}`, `BuildError::UndeclaredKey` — match the shipped enums/markers. +- `AuthSource::authorize(&self, &mut http::Request) -> impl Future> + Send` — `CounterAuth` matches the shipped trait exactly. +- lib.rs additions: `pub mod stack;` + `pub use stack::{stack, HttpConfig};` (module in the type namespace, fn in the value namespace — no clash) + one module-doc bullet. + +**Known risks to watch during impl:** listed inline in Task 1 Step 5 (return-bound compile error, `ServiceBuilder`/`Layer` import, `const fn`, `TokenBucket` fields) and Task 2 Step 1/3 (the no-op import guard). diff --git a/docs/superpowers/specs/2026-07-05-net-http-stack-assembly-design.md b/docs/superpowers/specs/2026-07-05-net-http-stack-assembly-design.md new file mode 100644 index 0000000..7aa305a --- /dev/null +++ b/docs/superpowers/specs/2026-07-05-net-http-stack-assembly-design.md @@ -0,0 +1,230 @@ +# net-http `stack()` assembly + `HttpConfig` — design + +**Status:** Approved design, pre-implementation. +**Date:** 2026-07-05. +**Crate:** `oath-adapter-net-http-api` (`crates/adapter/net/http/api`). +**Slice:** Slice 2 (assembly), runtime-free half. The hyper leaf + `build()` + +`TokioTimer` are a separate, following slice (the "hyper-backend slice"). + +## Context + +The [net-http construction-surface spec](2026-06-30-net-http-construction-surface-design.md) +(Seam #3) fixes the `stack()`/`build()` split, the return bound, the config split, +and boot-time pacing coverage. [ADR-0031 §1](../../adr/0031-http-resilience-venue-pacing.md) +fixes the canonical layer order. This spec closes the last runtime-free gap in that +surface: the concrete `stack()` assembly and the `HttpConfig` aggregate that feeds it. + +Everything the assembly composes already ships in `oath-adapter-net-http-api`: + +- All five layers — `Tracing` (#86), `CircuitBreaker` (#85), `Retry` (#82), + `Timeout` (#78), `RateLimit` (#76) — plus `Auth`/`SetHeaders`/`Guarded` (#66). +- The `RateScope`/`Scope` per-request extension with **absent ⇒ fail-closed** + (`HttpError::Throttled`, non-retryable) shipped *with* `RateLimit` (#76, + ADR-0034 Amendment #1). +- `RateKey`, `RateLimitConfig`, `LimitPolicy`/`LimitDecl`, `BuildError`, and the + standalone `validate_coverage` boot check (#72). + +So this slice adds **no new dependency**, no runtime, and **changes no existing +layer** — it is pure composition plus the config aggregate, plus the full-stack +ordering-invariant tests that only an assembly makes possible. + +### Governing decisions (inherited, not re-litigated) + +- **Layer order** — [ADR-0031 §1](../../adr/0031-http-resilience-venue-pacing.md): + `Tracing → CircuitBreaker → Retry → RateLimit → Timeout → BufferOrStream → Auth → leaf` + (first `.layer()` outermost, ADR-0029). `SetHeaders` folds in just outside `Auth`. +- **`stack()` signature + return bound** — construction-surface spec, Seam #3. +- **Config split** — non-generic `HttpConfig` data + the single `K`-generic + `RateLimitConfig` arg; `serde` stays in the adapter (ADR-0003). +- **Boot-time coverage** — `stack()` calls `validate_coverage` before assembling, so a + missing/ill-configured bucket is a `BuildError`, not a first-live-order 429. + +## Goal + +Deliver `HttpConfig` and `stack()` so the canonical resilience stack can be assembled +once, over any leaf, and its **ordering invariants** (not just per-layer behaviours) +are regression-tested deterministically over an inline recording leaf + `MockTimer`. + +## Scope (in) + +- **`HttpConfig`** — a non-generic aggregate of the per-layer configs + static headers. +- **`stack()`** — validate-then-compose the canonical order over an + arbitrary `HttpClient` leaf, returning `Result`. +- **Ordering-invariant + boot-coverage + fail-closed tests** over + `stack(, …, MockTimer)` (no `MockClient` — see Testing). +- **ADR-0034 amendment + CHANGELOG.** + +## Non-goals (deferred — the hyper-backend slice) + +| Deferred item | Why | Lands with | +| --- | --- | --- | +| `build()`, `hyper_leaf(conn)`, `ConnConfig` | Backend-specific; `build() = stack(hyper_leaf(conn), …)` over the same assembly | hyper-backend slice | +| `TokioTimer`, rustls/HTTPS connector, `hyper::Error → HttpError` | Runtime/TLS integration behind the `HttpClient` seam | hyper-backend slice | +| `BufferOrStream` as a middleware | Not a layer — buffering is a leaf-side body-construction concern (`ResponseBody::buffered`/`streaming` per `BufferMode`); the innermost leaf already satisfies "inside `Retry`" | hyper-backend slice (leaf) | +| `serde` on `HttpConfig` | Config deserialisation is an adapter concern (ADR-0003) | IBKR adapter slice | + +## Decisions + +### `HttpConfig` — one field per configurable layer + +```rust +/// Non-generic assembly configuration: one field per configurable layer, plus the +/// static default headers. The `K`-generic pacing map (`RateLimitConfig`), +/// `auth`, and `timer` are separate `stack()` args, so this type carries no type +/// parameter and no `serde` (deserialisation stays in the adapter, ADR-0003). +#[derive(Debug, Clone)] +pub struct HttpConfig { + /// Per-attempt send timeout (bounds the send, not the permit wait). + pub timeout: Duration, + /// Retry policy (attempts, backoff schedule). + pub retry: RetryConfig, + /// Circuit-breaker thresholds and cooldown. + pub circuit_breaker: CircuitBreakerConfig, + /// Static default request headers, stamped by `SetHeaders` (just outside `Auth`). + pub headers: HeaderMap, + /// Ceiling on how long an exhausted bucket back-pressures before the request + /// returns `HttpError::Throttled` — `RateLimitLayer::new`'s third parameter. + /// Distinct from `timeout`: `RateLimit` sits **outside** `Timeout`, so the permit + /// wait is bounded by this, not the send timeout — and at IBKR's 1/15-min buckets + /// it is minutes, not seconds. + pub rate_limit_max_wait: Duration, +} +``` + +**Five fields, exactly.** `Tracing` needs no config (only the clock). The pacing +*map* is the one generic arg (`RateLimitConfig`), isolated so `HttpConfig` stays +non-generic; the pacing *permit-wait ceiling* (`rate_limit_max_wait`) is non-generic +and lives here, feeding `RateLimitLayer::new`'s third parameter. `Auth` is supplied as +the `auth: A` value. `HttpConfig` is a plain struct literal (not `#[non_exhaustive]`): +adapters construct it directly, matching every other config type in the crate; a +future field is a deliberate, reviewed breaking change, not something to pre-absorb +(YAGNI). + +### `stack()` — validate, then compose in canonical order + +```rust +/// Assemble the canonical resilience stack (ADR-0031 §1) over an arbitrary leaf. +/// +/// Builds the fallible `RateLimit` layer **first**, so a config that is not total +/// over `K::all()` (or carries an out-of-range policy param, or breaches the +/// ≤1-concurrency-permit invariant) is a `BuildError` before the infallible layers +/// are assembled. Then composes, outermost-first: +/// `Tracing( CircuitBreaker( Retry( RateLimit( Timeout( SetHeaders( Auth( leaf ) ) ) ) ) ) )`. +/// +/// # Errors +/// [`BuildError`] if `rate_limits` is not total over `K::all()`, any policy is out of +/// range, or the concurrency-singleton invariant is breached — all propagated from +/// `RateLimitLayer::new` (which runs `validate_coverage` + +/// `validate_concurrency_singleton` internally). +pub fn stack( + leaf: S, + cfg: HttpConfig, + timer: T, + auth: A, + rate_limits: RateLimitConfig, +) -> Result +where + S: HttpClient + Clone + Send + Sync + 'static, + S::Body: Send, + T: Timer + 'static, + A: AuthSource + 'static, + K: RateKey + fmt::Debug, +``` + +> **Bound refinements over the construction-surface sketch.** The sketch wrote +> `T: Timer, A: AuthSource, K: RateKey`. The implementation adds: `S::Body: Send` +> (transitively required — `Retry`/`RateLimit` carry a `B: Send` bound, and +> `HttpClient::Body` is not `Send` by default, so it must surface at `stack()`); +> `T`/`A` `+ 'static` (the composed value is returned `'static`); and `K: + Debug` +> (`RateLimitLayer::new` renders the offending key when coverage fails). Recorded in +> ADR-0034 Amendment #11. + +- **Validate via the RateLimit layer.** `RateLimitLayer::new(&rate_limits, timer, + cfg.rate_limit_max_wait)?` runs `validate_coverage` + `validate_concurrency_singleton` + and is constructed first, so a coverage/param/singleton failure short-circuits with + `BuildError` before the rest is assembled (fail-closed at construction). `stack()` + does not call `validate_coverage` separately — that would double-validate. +- **One clock, cloned in.** The single `timer: T` is cloned into each timing layer + (`CircuitBreaker`, `Retry`, `RateLimit`, `Timeout`, `Tracing`); `Timer: Clone`. +- **Assembly mechanism.** `Auth`/`SetHeaders` are direct `Service` wrappers + (`::new(inner, …)`, no `Layer` factory), so they pre-wrap the leaf; the five + `Layer`-factory layers compose over that via the kernel's `ServiceBuilder` (first + `.layer()` = outermost). The composed value auto-satisfies `HttpClient` (blanket + impl). The mechanism is internal; the observable contract is the order + return bound. +- **Return bound.** The full `impl HttpClient + Clone + Send + Sync + 'static` (not + bare `impl HttpClient`) turns any `Send`/`Clone`/`'static` regression in a layer + into a compile error *at `stack()`*, and promises adapters the share/spawn they + need. `build()` (next slice) reuses this exact bound over the hyper leaf. +- **`BufferOrStream` absent.** Not composed here — buffering is the leaf's concern + (see Non-goals). The seven built layers wrap the leaf directly. + +### Layer nesting (built layers only) + +| Position | Layer | Config source | Invariant it anchors | +| --- | --- | --- | --- | +| outermost | `Tracing` | `timer` | one span over the whole logical request | +| | `CircuitBreaker` | `cfg.circuit_breaker`, `timer` | **outside `Retry`** — short-circuits before retry runs | +| | `Retry` | `cfg.retry`, `timer` | order-safe, retryability-aware | +| | `RateLimit` | `rate_limits`, `timer`, `cfg.rate_limit_max_wait` | **inside `Retry`** — each attempt spends budget | +| | `Timeout` | `cfg.timeout`, `timer` | bounds the send, **not** the permit wait | +| | `SetHeaders` | `cfg.headers` | static stamp, just outside `Auth` | +| innermost layer | `Auth` | `auth` | re-stamps credentials **per attempt** | +| leaf | `S` (inline double in tests / hyper in prod) | — | — | + +## Testing + +Full-stack over `stack(, …, MockTimer)` — the only tests that catch a +builder reorder (per-layer isolation tests cannot). + +**Leaf: an inline recording double, NOT `MockClient`.** `net-http-api` must not +dev-depend on `oath-adapter-net-http-mock` — that crate normal-depends on +`net-http-api`, so the dev-dep closes a cycle that recompiles a second, non-unifying +copy of the crate (the same constraint every layer suite already follows; see +`net-http-api-test-doubles` note). And `MockClient` is a fixed-status leaf — it cannot +script a `503 → 200` sequence or a never-completing send. So the stack tests build a +small **inline** leaf in the test module: a `Clone` struct with `Arc>>` +recording + a scripted/hanging response, satisfying `HttpClient + Clone + Send + Sync + +'static`, exactly like `retry.rs`/`circuit_breaker.rs`'s `ScriptLeaf`. `MockTimer` +comes from `oath-adapter-net-mock` (an existing dev-dep); tests run on `tokio` +(dev-only). No new dev-dependency. + +1. **`CircuitBreaker` outside `Retry`** — with the circuit forced open (a leaf scripted + to fail past `failure_threshold`, then a follow-up request), the leaf is **not** + called again and `Retry` does not spin: assert the send count does not increase and + the outcome is `HttpError::CircuitOpen`. +2. **`RateLimit` inside `Retry`** — a leaf scripted `503 → 200` under a rate bucket: + assert the second attempt spends a token (advancing `MockTimer` is required between + attempts for the token to refill), proving the limiter sits inside the retry loop. +3. **`Timeout` bounds the send, not the permit wait** — a leaf whose send never + completes; advancing `MockTimer` past `cfg.timeout` yields `HttpError::Timeout`. +4. **`Auth` re-stamps per attempt** — a recording inline leaf + a counter `AuthSource` + whose `authorize` stamps a monotonic header: assert each of the N attempts carries a + distinct credential. +5. **Boot coverage** — `stack()` with a `RateLimitConfig` missing a `K::all()` variant + returns `Err(BuildError::UndeclaredKey)` and constructs nothing. +6. **`Scope` fail-closed end-to-end** — a request with **no** `RateScope` extension, + driven through the fully assembled stack, is rejected (`HttpError::Throttled`, + non-retryable — `Retry` does not spin), confirming the fail-closed path survives + composition. +7. **Ordering-sanity smoke** — a plain `200` request threads through all seven layers + and returns the leaf's body intact (transparency end-to-end). + +## Delivery + +One PR (`feat(net): stack() assembly + HttpConfig (Slice 2)`), one issue, one worktree +under `.claude/worktrees/`. No new dependency (so `deny`/`machete` are +unaffected). Records an ADR-0034 append-only amendment and a `CHANGELOG.md` +`[Unreleased]` entry. DoD: `just ci` green (incl. `just doc`). + +## ADR reconciliation + +Append an ADR-0034 amendment recording: `HttpConfig`'s five-field shape (including +`rate_limit_max_wait`, which feeds `RateLimitLayer::new` and is distinct from the send +`timeout` because `RateLimit` sits outside `Timeout`) and its non-generic/`serde`-free +rationale; `stack()`'s construct-RateLimit-first validation (delegating to +`RateLimitLayer::new`'s internal `validate_coverage` + `validate_concurrency_singleton`, +not a separate call) and compose contract; the exact nesting of the seven built layers +with `Auth`/`SetHeaders` as direct wrappers; the `BufferOrStream`-is-leaf-side +resolution; and that `build()` (next slice) delegates to this `stack()` over the hyper +leaf.