diff --git a/CHANGELOG.md b/CHANGELOG.md index bdceb67..2e08f49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 dynamic wins), and `Guarded` (response body carrying an optional `async-lock` concurrency permit, released at the earlier of stream-end or drop). ADR-0034 records the construction-surface decisions and the ADR-0030/0031 amendments. +- 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 + under-pacing path; `Guarded` releases its permit on the earliest of terminal + frame, **mid-stream error**, or drop; the `async-lock` choice is re-grounded on + the stated multi-backend goal (a non-tokio stack stays genuinely `tokio`-free); + and `MockTimer` will relocate from `net-http-mock` into a shared dev-only + `oath-adapter-net-mock` crate so the WS resilience slice shares one fake clock. + Code changes land with their slices. - `oath-adapter-net-ws-api` WebSocket contract (ADR-0032/0033) — `Frame`/`CloseFrame` (RFC 6455 frame vocabulary), `WsError` (one concrete transport error with `HasErrorKind`), the split owned halves (`WsSink` one-shot RPITIT send half with 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 aa71fcf..44407e5 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 @@ -107,3 +107,50 @@ response continues downstream unchanged. - Implementation lands in slices: `AuthSource`/`Auth`/`SetHeaders`/`Guarded` first; `RateKey`/`RateLimitConfig`/`BuildError` next; the resilience layers and `stack()`/`build()` + the hyper leaf in later slices. + +## Amendments (2026-07-04) + +Refinements from a follow-up design review, recorded append-only (the decision text +above is not edited). Each lands with its implementation slice; the +[construction-surface spec](../superpowers/specs/2026-06-30-net-http-construction-surface-design.md) +carries the full reasoning. + +1. **Absent `RateLimit` directive fails closed (tightens §3 / 0031 §3).** With + config-side totality (§3) in place, 0031's "absent directive defaults to `Global`" + was the one remaining *silent* under-pacing path: an adapter that adds an endpoint + and forgets to stamp the directive would fly global-paced only, invisible to the + boot check because the gap is in the request, not the config. So a request with + **no** `RateLimit` extension is now **rejected fail-closed** (the same + non-retryable classification error as the missing-bucket backstop). "Global only" + is said with an explicit `Scope::Global`; opt-out with `Scope::None`. Lands with + the `RateLimit` slice. + +2. **`Guarded` releases the permit on a mid-stream error too (refines §2).** §2's + eager release fires on the clean terminal frame (`None`); it now *also* fires on a + mid-stream `Some(Err(_))` — a connection reset during a `/history` transfer is as + over as a clean end (http-body 1.x yields no further frames after an error), and a + consumer holding the errored body for error context must not pin one of `/history`'s + 5 permits. Release rule: **the earliest of terminal frame, mid-stream error, or + drop.** Touches the `Guarded` body shipped in PR #66; lands as a follow-up code + change. + +3. **The `async-lock` choice rests on the stated multi-backend goal (sharpens §2's + rationale).** OATH intends non-tokio backends (smol/async-std) — a stated goal, not + a hedge — which decides `async-lock` (option R) over `tokio` `features=["sync"]` + (option P) on a concrete basis, not a "thin margin": under P a future *smol* stack + would drag `tokio` into its graph purely for the semaphore; under R the whole smol + stack is genuinely `tokio`-free. Option Q (a `Timer`-style semaphore trait) is + unnecessary, not merely YAGNI — `async_lock::Semaphore` is already cross-runtime, so + backends reuse the same layer semaphore; nothing to abstract. + +4. **`MockTimer` is *relocated* into its own dev-only `oath-adapter-net-mock` crate** + (`crates/adapter/net/mock`), beside the `Timer` contract in `net-api` — *not* left + in `net-http-mock`, which keeps only `MockClient`. `MockTimer` already ships in + `net-http-mock` (`crates/adapter/net/http/mock/src/timer.rs`, PR #66); this moves + it and re-points the http-stack tests. **Time-critical:** the WS resilience slice + (ADR-0033 §9) is imminent and `net-ws-mock`'s own header already says + "`MockTimer`/`MockSpawn` arrive with the resilience slice" — without the + relocation that slice must either duplicate `MockTimer` or dev-depend a *WS* mock + on an *HTTP* mock (the nonsense edge across the crate cut). Extracting to a shared + `net-mock` lets both stacks share one fake clock. Both mocks keep the + production-reachability guard (`cargo tree -e no-dev -i …` → no non-dev dependents). diff --git a/docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md b/docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md index 8e722c4..917bf48 100644 --- a/docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md +++ b/docs/superpowers/specs/2026-06-30-net-http-construction-surface-design.md @@ -4,7 +4,9 @@ **Date:** 2026-06-30. **Crates:** `oath-adapter-net-http-api` (`crates/adapter/net/http/api`), `oath-adapter-net-http-hyper` (`crates/adapter/net/http/hyper`), -`oath-adapter-net-http-mock` (`crates/adapter/net/http/mock`, **new, dev-only**). +`oath-adapter-net-http-mock` (`crates/adapter/net/http/mock`, **new, dev-only** — +`MockClient`), `oath-adapter-net-mock` (`crates/adapter/net/mock`, **new, dev-only** +— `MockTimer`, beside the `Timer` contract in `net-api`). ## Context @@ -69,7 +71,12 @@ the `stack()`/`build()` split with boot-time pacing-coverage validation. `RateLimitConfig` (the `K`-generic arg); the `RateKey` trait; `BuildError`; boot-time total pacing coverage. - **`oath-adapter-net-http-mock`** — a dev-only crate providing the `MockClient` - leaf and `MockTimer`, used to regression-test the stack's ordering invariants. + leaf (HTTP-specific canned responses / recorded requests), used to regression-test + the stack's ordering invariants. +- **`oath-adapter-net-mock`** — a dev-only crate providing `MockTimer` (fake clock), + beside the transport-neutral `Timer` contract in `net-api` (not the HTTP mock), so + the WS stack (ADR-0032/0033) can fake the same clock without dev-depending on an + HTTP crate. - **ADR reconciliation** — the two amendments to 0030/0031 (below), recorded so the landed ADRs are not silently edited. @@ -110,6 +117,14 @@ pub struct NoAuth; The `Auth` layer is then `self.auth.authorize(&mut req).await?; self.inner.call(req).await` — a straight `?`, no conversion. +**Refresh-coalescing is the impl's job, deliberately.** `&self` (shared, `Clone`) +means any single-flighting of an expensive token refresh — so N concurrent post-401 +retries trigger *one* refresh, not N — lives inside the `AuthSource` impl over its +own token cache, not in the trait. The four real schemes that never refresh +(static/HMAC/cookie) pay nothing for it; only the future OAuth impl implements it. +The hazard is recorded with the deferred 401-refresh-retry note below, where it will +be built. + **Shape — `fn -> impl Future + Send`, not `async fn` (required).** The composed stack future must be `Send` (multithreaded tokio). `async fn` in a trait yields `fn -> impl Future` *without* a `Send` bound and offers no clean way to add one; @@ -168,7 +183,7 @@ impl> http_body::Body for Gu -> Poll, HttpError>>> { let this = self.project(); let frame = ready!(this.inner.poll_frame(cx)); - if frame.is_none() { *this.permit = None; } // eager release at stream-end + if !matches!(frame, Some(Ok(_))) { *this.permit = None; } // eager release: end OR error Poll::Ready(frame) } fn is_end_stream(&self) -> bool { self.inner.is_end_stream() } // MUST forward @@ -190,11 +205,15 @@ Two non-obvious correctness points the `/* delegate */` shorthand would hide: fails open.** The *same forwarding requirement applies to `ResponseBody` (0030 §3)*, which has the identical newtype-over-`Either` shape. A test asserts both newtypes report `size_hint`/`is_end_stream` identical to their inner body. -- **Release at the earlier of stream-end or drop.** The struct field alone gives - *drop-only* release — a body read to completion but still held keeps the permit, - wasting one of IBKR `/history`'s 5. So `poll_frame` `take()`s the permit on the - terminal frame (`None`), while the field stays for the early-abort case (a cancelled - read drops the still-`Some` guard). Dropping the guard is synchronous/runtime-free +- **Release at the earliest of terminal frame, mid-stream error, or drop.** The + struct field alone gives *drop-only* release — a body read to completion but still + held keeps the permit, wasting one of IBKR `/history`'s 5. So `poll_frame` + `take()`s the permit on **both** terminal outcomes: the clean end (`None`) *and* a + mid-stream `Some(Err(_))` (a connection reset during a `/history` transfer is just + as over as a clean end — in http-body 1.x practice an errored body yields no + further frames, and a consumer that holds the errored body for error context must + not pin a permit). The field stays for the early-abort case (a cancelled read + drops the still-`Some` guard). Dropping the guard is synchronous/runtime-free (decrement a counter, wake one `event-listener` waiter — safe in both `Drop` and `poll_frame`). @@ -204,9 +223,9 @@ Two non-obvious correctness points the `/* delegate */` shorthand would hide: permit (if any) is dropped at `call`-return per 0031 §3 (buffered work is done when the fetch returns). - `permit: Some(_)` — a **streaming** concurrency-scoped response (the real IBKR - `/history` case): the guard rides the body and releases at the earlier of - stream-end or drop. No caller discipline, no response-extension hand-off (0031 - rejected that). + `/history` case): the guard rides the body and releases at the earliest of + terminal frame, mid-stream error, or drop. No caller discipline, no + response-extension hand-off (0031 rejected that). Final assembled body type: `Guarded>`, opaque to adapters behind `impl HttpClient`. @@ -228,26 +247,33 @@ crate's dependency graph free of any async runtime. Rationale, in the order that decided it: -- **A semaphore needs no mock** (unlike `Timer`, which must fake a 15-minute - cooldown): acquiring permits is deterministic in real time, so tests use the real - thing. The `Timer`-style trait abstraction (considered option **Q**) buys nothing - and would thread a generic through `RateLimit` *and* `Guarded` — rejected as - over-engineering until a second backend supplies its own semaphore (YAGNI). -- **Neither candidate forces a backend's runtime.** The semaphore lives in the - `RateLimit` *layer*, never the leaf; and both `tokio::sync::Semaphore` and - `async_lock::Semaphore` are runtime-agnostic (no reactor). So a future non-tokio - backend is not precluded either way. -- The deciding axis is therefore **declared vs implicit neutrality.** `async-lock` - keeps `tokio` out of `net-http-api`'s graph, so the graph *states* runtime- - neutrality rather than relying on the reader knowing `tokio/sync` is reactor-free. - **Honest cost accounting:** option **P** (`tokio` with `default-features = false, - features = ["sync"]`) is *not* heavy — that subset is `mio`-free and runtime-free, - and since `tokio` is already a workspace dep, P would add **zero** new crates. **R** - adds **two** (`async-lock`, `event-listener`). So R's real price is two small - smol-rs crates bought purely for *graph-level* neutrality — the technical delta is - just "does `net-http-api`'s graph name `tokio`." Chosen deliberately on the - neutrality principle (ADR-0029's runtime-neutral-contract charter), with the thin - margin recorded so the trade reads straight. +- **A semaphore needs no *mock* — and needs no *trait* either.** Acquiring permits + is deterministic in real time, so tests use the real thing (unlike `Timer`, which + must fake a 15-minute cooldown). The `Timer`-style trait abstraction (considered + option **Q**) is rejected — but *not* on YAGNI grounds now that multi-backend is a + stated goal (below). It is rejected because it is **unnecessary**: `async_lock::Semaphore` + is already cross-runtime, so a smol/async-std backend reuses the *same* semaphore in + the `RateLimit` layer. No backend ever needs to *supply its own*, so there is + nothing to abstract; Q would only thread a dead generic through `RateLimit` *and* + `Guarded`. +- **The semaphore lives in the `RateLimit` *layer*, never the leaf**, and both + `tokio::sync::Semaphore` and `async_lock::Semaphore` are runtime-agnostic (no + reactor) — so neither candidate *precludes* a non-tokio backend at runtime. The + difference is what the **graph names**. +- **Multi-backend is a stated project goal (tokio now; smol/async-std later), so the + future-backend argument is real, not a hedge — and it decides R over P.** Under + option **P** (`tokio` with `default-features = false, features = ["sync"]`), a + future *smol* stack would still drag `tokio` into its dependency graph **purely for + the semaphore**, even though its leaf is smol — a non-tokio stack that names tokio. + Under **R** (`async-lock`), `net-http-api` names no runtime, so the whole smol stack + is genuinely `tokio`-free. That is the concrete payoff the two extra crates buy. + **Honest cost accounting:** P is *not* heavy — its subset is `mio`-free and + runtime-free, and since `tokio` is already a workspace dep, P adds **zero** new + crates; **R** adds **two** small smol-rs crates (`async-lock`, `event-listener`). + We pay those two crates deliberately: they are the price of a contract crate whose + graph is honestly runtime-neutral, which is what makes the planned smol backend a + clean drop-in rather than a tokio-tainted one. Consistent with ADR-0029's + runtime-neutral-contract charter. ### Seam #3 — the `build()` / `stack()` construction surface @@ -311,11 +337,20 @@ away and silently drops coverage to runtime-only. **`all()` must not drift** — the boot-time coverage guarantee is only as trustworthy as `all()`'s exhaustiveness, and a hand-written slice silently rots when a variant is -added. Two drift-proof options for the adapter (the *impl* side; `net-http-api`'s -trait stays dependency-free): derive it via **`strum::VariantArray`** (an *adapter* -dep, regenerated automatically), or — dependency-free — back `all()` with a slice and -add an **exhaustive-`match` test** (no wildcard arm) that fails to compile when a -variant is added without being listed. +added. So drift-proofing is a **contract obligation on the adapter's `all()`**, not a +menu: `all()` MUST be drift-proof — exhaustiveness **mechanically enforced**, by +compiler exhaustiveness checking or by codegen, **never hand-maintained**. E.g. a +`#[derive(strum::VariantArray)]`, or — dependency-free — a slice backed by a +**no-wildcard exhaustive-`match` test** that fails to compile when a variant is added. +A hand-written slice with no enforcement is **non-conforming**: it silently defeats +the boot-coverage check one crate over. The spec does **not** mandate a specific tool: +the *property* is the contract, the mechanism stays with the adapter (ADR-0003). And +it deliberately does not force `strum` — the no-wildcard match is checked by rustc's +own exhaustiveness checker (compiler-checked > macro-trusted), and mandating a +proc-macro dep to buy a property the compiler already gives for free would cut against +the repo's dep-minimalism *and* reach across the ADR-0003 boundary into the deferred +IBKR slice's dependency list. `net-http-api`'s `RateKey` trait itself stays +dependency-free. **Boot-time total pacing coverage (hardening beyond 0031 §3).** 0031 §3 validates config sanity and fails closed at runtime. For the order path that is a missed @@ -335,6 +370,23 @@ variant and forgetting to pace it becomes a **boot failure**, not a first-live-o 429. The runtime `Throttled` fail-closed (0031 §3) stays as an **unreachable backstop** (defense in depth). +**Call-site totality — absent directive fails closed (tightens 0031 §3).** The +config-side totality above still leaves a call-site hole under 0031's "absent +directive defaults to `Global`": an adapter that adds an endpoint and forgets to +stamp the `RateLimit` extension flies global-paced only — the silent +under-pacing path to the 429 penalty box, invisible to the boot check because the +gap is in the request, not the config. So the same principle (*explicit +classification only; absent is not a classification*) applies to the request +surface: a request with **no** `RateLimit` extension is **rejected fail-closed** +(the same non-retryable classification-error path as the missing-bucket backstop, +so `Retry` never spins on it). Every request carries an explicit `Scope` — unlisted +endpoints stamp `Scope::Global` deliberately, opt-outs stamp `Scope::None` +deliberately; forgetting entirely is a loud first-use error in shadow, not a silent +under-pace. Performance is unchanged: the layer already performs the one typed +extension lookup per call, and the `None` arm returns an error instead of +substituting `Global`; the call-site cost is one small typed insert on +previously-unstamped endpoints. + **Return + errors.** Fully opaque `impl HttpClient` — adapters never name `Tracing>>>>>`; they use `Self::Body` through the `http_body::Body` trait. `BuildError` is a @@ -347,9 +399,26 @@ concurrency max < 1 — missing global). No panic (CLAUDE.md). separate **`oath-adapter-net-http-mock`** crate, depending on `net-http-api` and pulled only through `[dev-dependencies]`, has **no production dependency edge** — the feature graph cannot turn it on. It provides `MockClient` (canned responses, -recorded requests) and `MockTimer` (fake clock for the timing layers). The invariant -"the mock cannot exist in production" is enforced by the dependency graph, not by -convention. +recorded requests). The invariant "the mock cannot exist in production" is enforced +by the dependency graph, not by convention. + +**`MockTimer` relocates into its own dev-only crate, beside its contract.** `Timer` +lives in `net-api` (the transport-neutral contract, ADR-0029), not in the HTTP +stack; `MockTimer` fakes `Timer`, so it belongs one level below `net-api`, not +inside the HTTP mock. **`MockTimer` already ships** in `net-http-mock` +([timer.rs](../../../crates/adapter/net/http/mock/src/timer.rs), PR #66) — so this is +a *relocation*, not a greenfield crate: move it to a small dev-only +**`oath-adapter-net-mock`** crate (depending only on `net-api`) and re-point the +http-stack tests. **The move is time-critical, not speculative:** `net-ws-mock` +already exists and its header states *"`MockTimer`/`MockSpawn` arrive with the +resilience slice (ADR-0033 §9)"* — that slice is imminent (its design spec is already +drafted). Leaving `MockTimer` in `net-http-mock` forces that slice to either +dev-depend a *WS* mock on an *HTTP* mock — a nonsense edge across the crate cut — or +grow a second, drift-prone `MockTimer`. Extracting now lets both stacks share one +fake clock. Each mock then sits exactly one level below the contract it fakes, +mirroring how `Timer` sits below `HttpClient`. The http-stack tests dev-depend on +**both** `net-mock` and `net-http-mock` (a test crate pulling two dev-deps is free); +the same production-reachability guard applies to each. ### Recorded tradeoffs and deferred refinements @@ -366,7 +435,13 @@ convention. `Retry` order already composes to do it (a `Retry` on 401 re-invokes `authorize`, which refreshes). Implement in the `Retry` slice with guards: **once only**, **definitive 401 only** (never a timeout/5xx), **venue opt-in**. Not part of this - construction surface. + construction surface. **Concurrent-refresh hazard (recorded for that slice):** when + a venue rotates tokens, multiple in-flight requests can 401 at once and each + re-invoke `authorize` — the `AuthSource` impl **must single-flight its token + refresh** (coalesce concurrent refreshes over its own token cache; the trait + deliberately leaves this to the impl — see Seam #1), so N concurrent 401s trigger + one refresh, not N. Moot for IBKR's `NoAuth`; a hard requirement for the future + OAuth `AuthSource`. - **High-throughput perf cliffs (deferred, YAGNI).** Per-bucket mutex contention, `async-lock`-vs-`tokio` semaphore behaviour under load, and per-request span cost are all irrelevant at IBKR's ≤10 req/s (network + deliberate pacing waits dominate by @@ -393,12 +468,16 @@ landed append-only): - **ADR-0031 §3** — replace `enum Permit { Rate, Concurrency(OwnedSemaphorePermit) }` with a plain `Option` carried by a new `Guarded` body newtype (the `Rate` arm was redundant with `None`); specify that `Guarded` - forwards `is_end_stream`/`size_hint` and eagerly releases on the terminal frame, and - that body-attach (vs tower's response-future) is the deliberate, stronger guarantee - for streaming concurrency; correct the §3 wording "stream-end/drop" → "the earlier of - stream-end or drop"; add the `LimitDecl` total-coverage requirement and the + forwards `is_end_stream`/`size_hint` and eagerly releases on **both** terminal + outcomes — the clean end (`None`) *and* a mid-stream error frame — and that + body-attach (vs tower's response-future) is the deliberate, stronger guarantee + for streaming concurrency; correct the §3 wording "stream-end/drop" → "the + earliest of terminal frame, mid-stream error, or drop"; add the `LimitDecl` total-coverage requirement and the `RateKey::all()` boot-time check, demoting the runtime `Throttled` path to an - unreachable backstop. + unreachable backstop. And: replace *"absent directive defaults to `Global`"* with + **absent directive fails closed** (non-retryable classification error) — with + config-side totality in place, default-`Global` was the one remaining silent + under-pacing path; explicit `Scope::Global` is now the way to say "global only." ## Testing @@ -416,11 +495,12 @@ The construction surface is verified through the mock crate, deterministically: `is_end_stream` identical to their inner body (the metadata-forwarding fix); a `size_hint().upper()` size-guard is not silently widened to unbounded by the wrapper. - **Permit lifetime** — a concurrency-scoped **buffered** response releases its permit - at `call`-return (`permit: None`); a **streaming** one releases at the *earlier of - stream-end or drop*: assert the (N+1)-th concurrent acquire unblocks when the N-th - body is **read to its terminal frame** (eager `take()`), and in a separate test when - the N-th body is **dropped early** (mid-read); a rate-scoped response carries - `permit: None`. + at `call`-return (`permit: None`); a **streaming** one releases at the *earliest of + terminal frame, mid-stream error, or drop*: assert the (N+1)-th concurrent acquire + unblocks when the N-th body is **read to its terminal frame** (eager `take()`), + when the N-th body yields a **mid-stream error frame** (permit released even while + the errored body is still held), and in a separate test when the N-th body is + **dropped early** (mid-read); a rate-scoped response carries `permit: None`. - **Acquire fairness** — an ordering test pins the no-starvation behaviour `RateLimit` inherits from `async-lock`/`event-listener` (FIFO-ish), recorded as an *inherited* property rather than an API guarantee (not strictly depended on at 5 permits, but @@ -428,9 +508,14 @@ The construction surface is verified through the mock crate, deterministically: - **Boot-time coverage** — `build()` with a `RateLimitConfig` missing a `K` variant returns `Err(BuildError::UndeclaredKey)`; a total config builds; bad policy params and a missing global are rejected at construction. +- **Call-site totality** — a request with **no** `RateLimit` extension is rejected + fail-closed (non-retryable; `Retry` does not re-attempt it), never sent + global-paced; an explicit `Scope::Global` request spends only the global bucket; + an explicit `Scope::None` request acquires nothing. - **Production-reachability guard** — a CI assertion that - `cargo tree -e no-dev -i oath-adapter-net-http-mock` yields no non-dev dependents - (the mock cannot reach a release build through the feature/dep graph). + `cargo tree -e no-dev -i oath-adapter-net-http-mock` **and** + `cargo tree -e no-dev -i oath-adapter-net-mock` each yield no non-dev dependents + (neither mock can reach a release build through the feature/dep graph). ## Dependencies @@ -442,7 +527,10 @@ The construction surface is verified through the mock crate, deterministically: - `oath-adapter-net-http-hyper` — owns the only `hyper`/`tokio`/`rustls` deps and `TokioTimer`. - `oath-adapter-net-http-mock` — **new**, dev-only; depends on `net-http-api`; pulled - by other crates only through `[dev-dependencies]`. Provides `MockClient` + `MockTimer`. + by other crates only through `[dev-dependencies]`. Provides `MockClient`. +- `oath-adapter-net-mock` — **new**, dev-only; depends only on `net-api`; pulled by + other crates only through `[dev-dependencies]`. Provides `MockTimer` (shared by the + HTTP and future WS stacks). All new workspace deps go through `[workspace.dependencies]` per the repo pattern. @@ -451,8 +539,9 @@ All new workspace deps go through `[workspace.dependencies]` per the repo patter - The three contracts (`AuthSource`, `Guarded` (permit-carrying) + the semaphore choice, `stack()`/`build()` + `RateKey`/`RateLimitConfig`/`BuildError`) are implemented as specified, with the mock crate and the tests above. -- The two ADR amendments are landed (as ADR edits or a short follow-up ADR per the - repo's append-only norm — decided in the implementation plan). +- ADR-0034 records the construction-surface decisions (merged in #66); the four + follow-up refinements are captured in its append-only **Amendments (2026-07-04)** + section and each lands with its implementation slice. - `just ci` green (fmt, lint = deny, test + doctests, doc, deny, typos, …); no new warnings; no `unsafe`/`unwrap`/`expect`/indexing in non-test code. - `CHANGELOG.md` `[Unreleased]` updated. @@ -461,12 +550,17 @@ All new workspace deps go through `[workspace.dependencies]` per the repo patter ## Open questions (for the implementation plan) -1. **ADR form** — fold the two amendments into 0030/0031 directly, or land a short - **ADR-0034** that records them and the construction-surface decisions? (The repo has - landed 0029–0031 append-only; leaning a standalone ADR. 0032/0033 are reserved by the - WebSocket transport pair, so this workstream's ADR is **0034**, not 0032.) +1. **ADR form** — *Resolved:* standalone **ADR-0034**, written and merged in PR #66 + (`4adeac1`), with append-only pointers added to 0030/0031. Today's four refinements + (call-site fail-closed, `Guarded` mid-stream-error release, the multi-backend + semaphore rationale, the `net-mock` split) are recorded as a dated append-only + **Amendments (2026-07-04)** section in ADR-0034; each lands with its implementation + slice (the `Guarded` change touches #66's shipped code). 2. **Slice boundaries** — does the construction surface land as one PR or several (e.g. `AuthSource` + `Auth` layer; `Guarded`/semaphore + `RateLimit`; `stack`/`build`/coverage; the mock crate)? A `writing-plans` concern. -3. **`MockTimer` home** — confirmed in `net-http-mock` alongside `MockClient`, or a - thinner test-util tied to `net-api` where `Timer` is defined? +3. **`MockTimer` home** — *Resolved:* its own dev-only **`oath-adapter-net-mock`** + crate (`crates/adapter/net/mock`), beside the `Timer` contract in `net-api`, so + the WS stack (ADR-0032/0033) shares one fake clock without an HTTP dep edge. + `net-http-mock` keeps only `MockClient`. (See Seam #3, "`MockTimer` splits into + its own dev-only crate.")