One idempotency key per logical request across retry attempts (#73)

docs/deviations.md

@@ -143,7 +143,7 @@ non-bridge clause survives and is enforced as an ordinary obligation on `send()`
 
 **Verified.** One engine — `runWithRetry` (`packages/core/src/retry/engine.ts:367`) — with exactly two thin
 callers: the pillar step (`packages/core/src/retry/retry-step.ts:151`) and the dispatch adapter
-(`packages/core/src/retry/retry-dispatch.ts:55`). `totalTimeoutMs` is `readonly totalTimeoutMs?: number |
+(`packages/core/src/retry/retry-dispatch.ts:85`). `totalTimeoutMs` is `readonly totalTimeoutMs?: number |
 undefined` and undefined by default (`packages/core/src/retry/settings.ts:27`), pinned by a test named for
 `RETRY-28` (`packages/core/src/retry/settings.test.ts:20`).
 
@@ -482,6 +482,7 @@ frozen tree and is amended only deliberately, by hand. When §10 is next amended
 | **`XCUT-16`'s replay guard is keyed on whether the hop was guarded, not on whether the replacement looks credentialed.** `XCUT-16` and `AUTH-28` say the guard applies "on any path where a credential will be attached", and carve out "a deliberately credential-free re-issue MAY proceed over any scheme". Deciding which of the two a challenge replacement is cannot be done by reading header names: the step's own `ApiKeyCredentialConfig.headerName` stamps whatever header the caller names, and a `challengeHook` may invent a carrier this step has never been told about. The port therefore reads "a credential will be attached" as a property of the HOP — if the outbound pass ran the HTTPS guard, so does the replay, whatever URL and headers the hook chose. **Strictly wider than the requirement's letter**, and knowingly so: it refuses a downgraded replacement that carries no credential at all, on a hop that is credentialed. The carve-out is preserved where it is observable — a `NO_AUTH` hop is never guarded outbound, and its replay is guarded only when the replacement carries `Authorization` or `Proxy-Authorization`, which is the previous rule kept as a second arm. *Rejected:* deriving the credential-carrying header names from configuration, which misses the hook-invented carrier and is the shape that let the reported leak through | audit #67 / #71 | 2026-09-04 | `docs/product-spec/19-cross-cutting-invariants-and-policies.md:44` is the requirement and its carve-out. `packages/core/src/auth/auth-step.ts:389` sets `OutboundPlan.guarded`; `:564-575` is `guardReplayScheme` and its two arms. Pinned by "a replacement carrying a NON-standard credential header over plaintext is refused" and "a header-free replacement over plaintext is refused too" in `packages/core/src/auth/auth-step.test.ts`, and by the "XCUT-16: a guarded hop stays guarded across a challenge replay" block in `tests/conformance/xcut/security-by-default.conformance.test.ts` | not yet in §10 |
 | **`ASYNC-21`'s "MUST NOT close the caller-owned source on any termination" is not honoured: the RxJS SSE adapter takes ownership and closes.** `sseEvents$` and `typedSse$` pass `() => stream.close()` as `fromAsyncIterable`'s `release`, and RxJS runs a subscriber's finalizer on *every* termination — unsubscription, end-of-source and a source error alike, which is the complete list the clause names. **Kept, deliberately, on two grounds.** (1) **The clause has no subject on this platform.** It presumes a source whose iterator return leaves the source open; this port's `SseStream` is deliberately not that one. `#iterate`'s `finally` calls `#releaseQuietly()`, so the resource is released whenever the runtime drives `return()` — which `fromAsyncIterable` must do exactly once (`ASYNC-6`), and which a plain `for await` with `break` does too. Removing the callback would change which channel reports a release failure and when the release runs, not whether the caller-owned source ends up closed. (2) **The ordering is load-bearing.** The release runs *ahead of* `iterator.return()` because an async generator's `return()` queues behind a suspended `next()`, and an SSE stream idling between events is parked in exactly that pull — so without the callback an `unsubscribe()` stays pending until the server next sends a byte, holding the socket open indefinitely. Measured: deleting the two `release` arguments turns four cases red — the two suspended-pull ones, as "the teardown did not settle within 500ms", and the two pre-existing idle-unsubscribe assertions — while every exactly-once release count stays green, which is the shape of the claim. Pagination attaches no release for the complementary reason: a `Paginator`'s pulls are bounded HTTP exchanges, never a wait on a server that may never answer. *Rejected:* dropping the callback to match the letter (reintroduces the hang for no change in what closes). *Rejected:* a caller-facing `{ownership}` option (two behaviours to document for a case with one correct answer). The public TSDoc and `packages/rx/README.md` now state the transfer outright — subscribing hands the stream over, do not close it yourself and do not iterate it afterwards — rather than leaving the `ASYNC-21` citation on the doc comment's first line to read as satisfied | audit #67 / #75 | 2026-09-05 | `packages/rx/src/sse.ts:46` and `:73-75` are the two `release` arguments; `packages/rx/src/from-async-iterable.ts:103-108` is the teardown that runs one on every termination; `docs/product-spec/18-asynchronous-runtime-adapter-contract.md:42` is the requirement. Ground 1: `packages/core/src/sse/stream.ts:136-139` (`#iterate`'s `finally` → `#releaseQuietly()`) with `:117-121` (`close()` memoized, `SSE-28`). Ground 2: `packages/rx/src/from-async-iterable.ts:44-48` states the ordering and why. Pinned by the two `resource ownership` blocks in `packages/rx/src/sse.test.ts`, which count the release the OWNED resource sees rather than `SseStream.close()` calls — the facade memoizes, so a facade-level count reads "once" however many paths call it — and by "SSE ownership transfer releases once on Node" in `tests/node-conformance/rx-bridge.test.mjs`. Phase 8b marked `ASYNC-21` ✅ with this clause dropped from its gist (`docs/work/mvp/phase8/phase8b/2026-07-28-phase8b-async-runtime-checklist.md:67`); that is a dated record and is left as written. The other half is `SSE-41`'s own "documented source ownership" clause, which the same checklist marked ✅ (`:74`) on the strength of documentation that named unsubscription only — completed by the TSDoc and README rewrite this row accompanies | not yet in §10 |
 | **`AUTH-22`'s "emit cnonce/nc/qop only when qop is negotiated" is not applied to `cnonce` for a `-sess` algorithm.** A `-sess` HA1 is `H(H(user:realm:pass):nonce:cnonce)` (RFC 7616 §3.4.2), so the client nonce is an *input to the hash* for `MD5-sess` and `SHA-256-sess` whatever `qop` the challenge offered. The port implemented AUTH-22 to the letter: it drew a fresh cnonce, folded it into HA1, and then omitted it from the header whenever `qop` was absent — a response no server can verify, because it has no way to reconstruct HA1. AUTH-30 bounds the re-challenge replay to one 401, so every such exchange simply failed. **`cnonce` is now emitted for any `-sess` algorithm; `nc` and `qop` stay conditional exactly as AUTH-22 says**, because RFC 2069's response input is `H(HA1:nonce:HA2)` and carries no nonce count, so emitting one would advertise a count the response was not computed over. RFC 7616 §3.4 states the wider rule outright — "cnonce: This parameter MUST be used by all implementations". AUTH-22's clause is RFC 2617's RFC 2069-compatibility form, written before `-sess` existed, and the requirement's own AUTH-15 mandates both `-sess` algorithms, so the two sentences cannot both be followed. *Rejected:* declining a `-sess`-without-`qop` challenge instead, which turns every such server into a guaranteed 401 for no security gain, when the value the server needs has already been computed | audit #67 / #74 | 2026-09-05 | `packages/core/src/auth/digest.ts:345-350` (the `-sess` HA1 that consumes the cnonce) against `:405-408` (`buildHeaderValue`, where the `else if` now emits it); `docs/product-spec/11-authentication.md:18` and `docs/product-spec/appendix-c-consolidated-normative-requirement-index.md:357` are AUTH-22's wording. Pinned by the `digestHandler -sess without qop (AUTH-17/AUTH-22)` block in `packages/core/src/auth/digest.test.ts` — one row asserting the header carries `cnonce` and neither `nc` nor `qop`, one recomputing the response from the header's OWN cnonce so a value drawn twice would fail — and by the `MD5-sess, no qop` vector in the same file | not yet in §10 |
+| **`RETRY-44`'s "downstream chain" is read as everything BELOW the retry point, which in the recovery stack excludes the request chain.** The requirement has two clauses: each attempt re-executes the downstream chain with fresh per-attempt state, and "upstream steps MUST NOT mutate the shared in-flight request between attempts". The port originally read the first clause as covering the *whole* recovery chain and re-ran `RequestRecoveryChain.apply()` per attempt, with a test that said so by name. That makes `packages/core/src/recovery/idempotency-key.ts` generate a fresh key on every attempt, so three attempts of one logical request reach the server as three unrelated writes — the precise failure `RECOV-32` exists to prevent, and the opposite of what that step's own `@public` TSDoc promises. **The chain is now applied once, above the loop; each attempt re-executes transport plus response chain over `stampAttempt`'s fresh copy of the prepared request.** Under this reading both clauses hold and the second holds *by construction*: upstream steps cannot mutate the in-flight request between attempts because they no longer run between attempts. The pillar stack is untouched — there "downstream" is the forked continuation (`ctx.fork()`), and `retryStep` still re-drives it per attempt. *Rejected:* memoizing the key on the template (a `WeakMap` keyed by the `Request` instance) — a caller who deliberately sends one immutable `Request` value twice would replay the key and have the server drop a genuine second call. *Rejected:* re-running the chain over the *prepared* request each attempt — the chain would read its own output, which is clause two's mutation in different clothes, and every shipped and caller-written step would have to be proven idempotent. One consequence recorded rather than assumed: the re-send gate (`RETRY-5`/`RECOV-18`) now judges the prepared request rather than the caller's, which is what a retry would actually re-send | audit #67 / #73 | 2026-09-05 | `packages/core/src/retry/retry-dispatch.ts:83-88` (the chain applied once, then `runWithRetry`) against `:27-33` (the per-attempt half); `packages/core/src/recovery/orchestrator.ts:62` (`prepareRequest`) and `:117` (`dispatchPrepared`); `packages/core/src/retry/engine.ts:243` is the gate that now reads the prepared request. `RETRY-44`'s wording is `docs/product-spec/09-retry-and-resilience.md:35` and `docs/product-spec/appendix-c-consolidated-normative-requirement-index.md:306`. Pinned by `packages/core/src/retry/retry-dispatch.test.ts:126` (chain applied once), `:169` (one `generate()`, one key on three sends), `:201` (the `RETRY-38` ordinal varies while the key does not) and `:227` (a request-chain throw is not retried and meets the recovery phase exactly once) | not yet in §10 |
 
 ### Proposed erratum for `PIPE-40` (drafted 2026-09-04, not applied)
 

packages/core/src/recovery/idempotency-key.ts

@@ -30,9 +30,17 @@ export interface IdempotencyKeyOptions {
 /**
  * A `RequestStep` that stamps an idempotency key on write requests (RECOV-32).
  *
- * Runs ONCE per call, upstream of retry -- not per attempt. `retry/attempt-stamp.ts` is its sibling:
- * that one writes the attempt ordinal on each per-attempt copy and preserves whatever this wrote
- * (RETRY-38), so the server sees one stable key across every retry of the same logical request.
+ * Runs ONCE per logical request, upstream of retry -- not per attempt. `retry/attempt-stamp.ts` is
+ * its sibling: that one writes the attempt ordinal on each per-attempt copy and preserves whatever
+ * this wrote (RETRY-38), so the server sees one stable key across every retry of the same logical
+ * request.
+ *
+ * **That is a property of the composition, and the SDK's own retry adapter is what supplies it**:
+ * `retry/retry-dispatch.ts` applies the `RequestRecoveryChain` once, above the retry loop, and each
+ * attempt re-sends a copy of the request it produced. On its own a step can only promise RECOV-32's
+ * letter -- `generate()` is invoked at most once per *application* to an applicable request -- so a
+ * caller who re-applies their own chain per attempt will get a fresh key per attempt. Install the
+ * chain once and let the retry layer sit below it.
  *
  * @param options - the key strategy plus the header name, method set, and existing-key policy.
  * @returns the request step to install in a `RequestRecoveryChain`.