From 7f5bcafc78e9bfbfd4c66d0ff0a556513bddd2ab Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 10:24:32 +0200 Subject: [PATCH 01/18] chore(drive): spec + dispatch plan for the rpc-cold-start slice Contract-declared idempotence for rpc methods, the bounded client-side retry for marked methods only, and the RPC face of the PRO-217 canary, inheriting every rule from the cold-start canary's rebuild. Execution held pending operator sign-off. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../slices/rpc-cold-start/plan.md | 51 +++++++++ .../slices/rpc-cold-start/spec.md | 104 ++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 .drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md create mode 100644 .drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md new file mode 100644 index 00000000..75c7947b --- /dev/null +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md @@ -0,0 +1,51 @@ +# Dispatch plan: rpc-cold-start + +Contract source: [spec.md](spec.md). Branch: +`claude/streams-cold-start-rpc-37e5c1` off merged main (`2bafbbf`). Three +dispatches, sequential; reviewer round after D1+D2 together, then D3. +Orchestrator owns all docs (`gotchas.md`, `.drive/`, `docs/`) — implementers +report staleness, never edit. Evidence rules from the streams slice apply +verbatim: raw program output only; any number an implementer reports is +checked against the code's real format strings before it is believed. + +## D1 — the flag and the client retry (rpc package) + +**Outcome:** `rpc({ input, output, idempotent: true })` carried on +`__cmp[method]`; `makeClient` applies the bounded idempotent-retry policy +(streams numbers: 250 ms / ×2 / 5 s cap / 5 attempts / jitter; retry thrown +network errors + 5xx + 429, never other 4xx) to marked methods only; +unmarked methods byte-identical to today. Comments state the generic +idempotence semantics, with PRO-217/PRO-219 + the canary named as the +motivating urgency. No prisma-cloud imports. + +**Completed when:** the spec's wire-counted tests are green with teeth +confirmed red-by-mutation (retry deleted → marked-method test fails; +retry widened to unmarked → one-request test fails; 4xx retried → 404 test +fails); `test-d` pins the signature; rpc + dependent packages green; repo +checks green; committed with DCO dual sign-off. + +## D2 — the canary (scripts + CI) and the example flags + +**Outcome:** `scripts/rpc-cold-start-canary.ts` + `-classify.ts` + tests, +cloned from the cold-start canary's proven contract (spaced ≥60 s samples +including before #0, promote-race trigger, log-confirmed coldness with the +2 s margin, 14-hold bug-gone budget, first-close early exit, `MAX_RUN_MS`, +requirable exits); job in `e2e-deploy.yml` over `examples/storefront-auth`, +probing an unmarked method; `examples/store` catalog reads gain +`idempotent: true`, writes stay unmarked. Live rounds during development +must reproduce or honestly fail to reproduce the close, raw output only. + +**Completed when:** classify tests green with confirmed teeth; +`test:scripts` green; a live canary run reports `bug-present` (or the +implementer stops and reports why not — a clean run today means a broken +canary, not a fixed platform); workspace left clean with counts; committed. + +## D3 — review round, live re-proof, docs, PR + +**Outcome:** hostile reviewer pass over D1+D2 (priorities: the retry can +never touch an unmarked method; the canary cannot be masked by the +compensation; the numbers in every report are real); findings closed; a +full live round (deploy storefront-auth, canary verify, destroy); gotchas' +PRO-217 entry gains the RPC face and removal guard (orchestrator writes +it); PR opened against main with the slice narrative, review requested from +Will. No auto-merge armed; merge only on his word. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md new file mode 100644 index 00000000..b23ab19f --- /dev/null +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md @@ -0,0 +1,104 @@ +# Slice: RPC cold-start handling — contract-declared idempotence, with its own canary + +## At a glance + +`rpc()`'s `makeClient` carries no cold-start handling: every service-to-service +RPC edge hits PRO-217's intermittent socket close raw on a cold target — and +PRO-217 is **live**, reproduced repeatedly during the streams slice +(2026-07-17: multiple `502 … socket connection was closed` on log-confirmed +cold starts). Unlike streams, every RPC call is a POST with no +transport-level idempotency signal, so a blanket retry in `makeClient` would +be exactly the double-execution hazard the streams append work fenced off. + +The safe shape, settled with Will (2026-07-17): **a method opts in at its +definition** — `rpc({ input, output, idempotent: true })` — and `makeClient` +applies a bounded cold-start backoff to marked methods only. Everything else +keeps today's fail-fast behavior with the failure surfaced. Own canary, +cloned from the (now trustworthy) cold-start canary's contract. + +## The design + +### The flag + +- `rpc({ input, output, idempotent: true })` — optional, default absent + (= not idempotent, never retried). The flag rides on the method's runtime + value in `__cmp[method]` beside the two schemas, exactly where `makeClient` + already reads them. +- Declaring it is the METHOD AUTHOR's claim ("calling this twice is the same + as calling it once"), stated where the method is defined — the one place + that claim can be reviewed. Nothing infers it; nothing upgrades it. + +### The retry (client-side only; serve() unchanged) + +- `makeClient` wraps marked methods in a bounded backoff mirroring the + streams client's policy and numbers (250 ms initial, ×2, 5 s cap, 5 + attempts, jittered): retry thrown network errors, 5xx, and 429; never any + other 4xx (a real protocol answer surfaces on the first try). Unmarked + methods get today's single attempt, byte-for-byte. +- **Layering note, resolved:** rpc is framework-layer (target-agnostic), so + the policy is stated as generic idempotent-retry semantics — correct on + any transport for any method whose author declares idempotence — with the + comment naming PRO-217/PRO-219 as the motivating platform behavior and the + new canary as the removal trigger for the *urgency*, not the mechanism. + No prisma-cloud import enters the rpc package. + +### The canary (PRO-217, RPC face) + +A sibling of `scripts/cold-start-canary.ts`, inheriting every hard-won rule +of its 2026-07-17 rebuild — these are requirements, not suggestions: + +- **Trigger:** fresh deployment of the target service via create → upload → + start → race the promote call (never wait for `running`); first-touch RPC + call through the consumer the instant promote succeeds. +- **Cadence:** ≥60 s between samples, including before sample #0 — + back-to-back promotions produce ~1 s boots the bug does not live in. +- **Coldness is proven, not inferred:** the touch counts only if the + deployment's own boot log shows it was sent before the server's listening + line, outside the 2 s cross-clock margin; no latency guessing. +- **Statistical bug-gone rule:** `bug-gone` requires 14 confirmed cold-start + holds (20% target close rate, ≤5% false-clean chance); any close is + decisive `bug-present`; anything else inconclusive → warning, exit 0. +- **Probe an UNMARKED method**, so the raw platform behavior stays + observable — the compensation must not be able to mask the canary. +- Requirable exits: bug present → 0 (green), conclusively gone → 1 (red, + message names what to delete: the retry policy's cold-start framing, this + canary, the gotchas paragraph), inconclusive → 0 + `::warning::`. +- Rides `examples/storefront-auth` (the minimal storefront → auth RPC edge) + through the existing deploy-verify-destroy action; own `-classify.ts` + module with unit tests, own job in `e2e-deploy.yml`, NOT in the required + set until Will adds it. + +### The example + +`examples/store`'s catalog contract gets the flag where it is true — +`listProducts`, `getProduct`, `getSpecial` are idempotent reads; +`rotateSpecial` and `placeOrder` stay unmarked — so the example documents +the judgment call the flag exists to force. + +## Verification bar + +- **Wire-counted, mutation-verified tests** in the rpc package (the streams + append-test pattern): a marked method against a transport that 503s then + succeeds → resolves, with the retry counted at the stub transport; an + UNMARKED method against the same transport → rejects after exactly ONE + request, re-asserted after a settle window; a marked method against a 404 + → rejects after exactly one request (4xx never retried). Each verified red + by deleting the behavior it pins. +- Type-level: the flag is optional and absent-by-default; `test-d` pins that + marking a method does not change its call signature. +- Canary classify tests extended from the cold-start suite's shapes, + including the never-went-cold and sample-budget rules; teeth confirmed. +- Live round: the canary run against a fresh deploy, raw output only — + expected verdict today is `bug-present` (the bug is live). A `bug-gone` + from a run today means the canary is broken; investigate, do not report + it as a result. +- Repo checks green: typecheck, lint, casts delta 0, depcruise, + `test:scripts`. + +## Out of scope + +- Retrying non-idempotent calls under any policy. Not negotiable — the + no-double-execution reasoning is recorded in the streams design docs. +- Server-side (serve()) changes; the retry is a client concern. +- Adding the canary to the required-checks list (Will's manual step). +- Typed `streamDef({ event })` and the streams follow-ups — separate slice. From 771ed7b5848594f5b79a784b65d7577a067d7863 Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 11:02:38 +0200 Subject: [PATCH 02/18] chore(drive): the rpc slice becomes idempotency keys, not a boolean Will's design supersedes the per-method idempotent flag: the protocol requires a client-minted key on every call, serve() implements single- flight and replay within the retry envelope, handlers may use the key for durable control but need not. Every method becomes safely retryable with no declaration to trust. Spec and plan rewritten; scope calibration and the accepted cross-instance residual recorded in design-notes. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../forcing-function-apps/design-notes.md | 17 ++ .../slices/rpc-cold-start/plan.md | 93 +++++---- .../slices/rpc-cold-start/spec.md | 189 ++++++++++-------- 3 files changed, 170 insertions(+), 129 deletions(-) diff --git a/.drive/projects/forcing-function-apps/design-notes.md b/.drive/projects/forcing-function-apps/design-notes.md index 1ea4fbaf..62ce1917 100644 --- a/.drive/projects/forcing-function-apps/design-notes.md +++ b/.drive/projects/forcing-function-apps/design-notes.md @@ -191,3 +191,20 @@ shipped inside Part B's refactor (implicit create, read-creates, contentType dropped), the complete unenforced-invariant inventory (T1–T9), deviations from the design doc, and decisions R1–R6 awaiting his ruling. No further dispatches until he rules. + +## RPC idempotency keys — settled design (2026-07-17, Will) + +Supersedes the `idempotent: true` per-method boolean considered earlier the +same day. The RPC protocol requires an `Idempotency-Key` on every call +(client-minted per logical call, reused across retries); `serve()` enforces +it, runs single-flight per key, and replays completed answers within the +retry envelope; handlers may read `ctx.idempotencyKey` for their own durable +control but need not. Rationale: a boolean is a human claim the framework +cannot check — the key is a mechanism it enforces, making every method +safely retryable with no declaration. Scope calibration from Will: this +protocol serves same-network service-to-service calls and guards the +cold-start bug specifically; in-memory control is sufficient, and the +cross-instance residual window is documented and accepted, not closed. +The bounded retry becomes permanent protocol semantics — the PRO-217 canary +retires itself when the platform heals, never the retry or the keys. +Slice: [slices/rpc-cold-start/](slices/rpc-cold-start/spec.md). diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md index 75c7947b..c9b3d671 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md @@ -1,51 +1,56 @@ -# Dispatch plan: rpc-cold-start +# Dispatch plan: rpc-cold-start (idempotency keys) Contract source: [spec.md](spec.md). Branch: `claude/streams-cold-start-rpc-37e5c1` off merged main (`2bafbbf`). Three -dispatches, sequential; reviewer round after D1+D2 together, then D3. +dispatches, sequential; hostile reviewer round after D1+D2, then D3 closes. Orchestrator owns all docs (`gotchas.md`, `.drive/`, `docs/`) — implementers report staleness, never edit. Evidence rules from the streams slice apply -verbatim: raw program output only; any number an implementer reports is -checked against the code's real format strings before it is believed. - -## D1 — the flag and the client retry (rpc package) - -**Outcome:** `rpc({ input, output, idempotent: true })` carried on -`__cmp[method]`; `makeClient` applies the bounded idempotent-retry policy -(streams numbers: 250 ms / ×2 / 5 s cap / 5 attempts / jitter; retry thrown -network errors + 5xx + 429, never other 4xx) to marked methods only; -unmarked methods byte-identical to today. Comments state the generic -idempotence semantics, with PRO-217/PRO-219 + the canary named as the -motivating urgency. No prisma-cloud imports. - -**Completed when:** the spec's wire-counted tests are green with teeth -confirmed red-by-mutation (retry deleted → marked-method test fails; -retry widened to unmarked → one-request test fails; 4xx retried → 404 test -fails); `test-d` pins the signature; rpc + dependent packages green; repo -checks green; committed with DCO dual sign-off. - -## D2 — the canary (scripts + CI) and the example flags - -**Outcome:** `scripts/rpc-cold-start-canary.ts` + `-classify.ts` + tests, -cloned from the cold-start canary's proven contract (spaced ≥60 s samples -including before #0, promote-race trigger, log-confirmed coldness with the -2 s margin, 14-hold bug-gone budget, first-close early exit, `MAX_RUN_MS`, -requirable exits); job in `e2e-deploy.yml` over `examples/storefront-auth`, -probing an unmarked method; `examples/store` catalog reads gain -`idempotent: true`, writes stay unmarked. Live rounds during development -must reproduce or honestly fail to reproduce the close, raw output only. +verbatim: raw program output only; every reported number is checked against +the code's real format strings before it is believed. + +## D1 — the keyed protocol: client retry + serve() idempotency control + +**Outcome:** `makeClient` mints one `Idempotency-Key` per logical call, +reuses it across a bounded retry (250 ms / ×2 / 5 s cap / 5 attempts / +jitter; retry network errors + 5xx + 429, never other 4xx). `serve()` +enforces the key (keyless → loud 400), runs single-flight per in-flight +key, replays completed 2xx/4xx answers for ~60 s under an LRU bound, does +not cache 5xx/throws, and passes `ctx.idempotencyKey` as an optional second +handler argument. No `idempotent` flag exists. No prisma-cloud imports — +this is generic RPC semantics; PRO-217/PRO-219 are named as motivating +urgency only. + +**Completed when:** every wire-counted and serve() test in the spec is +green with teeth confirmed red-by-mutation (including the same-key-across- +attempts and fresh-key-per-call assertions); one-argument handlers still +typecheck (`test-d`); both rpc-consuming example suites pass unchanged; +repo checks green; committed with DCO dual sign-off. + +## D2 — the canary (scripts + CI) + +**Outcome:** `scripts/rpc-cold-start-canary.ts` + `-classify.ts` + unit +tests, inheriting the cold-start canary's proven contract wholesale +(promote-race trigger, ≥60 s spacing including sample #0, log-confirmed +coldness with the 2 s margin, 14-hold bug-gone budget, first-close early +exit, `MAX_RUN_MS`, requirable exits, bug-gone message that retires the +canary + gotchas entry but NEVER the retry or keys); job in +`e2e-deploy.yml` over `examples/storefront-auth`, probing the auth +service's rpc endpoint with a bare single-attempt `fetch` and a manually +minted key — never through a framework client, which would mask the bug. **Completed when:** classify tests green with confirmed teeth; -`test:scripts` green; a live canary run reports `bug-present` (or the -implementer stops and reports why not — a clean run today means a broken -canary, not a fixed platform); workspace left clean with counts; committed. - -## D3 — review round, live re-proof, docs, PR - -**Outcome:** hostile reviewer pass over D1+D2 (priorities: the retry can -never touch an unmarked method; the canary cannot be masked by the -compensation; the numbers in every report are real); findings closed; a -full live round (deploy storefront-auth, canary verify, destroy); gotchas' -PRO-217 entry gains the RPC face and removal guard (orchestrator writes -it); PR opened against main with the slice narrative, review requested from -Will. No auto-merge armed; merge only on his word. +`test:scripts` green; at least one live run reports `bug-present` with raw +per-sample output (a clean run today means a broken canary — stop and +report, do not ship); workspace left clean with project counts; committed. + +## D3 — hostile review, live re-proof, docs, PR + +**Outcome:** reviewer pass over D1+D2 (attack priorities: a repeated key +can never double-execute within an instance; the replay cannot leak one +caller's response to a different logical call; keyless rejection cannot be +bypassed; the canary cannot be masked by the retry machinery; every +reported number is real). Findings closed. Full live round (deploy +storefront-auth, canary verify, destroy, zero leaks). Orchestrator writes +the gotchas PRO-217 RPC-face entry including the documented residual +window, and the design-notes record. PR opened against main with the slice +narrative; review requested from Will. No auto-merge; merge on his word. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md index b23ab19f..1e8e97b6 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md @@ -1,104 +1,123 @@ -# Slice: RPC cold-start handling — contract-declared idempotence, with its own canary +# Slice: RPC idempotency keys — safe retries for every call, with the PRO-217 canary ## At a glance `rpc()`'s `makeClient` carries no cold-start handling: every service-to-service RPC edge hits PRO-217's intermittent socket close raw on a cold target — and -PRO-217 is **live**, reproduced repeatedly during the streams slice -(2026-07-17: multiple `502 … socket connection was closed` on log-confirmed -cold starts). Unlike streams, every RPC call is a POST with no -transport-level idempotency signal, so a blanket retry in `makeClient` would -be exactly the double-execution hazard the streams append work fenced off. - -The safe shape, settled with Will (2026-07-17): **a method opts in at its -definition** — `rpc({ input, output, idempotent: true })` — and `makeClient` -applies a bounded cold-start backoff to marked methods only. Everything else -keeps today's fail-fast behavior with the failure surfaced. Own canary, -cloned from the (now trustworthy) cold-start canary's contract. +PRO-217 is live (reproduced repeatedly on 2026-07-17, log-confirmed cold +starts). A blanket retry was off the table while a retried POST could +double-execute a write. + +Settled design (Will, 2026-07-17, superseding an earlier `idempotent: true` +boolean proposal): **the RPC protocol requires an idempotency key on every +call**, and the framework implements idempotency control on both ends. With +the mechanism in place, every method is safely retryable and no per-method +declaration exists — a boolean would be a human claim the framework cannot +check; the key is a mechanism it enforces. + +Scope calibration (Will, verbatim intent): this protocol is primarily for +service-to-service calls inside one network, guarding against the cold-start +bug — not arbitrary distributed-systems failure. In-memory idempotency +control is sufficient; exposing the key to handlers is good practice, and it +is acceptable for users to ignore it. ## The design -### The flag - -- `rpc({ input, output, idempotent: true })` — optional, default absent - (= not idempotent, never retried). The flag rides on the method's runtime - value in `__cmp[method]` beside the two schemas, exactly where `makeClient` - already reads them. -- Declaring it is the METHOD AUTHOR's claim ("calling this twice is the same - as calling it once"), stated where the method is defined — the one place - that claim can be reviewed. Nothing infers it; nothing upgrades it. - -### The retry (client-side only; serve() unchanged) - -- `makeClient` wraps marked methods in a bounded backoff mirroring the - streams client's policy and numbers (250 ms initial, ×2, 5 s cap, 5 - attempts, jittered): retry thrown network errors, 5xx, and 429; never any - other 4xx (a real protocol answer surfaces on the first try). Unmarked - methods get today's single attempt, byte-for-byte. -- **Layering note, resolved:** rpc is framework-layer (target-agnostic), so - the policy is stated as generic idempotent-retry semantics — correct on - any transport for any method whose author declares idempotence — with the - comment naming PRO-217/PRO-219 as the motivating platform behavior and the - new canary as the removal trigger for the *urgency*, not the mechanism. - No prisma-cloud import enters the rpc package. +### Protocol + +- Every request `makeClient` sends carries an `Idempotency-Key` header: + a UUID minted **once per logical call** and reused byte-identically across + every retry of that call. Two separate calls never share a key. +- The key is REQUIRED: `serve()` rejects a keyless request with a loud 400 + naming the header — "requires" enforced, not suggested. (Manual `curl` + against an rpc endpoint must supply the header; document in the error.) + +### Client (`makeClient`) + +- Every method gets a bounded retry: the streams client's numbers (250 ms + initial, ×2, 5 s cap, 5 attempts, jittered). Retry thrown network errors, + 5xx, and 429; never any other 4xx — a real protocol answer surfaces on the + first try. Same key on every attempt. +- No per-method configuration. No `idempotent` flag anywhere. + +### Server (`serve()`) + +- **Single-flight per key:** a duplicate arriving while the first attempt is + still executing waits for it and receives the same response — the handler + runs once. +- **Replay window scoped to the retry envelope, not general replay:** + completed answers (2xx and 4xx — they are answers) are cached ~60 s with + an LRU bound and replayed byte-identically for a repeated key. 5xx and + thrown errors are not cached — they are the retryable outcomes, and a + retry re-executes. +- **Handler context:** handlers gain an optional second argument — + `(input, ctx)` with `ctx.idempotencyKey` — non-breaking for existing + handlers. A handler with hard exactly-once needs can write the key into + its own transaction; the framework does not require it. + +### The residual, documented and accepted + +In-memory control protects within an instance's lifetime. If an instance +applies a call, dies before responding, and the retry lands on a fresh +instance, the handler runs again. Accepted per the scope calibration above +(narrow window, same-network traffic, and PRO-217's specific failure — a +close mid-connection-establishment — never reached a handler at all, so its +retry was never the dangerous case). The gotchas entry states this window +plainly; no design pretends it is closed. + +### Status of the retry: permanent, not a compensation + +Bounded retry over keyed calls is correct RPC semantics on any network — +it does NOT get deleted when the platform fixes cold starts. The canary's +bug-gone message therefore retires only the canary itself, the gotchas +paragraph, and PRO-219's urgency framing — never the retry or the keys. ### The canary (PRO-217, RPC face) -A sibling of `scripts/cold-start-canary.ts`, inheriting every hard-won rule -of its 2026-07-17 rebuild — these are requirements, not suggestions: - -- **Trigger:** fresh deployment of the target service via create → upload → - start → race the promote call (never wait for `running`); first-touch RPC - call through the consumer the instant promote succeeds. -- **Cadence:** ≥60 s between samples, including before sample #0 — - back-to-back promotions produce ~1 s boots the bug does not live in. -- **Coldness is proven, not inferred:** the touch counts only if the - deployment's own boot log shows it was sent before the server's listening - line, outside the 2 s cross-clock margin; no latency guessing. -- **Statistical bug-gone rule:** `bug-gone` requires 14 confirmed cold-start - holds (20% target close rate, ≤5% false-clean chance); any close is - decisive `bug-present`; anything else inconclusive → warning, exit 0. -- **Probe an UNMARKED method**, so the raw platform behavior stays - observable — the compensation must not be able to mask the canary. -- Requirable exits: bug present → 0 (green), conclusively gone → 1 (red, - message names what to delete: the retry policy's cold-start framing, this - canary, the gotchas paragraph), inconclusive → 0 + `::warning::`. -- Rides `examples/storefront-auth` (the minimal storefront → auth RPC edge) - through the existing deploy-verify-destroy action; own `-classify.ts` - module with unit tests, own job in `e2e-deploy.yml`, NOT in the required - set until Will adds it. - -### The example - -`examples/store`'s catalog contract gets the flag where it is true — -`listProducts`, `getProduct`, `getSpecial` are idempotent reads; -`rotateSpecial` and `placeOrder` stay unmarked — so the example documents -the judgment call the flag exists to force. +A sibling of `scripts/cold-start-canary.ts`, inheriting every rule of its +2026-07-17 rebuild — requirements, not suggestions: + +- Fresh target via create → upload → start → **race the promote call** + (never wait for `running`); ≥60 s between samples, including before + sample #0; coldness proven from the deployment's own boot log (2 s + cross-clock margin), never inferred from latency; `bug-gone` needs 14 + confirmed cold-start holds (20% target close rate, ≤5% false-clean); any + close is decisive; first close exits early; `MAX_RUN_MS` self-stop under + the job timeout; requirable exits (present → 0, gone → 1 with the cleanup + message, inconclusive → 0 + `::warning::`). +- **Probes the target's rpc endpoint DIRECTLY with a bare `fetch`** (single + attempt, manually-minted key): every framework edge now auto-retries, so + a probe through a consumer would be masked by the machinery this slice + ships. The raw platform behavior must stay observable. +- Rides `examples/storefront-auth`'s deployed auth service through the + existing deploy-verify-destroy action; own `-classify.ts` + unit tests; + own job in `e2e-deploy.yml`; NOT required until Will adds it. ## Verification bar -- **Wire-counted, mutation-verified tests** in the rpc package (the streams - append-test pattern): a marked method against a transport that 503s then - succeeds → resolves, with the retry counted at the stub transport; an - UNMARKED method against the same transport → rejects after exactly ONE - request, re-asserted after a settle window; a marked method against a 404 - → rejects after exactly one request (4xx never retried). Each verified red - by deleting the behavior it pins. -- Type-level: the flag is optional and absent-by-default; `test-d` pins that - marking a method does not change its call signature. -- Canary classify tests extended from the cold-start suite's shapes, - including the never-went-cold and sample-budget rules; teeth confirmed. -- Live round: the canary run against a fresh deploy, raw output only — - expected verdict today is `bug-present` (the bug is live). A `bug-gone` - from a run today means the canary is broken; investigate, do not report - it as a result. +- **Wire-counted, mutation-verified client tests** (the streams append-test + pattern, counts asserted at a stub transport): 503-then-success → + resolves with exactly two requests carrying the SAME key; two separate + logical calls → different keys; 404 → rejects after exactly one request; + network-error-then-success → resolves. Teeth: delete the retry → red; + mint a fresh key per attempt → the same-key assertion reds. +- **serve() tests:** repeated key after completion → handler ran once, + response replayed byte-identically; concurrent same-key → one execution + (single-flight); keyless → 400 naming the header; 5xx not cached (a + retry re-executes); cache eviction bounds. Teeth confirmed per test. +- **Type-level:** existing one-argument handlers still typecheck; `ctx` + carries the key. +- **Live round:** canary against a fresh deploy, raw output only; expected + verdict today is `bug-present`. A clean run today means a broken canary — + investigate, do not report it as a result. - Repo checks green: typecheck, lint, casts delta 0, depcruise, - `test:scripts`. + `test:scripts`; `examples/store` and `examples/storefront-auth` compile + and their integration tests pass unchanged (handlers ignore `ctx`). ## Out of scope -- Retrying non-idempotent calls under any policy. Not negotiable — the - no-double-execution reasoning is recorded in the streams design docs. -- Server-side (serve()) changes; the retry is a client concern. +- Durable (cross-instance) idempotency storage — the handler's option via + `ctx.idempotencyKey`, never a framework requirement. +- Server-initiated replay semantics beyond the retry envelope. - Adding the canary to the required-checks list (Will's manual step). -- Typed `streamDef({ event })` and the streams follow-ups — separate slice. +- The streams follow-ups (typed `streamDef`, audit debt items) — separate. From d6ad6559bfbeb1576b27bff8bb406c2e25a9b97e Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 15:47:11 +0200 Subject: [PATCH 03/18] chore(drive): rebase the rpc slice onto the service-rpc rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Paths follow #131 (@internal/service-rpc, src/exports entrypoint). The accepted residual is rejustified against connection-contracts scope — internal means inside the topology, cross-network edges are in scope, and robustness is calibrated against named failure modes — so the slice names PRO-217 as what it guards and names the cross-instance window it leaves open. The three fixes promised when #114 was declined are absorbed here because they touch the same files. ADR-0037 reserved for the protocol. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../forcing-function-apps/design-notes.md | 25 ++ .../slices/rpc-cold-start/plan.md | 88 +++--- .../slices/rpc-cold-start/spec.md | 254 ++++++++++++------ .../2-authoring/rpc/src/protocol.ts | 12 + 4 files changed, 255 insertions(+), 124 deletions(-) create mode 100644 packages/0-framework/2-authoring/rpc/src/protocol.ts diff --git a/.drive/projects/forcing-function-apps/design-notes.md b/.drive/projects/forcing-function-apps/design-notes.md index 62ce1917..49b3bd69 100644 --- a/.drive/projects/forcing-function-apps/design-notes.md +++ b/.drive/projects/forcing-function-apps/design-notes.md @@ -208,3 +208,28 @@ cross-instance residual window is documented and accepted, not closed. The bounded retry becomes permanent protocol semantics — the PRO-217 canary retires itself when the platform heals, never the retry or the keys. Slice: [slices/rpc-cold-start/](slices/rpc-cold-start/spec.md). + +### Amendment (2026-07-17, later): rename, scope citation, absorbed fixes + +The kind was renamed to service-rpc (#131, ADR-0036) while this slice was in +flight; the branch is rebased and all paths follow the new package and the +`src/exports/` entrypoint convention (ADR-0035). Two corrections from Will: + +- The accepted residual is no longer justified by "same network" — that + framing contradicts `connection-contracts.md` § Purpose and scope, which + defines internal as membership of the application's topology and puts + cross-target, cross-network edges explicitly in scope, with robustness + calibrated per edge against the named failure modes of the targets + carrying it. The slice now names its concrete failure (PRO-217's close + during connection establishment, before any handler runs) and names the + residual it does not close (a retry reaching a different instance than the + one that applied the call), whose escalation path is the handler's own + durable control via `ctx.idempotencyKey`. +- Three fixes promised when #114 was declined touch the same files this + slice rewrites, so they are absorbed rather than left to collide: a + request body size limit, masking handler exception messages instead of + returning them to callers, and dropping the client's redundant second + validation of responses. + +ADR-0037 is the number for the keyed protocol (0033–0036 taken); it ships +with the implementation in this slice's PR, never as a docs-only change. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md index c9b3d671..ebdc36f6 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md @@ -1,30 +1,36 @@ # Dispatch plan: rpc-cold-start (idempotency keys) Contract source: [spec.md](spec.md). Branch: -`claude/streams-cold-start-rpc-37e5c1` off merged main (`2bafbbf`). Three -dispatches, sequential; hostile reviewer round after D1+D2, then D3 closes. -Orchestrator owns all docs (`gotchas.md`, `.drive/`, `docs/`) — implementers -report staleness, never edit. Evidence rules from the streams slice apply -verbatim: raw program output only; every reported number is checked against -the code's real format strings before it is believed. - -## D1 — the keyed protocol: client retry + serve() idempotency control - -**Outcome:** `makeClient` mints one `Idempotency-Key` per logical call, -reuses it across a bounded retry (250 ms / ×2 / 5 s cap / 5 attempts / -jitter; retry network errors + 5xx + 429, never other 4xx). `serve()` -enforces the key (keyless → loud 400), runs single-flight per in-flight -key, replays completed 2xx/4xx answers for ~60 s under an LRU bound, does -not cache 5xx/throws, and passes `ctx.idempotencyKey` as an optional second -handler argument. No `idempotent` flag exists. No prisma-cloud imports — -this is generic RPC semantics; PRO-217/PRO-219 are named as motivating -urgency only. - -**Completed when:** every wire-counted and serve() test in the spec is -green with teeth confirmed red-by-mutation (including the same-key-across- -attempts and fresh-key-per-call assertions); one-argument handlers still -typecheck (`test-d`); both rpc-consuming example suites pass unchanged; -repo checks green; committed with DCO dual sign-off. +`claude/streams-cold-start-rpc-37e5c1`, rebased onto main at/after `6ec2625` +(the [#131](https://github.com/prisma/compose/pull/131) service-rpc rename). +Three dispatches, sequential; hostile reviewer round after D1+D2, then D3 +closes. Orchestrator owns all docs (`gotchas.md`, `docs/`, `.drive/`, and +ADR-0037) — implementers report staleness and never edit. Evidence rules from +the streams slice apply verbatim: raw program output only; every reported +number is checked against the code's real format strings before it is +believed. + +## D1 — the keyed protocol, plus the three absorbed #114 fixes + +**Outcome:** in `packages/0-framework/2-authoring/service-rpc/`: +`makeClient` mints one `Idempotency-Key` per logical call and reuses it +across a bounded retry (250 ms / ×2 / 5 s cap / 5 attempts / jitter; retry +network errors + 5xx + 429, never other 4xx). `serve()` enforces the key +(keyless → loud 400), single-flights per in-flight key, replays completed +2xx/4xx for ~60 s under a justified LRU bound, never caches 5xx/throws, and +passes `ctx.idempotencyKey` as an optional third handler argument after +`deps`. Absorbed in the same pass: the request body size limit (413, +enforced while reading, not trusting `content-length`), masking handler and +output-validation exception messages behind a generic 500 while logging the +real error, and deleting the client's redundant response re-validation +(with the now-unnecessary `MethodSchemas` cast if it falls out). No +prisma-cloud import; no `idempotent` flag anywhere. + +**Completed when:** every client, server, and type-level test in the spec is +green with teeth confirmed red-by-mutation (including same-key-across- +attempts, fresh-key-per-call, 5xx-not-cached, and exception-message-not- +returned); both RPC-consuming example suites pass unchanged; repo checks +green with casts delta ≤ 0; committed with DCO dual sign-off. ## D2 — the canary (scripts + CI) @@ -32,25 +38,31 @@ repo checks green; committed with DCO dual sign-off. tests, inheriting the cold-start canary's proven contract wholesale (promote-race trigger, ≥60 s spacing including sample #0, log-confirmed coldness with the 2 s margin, 14-hold bug-gone budget, first-close early -exit, `MAX_RUN_MS`, requirable exits, bug-gone message that retires the -canary + gotchas entry but NEVER the retry or keys); job in -`e2e-deploy.yml` over `examples/storefront-auth`, probing the auth -service's rpc endpoint with a bare single-attempt `fetch` and a manually -minted key — never through a framework client, which would mask the bug. +exit, `MAX_RUN_MS`, requirable exits, and a bug-gone message that retires +the canary and its gotchas paragraph but NEVER the retry or keys); job in +`e2e-deploy.yml` over `examples/storefront-auth`, probing the auth service's +rpc endpoint with a bare single-attempt `fetch` and a manually minted key — +never through a framework client, which the new retry would mask. **Completed when:** classify tests green with confirmed teeth; `test:scripts` green; at least one live run reports `bug-present` with raw per-sample output (a clean run today means a broken canary — stop and report, do not ship); workspace left clean with project counts; committed. -## D3 — hostile review, live re-proof, docs, PR +## D3 — hostile review, live re-proof, records, PR + +**Outcome:** reviewer pass over D1+D2. Attack priorities: a repeated key can +never double-execute within an instance; a replay can never leak one logical +call's answer to another, across methods or callers; keyless rejection +cannot be bypassed; the body cap holds without a truthful `content-length`; +no handler exception text reaches a caller while operators keep it; the +canary cannot be masked by the retry machinery; every reported number is +real. Findings closed. Full live round (deploy storefront-auth, canary +verify, destroy, zero leaks). -**Outcome:** reviewer pass over D1+D2 (attack priorities: a repeated key -can never double-execute within an instance; the replay cannot leak one -caller's response to a different logical call; keyless rejection cannot be -bypassed; the canary cannot be masked by the retry machinery; every -reported number is real). Findings closed. Full live round (deploy -storefront-auth, canary verify, destroy, zero leaks). Orchestrator writes -the gotchas PRO-217 RPC-face entry including the documented residual -window, and the design-notes record. PR opened against main with the slice +Then the orchestrator writes, in this same PR: **ADR-0037** (calls carry an +idempotency key; the framework dedupes; retries are protocol semantics, with +the named residual and the connection-contracts scope calibration as its +reasoning), the `connection-contracts.md` protocol update, and the +`gotchas.md` PRO-217 service-RPC face. PR opened against main with the slice narrative; review requested from Will. No auto-merge; merge on his word. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md index 1e8e97b6..4f7b5c76 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md @@ -1,123 +1,205 @@ -# Slice: RPC idempotency keys — safe retries for every call, with the PRO-217 canary +# Slice: service-RPC idempotency keys — safe retries for every call, plus the PRO-217 canary ## At a glance -`rpc()`'s `makeClient` carries no cold-start handling: every service-to-service -RPC edge hits PRO-217's intermittent socket close raw on a cold target — and -PRO-217 is live (reproduced repeatedly on 2026-07-17, log-confirmed cold -starts). A blanket retry was off the table while a retried POST could -double-execute a write. - -Settled design (Will, 2026-07-17, superseding an earlier `idempotent: true` -boolean proposal): **the RPC protocol requires an idempotency key on every -call**, and the framework implements idempotency control on both ends. With -the mechanism in place, every method is safely retryable and no per-method -declaration exists — a boolean would be a human claim the framework cannot -check; the key is a mechanism it enforces. - -Scope calibration (Will, verbatim intent): this protocol is primarily for -service-to-service calls inside one network, guarding against the cold-start -bug — not arbitrary distributed-systems failure. In-memory idempotency -control is sufficient; exposing the key to handlers is good practice, and it -is acceptable for users to ignore it. +`makeClient` carries no cold-start handling, so every service-to-service RPC +edge hits PRO-217's intermittent socket close raw on a cold target — and +PRO-217 is live (reproduced repeatedly on 2026-07-17 against log-confirmed +cold starts). A blanket retry was previously unsafe because a retried POST +could double-execute a write. + +Settled design (Will, 2026-07-17, superseding an earlier per-method +`idempotent: true` boolean): **the protocol requires an idempotency key on +every call**, and the framework implements idempotency control on both ends. +With the mechanism in place every method is safely retryable and no +per-method declaration exists — a boolean is a human claim the framework +cannot check; a key is a mechanism it enforces. + +**Package note:** the kind was renamed by [#131](https://github.com/prisma/compose/pull/131) +(ADR-0036). Everything below lives in +`packages/0-framework/2-authoring/service-rpc/`, package +`@internal/service-rpc`, public import path `@prisma/composer/service-rpc`, +with the public entrypoint at `src/exports/index.ts` (ADR-0035). Branch must +be rebased onto main at or after `6ec2625` before any code is written. ## The design ### Protocol -- Every request `makeClient` sends carries an `Idempotency-Key` header: - a UUID minted **once per logical call** and reused byte-identically across - every retry of that call. Two separate calls never share a key. +- Every request `makeClient` sends carries an `Idempotency-Key` header: a + UUID minted **once per logical call** and reused **byte-identically across + every retry of that call**. Two separate logical calls never share a key. + `crypto.randomUUID()` — no new dependency. - The key is REQUIRED: `serve()` rejects a keyless request with a loud 400 - naming the header — "requires" enforced, not suggested. (Manual `curl` - against an rpc endpoint must supply the header; document in the error.) + naming the header. "Requires" is enforced, not suggested. ### Client (`makeClient`) -- Every method gets a bounded retry: the streams client's numbers (250 ms - initial, ×2, 5 s cap, 5 attempts, jittered). Retry thrown network errors, - 5xx, and 429; never any other 4xx — a real protocol answer surfaces on the - first try. Same key on every attempt. -- No per-method configuration. No `idempotent` flag anywhere. +- Every method gets a bounded retry: 250 ms initial, ×2, 5 s cap, 5 + attempts, jittered (the streams client's numbers; read its `client.ts` for + shape but **do not import from prisma-cloud** — service-rpc is + framework-layer and target-agnostic). Retry thrown network errors, 5xx and + 429; never any other 4xx. Same key on every attempt. +- No per-method configuration; no `idempotent` flag exists in this design. ### Server (`serve()`) -- **Single-flight per key:** a duplicate arriving while the first attempt is - still executing waits for it and receives the same response — the handler - runs once. -- **Replay window scoped to the retry envelope, not general replay:** - completed answers (2xx and 4xx — they are answers) are cached ~60 s with - an LRU bound and replayed byte-identically for a repeated key. 5xx and - thrown errors are not cached — they are the retryable outcomes, and a - retry re-executes. +- **Single-flight per in-flight key:** a duplicate arriving while the first + attempt is still executing waits for it and receives the same response — + the handler runs exactly once. +- **Replay within the retry envelope:** completed 2xx and 4xx responses (they + are answers) are cached ~60 s and replayed byte-identically for a repeated + key, under an LRU bound whose size is justified in a comment — this cache + is resident in every RPC provider. +- **5xx and thrown errors are not cached** — they are the retryable + outcomes; a retry must re-execute. +- A replayed answer must be structurally incapable of crossing methods. - **Handler context:** handlers gain an optional second argument — - `(input, ctx)` with `ctx.idempotencyKey` — non-breaking for existing - handlers. A handler with hard exactly-once needs can write the key into - its own transaction; the framework does not require it. - -### The residual, documented and accepted - -In-memory control protects within an instance's lifetime. If an instance -applies a call, dies before responding, and the retry lands on a fresh -instance, the handler runs again. Accepted per the scope calibration above -(narrow window, same-network traffic, and PRO-217's specific failure — a -close mid-connection-establishment — never reached a handler at all, so its -retry was never the dangerous case). The gotchas entry states this window -plainly; no design pretends it is closed. + `(input, deps, ctx)` carrying `ctx.idempotencyKey`, appended after the + existing `deps` argument so every existing handler still typechecks and + runs unchanged. Handlers may ignore it. + +### Robustness calibration, and the residual this leaves + +`docs/design/10-domains/connection-contracts.md` § *Purpose and scope* is the +governing scope statement: **internal means inside the application's +topology, not co-located on one network.** Components may come from +different targets, and such an edge necessarily crosses networks — that is in +scope. Protocol guarantees are calibrated per edge against **the named +failure modes of the targets carrying it**, and any proposal adding +robustness names the concrete failure it guards. + +The concrete failure this slice guards is **PRO-217**: the Prisma Compute +ingress closing a first-touch connection while a scale-to-zero target boots. +That close happens during connection establishment, **before any handler +runs** — nothing was applied, so retrying it cannot duplicate work +irrespective of dedupe. The keys additionally close the narrower window where +a response is lost after the handler applied it and the retry reaches the +**same** instance. + +**The residual, named not hidden:** in-memory control cannot cover a retry +that lands on a *different* instance than the one that applied the call +(instance death mid-request, or any edge where retries may be routed +elsewhere). This slice does not close it, and adds no durable store. The +escalation path is per-edge and belongs to the handler: `ctx.idempotencyKey` +lets a handler write the key inside its own transaction and obtain exactly- +once where its target's failure modes warrant it. Framework-side durable +dedupe is not proposed here — no concrete failure mode of a currently +supported target names it. ### Status of the retry: permanent, not a compensation -Bounded retry over keyed calls is correct RPC semantics on any network — -it does NOT get deleted when the platform fixes cold starts. The canary's -bug-gone message therefore retires only the canary itself, the gotchas -paragraph, and PRO-219's urgency framing — never the retry or the keys. - -### The canary (PRO-217, RPC face) +Bounded retry over keyed calls is correct protocol semantics for this kind — +it is **not** deleted when the platform heals. The canary's bug-gone message +retires the canary itself, its gotchas paragraph, and PRO-219's urgency +framing — never the retry or the keys. No comment on the retry or the key +machinery may say "remove when PRO-217 is fixed". + +## Absorbed fixes (promised on [#114](https://github.com/prisma/compose/pull/114), same files) + +These three were committed to when the oRPC PR was declined; they touch the +exact code this slice rewrites, so they land here rather than in a colliding +PR. + +1. **Request body size limit in `serve()`.** `await req.json()` is currently + unbounded. Cap it, answer over-limit with **413**, and do not trust + `content-length` alone — a lying or absent header must not bypass the cap, + so the bound is enforced while reading. Pick a limit appropriate to + internal RPC payloads and justify it in one comment. +2. **Stop leaking handler exception messages to callers.** The 500 path + currently returns `err.message` in the response body — an internal + exception string handed to the caller. Mask it: a generic failure message + to the caller, the real error logged server-side (`console.error`) so + operators keep it. Output-validation failures (a provider bug) are masked + the same way but logged distinguishably. **Input-validation 400s keep + their detail** — that is the caller's own malformed request and the + message is how they fix it. +3. **Remove the client's redundant second validation of responses.** + `serve()` already validates a handler's output against the method schema + before responding; `makeClient` re-validating it is duplicated work on + every call. Both ends of every edge are framework-provisioned (see the + scope statement above), so the second pass guards nothing. Expect this to + leave the client needing only method *names* off `__cmp` — if the + `MethodSchemas` blindCast in `client.ts` becomes unnecessary, delete it + (a cast-ratchet reduction, not a neutral change). + +## The canary (PRO-217, service-RPC face) A sibling of `scripts/cold-start-canary.ts`, inheriting every rule of its 2026-07-17 rebuild — requirements, not suggestions: - Fresh target via create → upload → start → **race the promote call** - (never wait for `running`); ≥60 s between samples, including before - sample #0; coldness proven from the deployment's own boot log (2 s - cross-clock margin), never inferred from latency; `bug-gone` needs 14 + (never wait for `running`); **≥60 s between samples, including before + sample #0**; coldness proven from the deployment's own boot log (2 s + cross-clock margin), never inferred from latency; `bug-gone` requires 14 confirmed cold-start holds (20% target close rate, ≤5% false-clean); any close is decisive; first close exits early; `MAX_RUN_MS` self-stop under the job timeout; requirable exits (present → 0, gone → 1 with the cleanup message, inconclusive → 0 + `::warning::`). -- **Probes the target's rpc endpoint DIRECTLY with a bare `fetch`** (single - attempt, manually-minted key): every framework edge now auto-retries, so - a probe through a consumer would be masked by the machinery this slice - ships. The raw platform behavior must stay observable. +- **Probes the target's rpc endpoint directly with a bare single-attempt + `fetch`** carrying a manually minted key — every framework edge now + auto-retries, so a probe through a consumer would be masked by the very + machinery this slice ships. The raw platform behavior must stay observable. - Rides `examples/storefront-auth`'s deployed auth service through the - existing deploy-verify-destroy action; own `-classify.ts` + unit tests; - own job in `e2e-deploy.yml`; NOT required until Will adds it. + existing deploy-verify-destroy action; own `-classify.ts` + unit tests; own + job in `e2e-deploy.yml`; NOT required until Will adds it. ## Verification bar -- **Wire-counted, mutation-verified client tests** (the streams append-test - pattern, counts asserted at a stub transport): 503-then-success → - resolves with exactly two requests carrying the SAME key; two separate - logical calls → different keys; 404 → rejects after exactly one request; - network-error-then-success → resolves. Teeth: delete the retry → red; - mint a fresh key per attempt → the same-key assertion reds. -- **serve() tests:** repeated key after completion → handler ran once, - response replayed byte-identically; concurrent same-key → one execution - (single-flight); keyless → 400 naming the header; 5xx not cached (a - retry re-executes); cache eviction bounds. Teeth confirmed per test. -- **Type-level:** existing one-argument handlers still typecheck; `ctx` - carries the key. -- **Live round:** canary against a fresh deploy, raw output only; expected - verdict today is `bug-present`. A clean run today means a broken canary — - investigate, do not report it as a result. -- Repo checks green: typecheck, lint, casts delta 0, depcruise, - `test:scripts`; `examples/store` and `examples/storefront-auth` compile - and their integration tests pass unchanged (handlers ignore `ctx`). +Client tests (counts asserted at a stub transport, the streams append-test +pattern — assert what the transport received, not what the client claims): + +- 503-then-success → resolves, **exactly two** requests, **same key on both**. +- Two separate logical calls → **different** keys. +- 404 → rejects after **exactly one** request; re-assert after a settle + window so a background retry would be caught. +- Thrown network error then success → resolves. + +Server tests: + +- Repeated key after completion → handler ran **once**, response replayed + byte-identically. +- Concurrent same-key → **one** execution (single-flight). +- Keyless → 400 naming the header. +- 5xx not cached → a same-key retry re-executes. +- LRU bound evicts. +- A replay can never answer a different method. +- Over-limit body → 413, including when `content-length` is absent or lies. +- A handler throw → 500 whose body does **not** contain the exception + message, with the real error logged. +- Input-validation failure → 400 that **does** carry its detail. + +Type-level (`test-d`): existing two-argument handlers `(input, deps)` still +typecheck; `ctx.idempotencyKey` is typed on the three-argument form. + +Each of the above has its teeth confirmed by mutation — break the behavior, +watch that specific test fail, restore. Explicitly including: delete the +retry; mint a fresh key per attempt instead of per call; cache 5xx; return +the exception message. + +Also green: `turbo run typecheck test`, `pnpm lint` (exit 0), `pnpm +lint:casts` (**delta ≤ 0**), `pnpm lint:deps`, `pnpm test:scripts`, and both +RPC-consuming example suites (`examples/store`, `examples/storefront-auth`) +passing **unchanged** — proof the handler-context argument is non-breaking. + +Live: canary against a fresh deploy, raw output only. Expected verdict today +is `bug-present`; a clean run today means a broken canary — investigate, do +not report it as a result. + +## Records to write (orchestrator, in this PR — never a docs-only PR) + +- **ADR-0037** (0033–0036 are taken): service-RPC calls carry an idempotency + key; the framework dedupes; retries are protocol semantics. +- `docs/design/10-domains/connection-contracts.md`: the protocol section + gains the key, the dedupe behavior, and the named residual. +- `gotchas.md`: PRO-217's entry gains the service-RPC face and the canary's + removal guard. ## Out of scope - Durable (cross-instance) idempotency storage — the handler's option via - `ctx.idempotencyKey`, never a framework requirement. -- Server-initiated replay semantics beyond the retry envelope. + `ctx.idempotencyKey`, never a framework requirement here. +- Middleware, metadata, streaming, rich errors — declined for this kind by + the scope statement. - Adding the canary to the required-checks list (Will's manual step). -- The streams follow-ups (typed `streamDef`, audit debt items) — separate. +- The streams follow-ups (typed `streamDef`, audit debt items). diff --git a/packages/0-framework/2-authoring/rpc/src/protocol.ts b/packages/0-framework/2-authoring/rpc/src/protocol.ts new file mode 100644 index 00000000..7d315b79 --- /dev/null +++ b/packages/0-framework/2-authoring/rpc/src/protocol.ts @@ -0,0 +1,12 @@ +/** + * Wire-protocol constants shared by the client (`client.ts`) and the server + * (`serve.ts`) — kept in one file so the two sides can't drift on the header + * name. + */ + +/** + * Every RPC request carries this header: a UUID minted once per logical + * call and reused byte-identically across every retry of that call. `serve()` + * rejects a request that omits it. + */ +export const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; From 8c164ecef1ffbbce122e8cb58ee8e5712898c3cd Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 15:47:35 +0200 Subject: [PATCH 04/18] chore: drop a stray file left at the pre-rename rpc path An interrupted dispatch wrote protocol.ts under the old rpc/ directory; it was untracked, so the rebase reset did not clear it and the previous commit swept it in. The slice writes into service-rpc/. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- packages/0-framework/2-authoring/rpc/src/protocol.ts | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 packages/0-framework/2-authoring/rpc/src/protocol.ts diff --git a/packages/0-framework/2-authoring/rpc/src/protocol.ts b/packages/0-framework/2-authoring/rpc/src/protocol.ts deleted file mode 100644 index 7d315b79..00000000 --- a/packages/0-framework/2-authoring/rpc/src/protocol.ts +++ /dev/null @@ -1,12 +0,0 @@ -/** - * Wire-protocol constants shared by the client (`client.ts`) and the server - * (`serve.ts`) — kept in one file so the two sides can't drift on the header - * name. - */ - -/** - * Every RPC request carries this header: a UUID minted once per logical - * call and reused byte-identically across every retry of that call. `serve()` - * rejects a request that omits it. - */ -export const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; From e25acc6faf490fb82209847357dcc25beda7afcd Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 16:18:52 +0200 Subject: [PATCH 05/18] feat(service-rpc): client mints an idempotency key and retries safely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit makeClient() now mints one crypto.randomUUID() per logical call and sends it as the Idempotency-Key header on every attempt, byte-identical across retries of that call and never shared with another call. Every method gets a bounded, jittered retry (250 ms initial, x2, 5 s cap, up to 5 retries): thrown network errors, 429, and any 5xx retry; any other 4xx does not, since a malformed or unauthorized request stays wrong on a second try. This exists because PRO-217 (the Prisma Compute ingress closing a first-touch connection while a scale-to-zero target boots) hits every service-to-service RPC edge raw today; a blanket retry was unsafe until every call carried a key a server could dedupe on. Also drops the client's redundant re-validation of the server's response against the output schema: serve() already validates a handler's output before responding, and both ends of every edge are framework-provisioned, so the second pass only duplicated work. The MethodSchemas blindCast this required falls out too — the client now only needs method names off contract.__cmp. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../service-rpc/src/__tests__/client.test.ts | 132 ++++++++++++---- .../2-authoring/service-rpc/src/client.ts | 143 +++++++++++++----- 2 files changed, 208 insertions(+), 67 deletions(-) diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts index 5de4fe61..e28d7f09 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts @@ -8,15 +8,16 @@ const authContract = contract({ verify: rpc({ input: type({ token: 'string' }), output: type({ ok: 'boolean' }) }), }); +const okResponse = () => + new Response(JSON.stringify({ ok: true }), { headers: { 'content-type': 'application/json' } }); + describe('makeClient()', () => { - test('POSTs JSON to /rpc/ and returns the validated output', async () => { + test('POSTs JSON to /rpc/ and returns the parsed response', async () => { const requests: Request[] = []; const client = makeClient(authContract, 'http://auth.internal', { fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -34,9 +35,7 @@ describe('makeClient()', () => { const client = makeClient(authContract, 'http://auth.internal/api/v1', { fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -45,20 +44,10 @@ describe('makeClient()', () => { expect(requests[0]?.url).toBe('http://auth.internal/api/v1/rpc/verify'); }); - test('rejects a response that fails the output schema — a lying server is caught', async () => { + test('throws naming the method when the transport responds with a non-retryable non-OK status', async () => { + // 400 (not 500 — a 5xx would now retry, see the idempotency-key tests below). const client = makeClient(authContract, 'http://auth.internal', { - fetch: async () => - new Response(JSON.stringify({ ok: 'not-a-boolean' }), { - headers: { 'content-type': 'application/json' }, - }), - }); - - await expect(client.verify({ token: 't' })).rejects.toThrow(); - }); - - test('throws naming the method when the transport responds non-OK', async () => { - const client = makeClient(authContract, 'http://auth.internal', { - fetch: async () => new Response('nope', { status: 500 }), + fetch: async () => new Response('nope', { status: 400 }), }); await expect(client.verify({ token: 't' })).rejects.toThrow(/verify/); @@ -90,9 +79,7 @@ describe('makeClient()', () => { serviceKey: 'edge-key', fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -106,9 +93,7 @@ describe('makeClient()', () => { const client = makeClient(authContract, 'http://auth.internal', { fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -117,3 +102,98 @@ describe('makeClient()', () => { expect(requests[0]?.headers.has('authorization')).toBe(false); }); }); + +describe('makeClient() — idempotency key and retry', () => { + test('every request carries a non-empty Idempotency-Key header', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return okResponse(); + }, + }); + + await client.verify({ token: 't' }); + + expect(requests[0]?.headers.get('idempotency-key')).toBeTruthy(); + }); + + test('two separate logical calls mint two different keys', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return okResponse(); + }, + }); + + await client.verify({ token: 'a' }); + await client.verify({ token: 'b' }); + + expect(requests).toHaveLength(2); + expect(requests[0]?.headers.get('idempotency-key')).not.toBe( + requests[1]?.headers.get('idempotency-key'), + ); + }); + + test('a 503 then success resolves after exactly two requests, both carrying the same key', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return requests.length === 1 ? new Response('cold', { status: 503 }) : okResponse(); + }, + }); + + await expect(client.verify({ token: 't' })).resolves.toEqual({ ok: true }); + + expect(requests).toHaveLength(2); + const firstKey = requests[0]?.headers.get('idempotency-key'); + const secondKey = requests[1]?.headers.get('idempotency-key'); + expect(firstKey).toBeTruthy(); + expect(firstKey).toBe(secondKey); + }); + + test('a 429 is retried the same as a 5xx', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return requests.length === 1 ? new Response('slow down', { status: 429 }) : okResponse(); + }, + }); + + await expect(client.verify({ token: 't' })).resolves.toEqual({ ok: true }); + expect(requests).toHaveLength(2); + }); + + test('a thrown network error is retried and the call still resolves', async () => { + let calls = 0; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async () => { + calls += 1; + if (calls === 1) throw new Error('socket reset'); + return okResponse(); + }, + }); + + await expect(client.verify({ token: 't' })).resolves.toEqual({ ok: true }); + expect(calls).toBe(2); + }); + + test('a 404 rejects after exactly one request — no background retry follows', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return new Response(JSON.stringify({ error: 'unknown method' }), { status: 404 }); + }, + }); + + await expect(client.verify({ token: 't' })).rejects.toThrow(); + expect(requests).toHaveLength(1); + + await new Promise((resolve) => setTimeout(resolve, 300)); // a retry would land here + expect(requests).toHaveLength(1); + }); +}); diff --git a/packages/0-framework/2-authoring/service-rpc/src/client.ts b/packages/0-framework/2-authoring/service-rpc/src/client.ts index 10a2e044..f344698d 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/client.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/client.ts @@ -1,25 +1,22 @@ /** * The RPC kind's network adapter — the client `rpc(contract)` hydrates to. - * Reads each method's Standard Schema pair off the contract's `__cmp[method]` - * runtime value (rpc()'s `{ input, output }`), POSTs JSON to - * `/rpc/`, and validates the response against the output schema - * before returning it (a provider can be typed-compatible and still lie at - * runtime — this is the per-call layer that catches that). When a - * `serviceKey` is supplied (ADR-0030), every request also carries - * `Authorization: Bearer `. + * POSTs JSON to `/rpc/` for each contract method read off + * `contract.__cmp`. Every request carries an `Idempotency-Key` header: one + * `crypto.randomUUID()` minted per logical call and reused byte-identically + * across every retry of that call — never shared between two separate + * calls. A thrown network error, a 429, or any 5xx is retried with a + * bounded, jittered backoff; any other 4xx is not, since a malformed or + * unauthorized request stays wrong on a second try. `serve()` already + * validates a handler's output against the method schema before responding, + * so the client trusts that and does not re-validate — both ends of every + * edge are framework-provisioned. When a `serviceKey` is supplied + * (ADR-0030), every request also carries `Authorization: Bearer + * `. */ import type { Contract } from '@internal/core'; import { blindCast } from '@internal/foundation/casts'; -import type { StandardSchemaV1 } from '@standard-schema/spec'; import type { Client, RpcFns } from './rpc.ts'; -import { standardValidate } from './standard-schema.ts'; - -/** rpc()'s runtime shape for one method: `{ input, output }` wearing a function's type. */ -interface MethodSchemas { - readonly input: StandardSchemaV1; - readonly output: StandardSchemaV1; -} /** * A fetch-shaped transport. Defaults to the real `fetch`; a served handler @@ -28,10 +25,33 @@ interface MethodSchemas { */ export type Transport = (req: Request) => Promise; -/** `/rpc/`, preserving a base URL's own path (e.g. a mount point). */ -function methodUrl(base: string, method: string): string { - const normalizedBase = base.endsWith('/') ? base : `${base}/`; - return new URL(`rpc/${method}`, normalizedBase).toString(); +/** + * The Prisma Compute ingress can close a service's first-touch connection + * while a scale-to-zero target boots (PRO-217), so every call is retried + * with a bounded, jittered exponential backoff — permanent protocol + * semantics for this kind, not a compensation that goes away once that + * platform behavior is fixed. These numbers mirror the streams module's + * IDEMPOTENT_BACKOFF; they are reimplemented here rather than imported + * because service-rpc is framework-layer and must not depend on + * prisma-cloud. `maxRetries` counts retries after the first attempt, so a + * persistently failing call sends 6 requests in total before giving up. + */ +const RETRY = { + initialDelayMs: 250, + multiplier: 2, + maxDelayMs: 5_000, + maxRetries: 5, +}; + +const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** Whether a non-OK response is safe to retry: 429 or any 5xx, never another 4xx. */ +function isRetryableStatus(status: number): boolean { + return status === 429 || status >= 500; } /** The server's `{ error }` body, if the response has one — undefined otherwise. */ @@ -46,40 +66,81 @@ async function errorDetail(res: Response): Promise { } } +/** `/rpc/`, preserving a base URL's own path (e.g. a mount point). */ +function methodUrl(base: string, method: string): string { + const normalizedBase = base.endsWith('/') ? base : `${base}/`; + return new URL(`rpc/${method}`, normalizedBase).toString(); +} + +/** + * Sends one logical call over `send`, retrying a thrown error, a 429, or a + * 5xx with full-jitter backoff (a random wait between 0 and the current + * delay, which then grows by `RETRY.multiplier` up to `RETRY.maxDelayMs`). + * `buildRequest` is called fresh for each attempt but always carries the + * same idempotency key — only the transport call is repeated, not the key. + */ +async function callWithRetry( + send: Transport, + buildRequest: () => Request, + method: string, +): Promise { + let delay = RETRY.initialDelayMs; + let retries = 0; + + for (;;) { + let res: Response; + try { + res = await send(buildRequest()); + } catch (err) { + if (retries >= RETRY.maxRetries) throw err; + retries += 1; + await sleep(Math.random() * delay); + delay = Math.min(delay * RETRY.multiplier, RETRY.maxDelayMs); + continue; + } + + if (res.ok) return res.json(); + + if (!isRetryableStatus(res.status) || retries >= RETRY.maxRetries) { + const detail = await errorDetail(res); + throw new Error( + `RPC call "${method}" failed: ${res.status} ${res.statusText}` + + (detail !== undefined ? ` — ${detail}` : ''), + ); + } + + retries += 1; + await sleep(Math.random() * delay); + delay = Math.min(delay * RETRY.multiplier, RETRY.maxDelayMs); + } +} + export function makeClient>( contract: C, url: string, opts?: { fetch?: Transport; serviceKey?: string }, ): Client { const send = opts?.fetch ?? fetch; - const headers: Record = { 'content-type': 'application/json' }; + const baseHeaders: Record = { 'content-type': 'application/json' }; if (opts?.serviceKey !== undefined) { - headers['Authorization'] = `Bearer ${opts.serviceKey}`; + baseHeaders['Authorization'] = `Bearer ${opts.serviceKey}`; } const client: Record Promise> = {}; - for (const [method, schemas] of Object.entries( - blindCast< - Record, - 'rpc() stores each method input/output Standard Schemas on the function value; RpcFns types only the call signature' - >(contract.__cmp), - )) { + for (const method of Object.keys(contract.__cmp)) { client[method] = async (input: unknown) => { - const res = await send( - new Request(methodUrl(url, method), { - method: 'POST', - headers, - body: JSON.stringify(input), - }), + const idempotencyKey = crypto.randomUUID(); + const body = JSON.stringify(input); + return callWithRetry( + send, + () => + new Request(methodUrl(url, method), { + method: 'POST', + headers: { ...baseHeaders, [IDEMPOTENCY_KEY_HEADER]: idempotencyKey }, + body, + }), + method, ); - if (!res.ok) { - const detail = await errorDetail(res); - throw new Error( - `RPC call "${method}" failed: ${res.status} ${res.statusText}` + - (detail !== undefined ? ` — ${detail}` : ''), - ); - } - return standardValidate(schemas.output, await res.json()); }; } From 12783721e6b63ef146c2385badbdf33e198619ee Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 16:19:09 +0200 Subject: [PATCH 06/18] feat(service-rpc)!: server requires the idempotency key and dedupes on it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit serve() now rejects a keyless request with a loud 400 naming the Idempotency-Key header. A duplicate key arriving while the first call for it is still executing single-flights onto that same execution, so the handler runs exactly once; a duplicate arriving after a completed 2xx/4xx answer replays it byte-identically for 60 seconds. A 5xx is never cached, since it's exactly the outcome a retry exists to re-execute. Storage is keyed by method first, then by idempotency key, so a lookup for one method can never find another method's cached answer, even if a key is reused across methods. The cache is LRU-bounded at 1000 entries so it stays a small, fixed amount of memory per provider process regardless of traffic. Handlers gain an optional third argument, ctx: RpcHandlerContext, carrying ctx.idempotencyKey after the existing deps argument — every existing two-argument handler still typechecks and runs unchanged. This is a breaking protocol change: a caller that doesn't go through makeClient (the client commit landing just before this one) must now supply its own key or its calls 400. This is permanent protocol semantics for this kind, not a compensation tied to any one platform incident — see PRO-217 for the motivating urgency. In-memory dedupe cannot cover a retry landing on a different instance than the one that applied the call; that residual is accepted, and the escalation path is a handler using ctx.idempotencyKey in its own transaction where its target's failure modes warrant it. Also lands the two remaining #114 fixes that touch this same dispatch path: - Request bodies are capped at 1 MiB, enforced against bytes actually read off the stream rather than the caller-supplied content-length (absent or lying, the cap still holds), and answered with 413. - A handler throw or an output-validation failure now masks its message behind a generic 500 and logs the real error server-side, so an internal exception string never reaches the caller. Input-validation 400s are unchanged — that detail is the caller's own malformed request and is how they fix it. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../src/__tests__/serve-handlers.test-d.ts | 11 + .../service-rpc/src/__tests__/serve.test.ts | 402 +++++++++++++++--- .../service-rpc/src/exports/index.ts | 7 +- .../2-authoring/service-rpc/src/serve.ts | 268 ++++++++++-- 4 files changed, 596 insertions(+), 92 deletions(-) diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts index 52c6a359..9e4d27c1 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts @@ -60,6 +60,17 @@ test('the exhaustive, correctly-typed handler map is accepted', () => { }); }); +test('ctx.idempotencyKey is typed on the three-argument handler form', () => { + serve(authService, { + rpc: { + verify: async ({ token }, { db }, ctx) => { + const key: string = ctx.idempotencyKey; + return { ok: token.length > 0 && db.validTokens.length >= 0 && key.length >= 0 }; + }, + }, + }); +}); + test('extra handler methods/ports beyond what is exposed are allowed (width)', () => { serve(authService, { rpc: { diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts index 3393a7a7..a15ddb82 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts @@ -5,7 +5,12 @@ import { type } from 'arktype'; import { makeClient } from '../client.ts'; import { contract } from '../contract.ts'; import { rpc } from '../rpc.ts'; -import { RPC_ACCEPTED_KEYS_ENV, serve } from '../serve.ts'; +import { + MAX_BODY_BYTES, + REPLAY_CACHE_MAX_ENTRIES, + RPC_ACCEPTED_KEYS_ENV, + serve, +} from '../serve.ts'; const authContract = contract({ verify: rpc({ input: type({ token: 'string' }), output: type({ ok: 'boolean' }) }), @@ -52,6 +57,21 @@ function fakeAuthService(load: () => FakeDb) { >; } +/** A POST /rpc/verify request, defaulting to a valid idempotency key — every test below overrides only what it's testing. */ +function verifyRequest( + body: unknown, + opts?: { readonly idempotencyKey?: string | null; readonly headers?: Record }, +): Request { + const headers: Record = { 'content-type': 'application/json', ...opts?.headers }; + const key = opts?.idempotencyKey === undefined ? 'test-key' : opts.idempotencyKey; + if (key !== null) headers['idempotency-key'] = key; + return new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers, + body: JSON.stringify(body), + }); +} + describe('serve()', () => { test('round trip: a valid call reaches the handler and returns the typed result', async () => { const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); @@ -64,19 +84,16 @@ describe('serve()', () => { await expect(client.verify({ token: 'bad-token' })).resolves.toEqual({ ok: false }); }); - test('a bad input is rejected — the server-side arktype validation fires', async () => { + test('a bad input is rejected with 400, and the body carries the validator detail', async () => { const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 123 }), - }), - ); + const res = await handler(verifyRequest({ token: 123 })); expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error.length).toBeGreaterThan(0); + expect(body.error).toMatch(/token/i); // arktype's message names the bad field }); test('an unknown method 404s', async () => { @@ -94,25 +111,35 @@ describe('serve()', () => { expect(res.status).toBe(404); }); - test('a handler throw is a 500, not a crash', async () => { + test('a handler throw is a 500 that does not leak the exception message, and logs the real error', async () => { const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => { - throw new Error('db unreachable'); + throw new Error('db credentials: hunter2'); }, }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 't' }), - }), - ); - - expect(res.status).toBe(500); + const logged: unknown[][] = []; + const originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + logged.push(args); + }; + try { + const res = await handler(verifyRequest({ token: 't' })); + + expect(res.status).toBe(500); + const bodyText = await res.text(); + expect(bodyText).not.toContain('hunter2'); + + const loggedRealError = logged.some((args) => + args.some((arg) => arg instanceof Error && arg.message.includes('hunter2')), + ); + expect(loggedRealError).toBe(true); + } finally { + console.error = originalConsoleError; + } }); test('the wrong HTTP verb on a known method is a 405', async () => { @@ -140,6 +167,285 @@ describe('serve()', () => { expect(loadCalls).toBe(1); }); + + test('a handler can read ctx.idempotencyKey — it matches the caller-supplied header', async () => { + let seenKey: string | undefined; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }, ctx) => { + seenKey = ctx.idempotencyKey; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: 'caller-key-42' })); + + expect(seenKey).toBe('caller-key-42'); + }); +}); + +describe('serve() — idempotency key requirement', () => { + test('a request without the Idempotency-Key header is rejected with 400 naming the header', async () => { + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const res = await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: null })); + + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toContain('Idempotency-Key'); + }); + + test('an empty Idempotency-Key header is treated as missing', async () => { + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const res = await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: '' })); + + expect(res.status).toBe(400); + }); +}); + +describe('serve() — idempotency dedupe', () => { + test('a repeated key after completion replays the same response byte-identically — the handler runs once', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const req = () => verifyRequest({ token: 'good-token' }, { idempotencyKey: 'replay-key' }); + const first = await handler(req()); + const firstBody = await first.text(); + const second = await handler(req()); + const secondBody = await second.text(); + + expect(handlerCalls).toBe(1); + expect(second.status).toBe(first.status); + expect(secondBody).toBe(firstBody); + }); + + test('concurrent requests sharing a key single-flight onto one execution', async () => { + let handlerCalls = 0; + let releaseHandler: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseHandler = resolve; + }); + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + await gate; // held open until the test releases it, below + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const req = () => verifyRequest({ token: 'good-token' }, { idempotencyKey: 'concurrent-key' }); + const first = handler(req()); + const second = handler(req()); + releaseHandler(); + const [firstRes, secondRes] = await Promise.all([first, second]); + + expect(handlerCalls).toBe(1); + expect(await firstRes.text()).toBe(await secondRes.text()); + }); + + test('a 5xx response is not cached — a same-key retry re-executes the handler', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { + rpc: { + verify: async () => { + handlerCalls += 1; + throw new Error('db unreachable'); + }, + }, + }); + + const req = () => verifyRequest({ token: 't' }, { idempotencyKey: 'failing-key' }); + const first = await handler(req()); + const second = await handler(req()); + + expect(first.status).toBe(500); + expect(second.status).toBe(500); + expect(handlerCalls).toBe(2); + }); + + test('the replay cache is LRU-bounded — the oldest entry is evicted once the bound is exceeded', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const call = (key: string) => + handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: key })); + + await call('key-0'); + for (let i = 1; i <= REPLAY_CACHE_MAX_ENTRIES; i++) { + await call(`key-${i}`); // pushes key-0 out once the bound is exceeded + } + expect(handlerCalls).toBe(REPLAY_CACHE_MAX_ENTRIES + 1); + + await call('key-0'); // evicted — re-executes instead of replaying + expect(handlerCalls).toBe(REPLAY_CACHE_MAX_ENTRIES + 2); + + await call(`key-${REPLAY_CACHE_MAX_ENTRIES}`); // still cached — replays + expect(handlerCalls).toBe(REPLAY_CACHE_MAX_ENTRIES + 2); + }, 20_000); + + test('a replayed answer cannot cross methods, even when the same idempotency key is reused', async () => { + const dualContract = contract({ + verify: rpc({ input: type({ token: 'string' }), output: type({ ok: 'boolean' }) }), + echo: rpc({ input: type({ note: 'string' }), output: type({ note: 'string' }) }), + }); + const db: DependencyEnd = dependency({ + name: 'db', + type: 'fake/db', + connection: { params: {}, hydrate: () => ({ validTokens: ['good-token'] }) }, + }); + const node = service({ + name: 'test-service', + extension: 'test/pack', + type: 'fake/rpc-test', + inputs: { db }, + params: {}, + build: { + extension: '@fake/adapter', + type: 'fake', + module: 'file:///test/service.ts', + entry: 'x', + }, + expose: { rpc: dualContract }, + }); + const dualService = { + ...node, + run: (_address: string, boot: () => Promise) => boot(), + load: () => ({ db: { validTokens: ['good-token'] } }), + } as unknown as RunnableServiceNode< + typeof node.inputs, + typeof node.params, + { rpc: typeof dualContract } + >; + + const handler = serve(dualService, { + rpc: { + verify: async ({ token }, { db }) => ({ ok: db.validTokens.includes(token) }), + echo: async ({ note }) => ({ note }), + }, + }); + + const sharedKey = 'shared-key'; + const verifyRes = await handler( + new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': sharedKey }, + body: JSON.stringify({ token: 'good-token' }), + }), + ); + expect(await verifyRes.json()).toEqual({ ok: true }); + + const echoRes = await handler( + new Request('http://auth.internal/rpc/echo', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': sharedKey }, + body: JSON.stringify({ note: 'hello' }), + }), + ); + // Must be echo's own answer, never verify's cached { ok: true }. + expect(await echoRes.json()).toEqual({ note: 'hello' }); + }); +}); + +describe('serve() — request body size cap', () => { + test('a body over the size limit is rejected with 413', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const oversized = 'x'.repeat(MAX_BODY_BYTES + 1); + const res = await handler(verifyRequest({ token: oversized })); + + expect(res.status).toBe(413); + }); + + test('the cap is enforced against bytes actually read, not content-length — a stream body with no content-length header is still capped', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const oversizedJson = JSON.stringify({ token: 'x'.repeat(MAX_BODY_BYTES + 1) }); + const bytes = new TextEncoder().encode(oversizedJson); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(bytes); + controller.close(); + }, + }); + + const req = new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': 'k' }, + body, + duplex: 'half', + } as RequestInit); + + expect(req.headers.has('content-length')).toBe(false); + + const res = await handler(req); + expect(res.status).toBe(413); + }); + + test('the cap is enforced even when content-length lies about being small', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const oversizedJson = JSON.stringify({ token: 'x'.repeat(MAX_BODY_BYTES + 1) }); + const bytes = new TextEncoder().encode(oversizedJson); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(bytes); + controller.close(); + }, + }); + + const req = new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'idempotency-key': 'k', + 'content-length': '10', // a lie — the real body is far larger + }, + body, + duplex: 'half', + } as RequestInit); + + const res = await handler(req); + expect(res.status).toBe(413); + }); + + test('a body at or under the limit is accepted', async () => { + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { verify: async ({ token }, { db }) => ({ ok: db.validTokens.includes(token) }) }, + }); + + const res = await handler(verifyRequest({ token: 'good-token' })); + + expect(res.status).toBe(200); + }); }); describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () => { @@ -175,11 +481,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }); const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json', authorization: 'Bearer wrong-key' }, - body: JSON.stringify({ token: 't' }), - }), + verifyRequest({ token: 't' }, { headers: { authorization: 'Bearer wrong-key' } }), ); expect(res.status).toBe(401); @@ -199,13 +501,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 't' }), - }), - ); + const res = await handler(verifyRequest({ token: 't' })); expect(res.status).toBe(401); expect(handlerCalled).toBe(false); @@ -216,13 +512,17 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 123 }), - }), - ); + const res = await handler(verifyRequest({ token: 123 })); + + expect(res.status).toBe(401); + }); + + test('401 fires before the idempotency-key requirement — a keyless request with no service key is 401, not 400', async () => { + process.env[RPC_ACCEPTED_KEYS_ENV] = JSON.stringify(['good-key']); + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const res = await handler(verifyRequest({ token: 't' }, { idempotencyKey: null })); expect(res.status).toBe(401); }); @@ -240,13 +540,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 'good-token' }), - }), - ); + const res = await handler(verifyRequest({ token: 'good-token' })); expect(res.status).toBe(401); expect(handlerCalled).toBe(false); @@ -258,13 +552,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () rpc: { verify: async ({ token }, { db }) => ({ ok: db.validTokens.includes(token) }) }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 'good-token' }), - }), - ); + const res = await handler(verifyRequest({ token: 'good-token' })); expect(res.status).toBe(200); }); @@ -282,13 +570,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 'good-token' }), - }), - ); + const res = await handler(verifyRequest({ token: 'good-token' })); expect(res.status).toBe(401); expect(handlerCalled).toBe(false); diff --git a/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts b/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts index 307117af..b8c10ed0 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts @@ -5,6 +5,11 @@ * client.ts; `serve()` generates a provider's fetch handler straight off a * service's `expose`. All web-standard (fetch/Request/Response) — runs * anywhere those exist, no node/bun coupling. + * + * Every call carries an idempotency key and retries safely: the client + * mints one per logical call and bounded-retries with it; the server + * requires it and dedupes on it. A handler's optional third argument, + * typed `RpcHandlerContext`, carries that key as `ctx.idempotencyKey`. */ export type { Transport } from '../client.ts'; @@ -12,5 +17,5 @@ export { makeClient } from '../client.ts'; export { contract } from '../contract.ts'; export type { Client } from '../rpc.ts'; export { perBindingToken, RPC_PEER_KEY, rpc } from '../rpc.ts'; -export type { Handlers } from '../serve.ts'; +export type { Handlers, RpcHandlerContext } from '../serve.ts'; export { RPC_ACCEPTED_KEYS_ENV, serve } from '../serve.ts'; diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index cbc1c739..6dc31383 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/serve.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/serve.ts @@ -13,6 +13,13 @@ * through; a provisioned `"[]"` (deployed, zero wired consumers) denies * every caller; a provisioned non-empty set requires membership via * `Authorization: Bearer `. + * + * Every request must also carry an `Idempotency-Key` header — a keyless + * request is rejected with a 400. The key drives per-method, + * per-key dedupe: a duplicate arriving while the first call is still + * executing single-flights onto that same execution, and a duplicate + * arriving after a 2xx/4xx answer replays it byte-identically for a bounded + * time. See `IdempotencyStore` below. */ import type { Contract, Expose, RunnableServiceNode } from '@internal/core'; @@ -32,8 +39,14 @@ type AnyRunnable = RunnableServiceNode; type CmpOf = C extends Contract ? Cmp : never; +/** What a handler's optional third argument carries. Handlers may ignore it. */ +export interface RpcHandlerContext { + /** The calling client's idempotency key for this logical call — the same value on every one of its retries. */ + readonly idempotencyKey: string; +} + type HandlerFor = Fn extends (input: infer I) => Promise - ? (input: I, deps: LoadedDeps) => Promise + ? (input: I, deps: LoadedDeps, ctx: RpcHandlerContext) => Promise : never; /** Every exposed port's methods, turned into a handler map typed off S's own `expose` and `load()`. */ @@ -51,15 +64,167 @@ interface MethodSchemas { readonly output: StandardSchemaV1; } -type RpcHandler = (input: unknown, deps: unknown) => Promise; +type RpcHandler = (input: unknown, deps: unknown, ctx: RpcHandlerContext) => Promise; + +/** A response, reduced to what the replay cache needs to reproduce it byte-identically. */ +interface Outcome { + readonly status: number; + readonly bodyText: string; +} -function jsonResponse(body: unknown, status = 200): Response { - return new Response(JSON.stringify(body), { - status, +function outcome(body: unknown, status = 200): Outcome { + return { status, bodyText: JSON.stringify(body) }; +} + +function toResponse(o: Outcome): Response { + return new Response(o.bodyText, { + status: o.status, headers: { 'content-type': 'application/json' }, }); } +/** The generic message every caller-facing 500 carries — the real error goes to `console.error` instead. */ +const INTERNAL_ERROR_MESSAGE = 'Internal server error'; + +/** + * Internal RPC payloads are small, schema-validated records, not file + * uploads — 1 MiB comfortably covers any real one while bounding the + * worst-case memory a single slow request can hold open on one instance. + */ +export const MAX_BODY_BYTES = 1_048_576; + +class RequestBodyTooLargeError extends Error {} + +/** + * Reads `req`'s body as text, aborting once more than `maxBytes` has + * actually been read off the stream — never trusting `content-length`, + * which is caller-supplied and may be absent or wrong. + */ +async function readBoundedBody(req: Request, maxBytes: number): Promise { + const reader = req.body?.getReader(); + if (reader === undefined) return ''; + + const decoder = new TextDecoder(); + let text = ''; + let total = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > maxBytes) { + await reader.cancel(); + throw new RequestBodyTooLargeError(); + } + text += decoder.decode(value, { stream: true }); + } + text += decoder.decode(); + return text; +} + +/** + * How many completed answers the replay cache holds across every method at + * once, LRU-evicted once full. This cache is resident in every RPC + * provider process, so its bound has to be a fixed, small amount of memory + * rather than grow with traffic: 1000 entries — each a small JSON body plus + * bookkeeping — comfortably fits in a few hundred KB, well past the + * concurrent in-flight key count one instance of an internal RPC provider + * realistically sees inside the replay window. + */ +export const REPLAY_CACHE_MAX_ENTRIES = 1000; + +/** How long a completed 2xx/4xx answer stays replayable for a repeated key. */ +const REPLAY_TTL_MS = 60_000; + +type CacheEntry = + | { readonly kind: 'pending'; readonly promise: Promise } + | { readonly kind: 'completed'; readonly outcome: Outcome; readonly completedAt: number }; + +/** + * Per-method, per-idempotency-key dedupe. A duplicate key arriving while + * the first call for it is still executing single-flights onto that same + * promise, so the handler runs exactly once. A completed 2xx/4xx answer (an + * answer, not a retryable outcome) replays byte-identically for + * REPLAY_TTL_MS; a 5xx is never kept, since 5xx is exactly the outcome a + * retry exists to re-execute. + * + * Storage is keyed by method first, then by idempotency key, so a lookup + * for method B can only ever find B's own entries — a replay is + * structurally incapable of answering a different method, even if a caller + * (buggy or malicious) reuses one key across two different methods. + */ +class IdempotencyStore { + private readonly byMethod = new Map>(); + // Global LRU order across every method's completed entries, oldest first; + // insertion order in a Map is exploited here rather than a separate + // linked list — re-inserting a key moves it to the end. + private readonly lruOrder = new Map(); + + async dispatch(method: string, key: string, run: () => Promise): Promise { + const bucket = this.bucketFor(method); + const existing = bucket.get(key); + + if (existing?.kind === 'pending') { + return existing.promise; + } + if (existing?.kind === 'completed') { + if (Date.now() - existing.completedAt < REPLAY_TTL_MS) { + this.touch(method, key); + return existing.outcome; + } + bucket.delete(key); + this.lruOrder.delete(this.lruKey(method, key)); + } + + const promise = run(); + bucket.set(key, { kind: 'pending', promise }); + + let result: Outcome; + try { + result = await promise; + } catch (err) { + bucket.delete(key); + throw err; + } + + if (result.status >= 500) { + bucket.delete(key); // retryable outcome — a retry must re-execute + } else { + bucket.set(key, { kind: 'completed', outcome: result, completedAt: Date.now() }); + this.touch(method, key); + } + return result; + } + + private bucketFor(method: string): Map { + let bucket = this.byMethod.get(method); + if (bucket === undefined) { + bucket = new Map(); + this.byMethod.set(method, bucket); + } + return bucket; + } + + private lruKey(method: string, key: string): string { + return `${method}${key}`; + } + + /** Marks (method, key) most-recently-used, evicting the oldest completed entry once over the bound. */ + private touch(method: string, key: string): void { + const lruKey = this.lruKey(method, key); + this.lruOrder.delete(lruKey); + this.lruOrder.set(lruKey, { method, key }); + + if (this.lruOrder.size > REPLAY_CACHE_MAX_ENTRIES) { + const oldestKey = this.lruOrder.keys().next().value; + const oldest = oldestKey === undefined ? undefined : this.lruOrder.get(oldestKey); + if (oldestKey !== undefined && oldest !== undefined) { + this.lruOrder.delete(oldestKey); + this.byMethod.get(oldest.method)?.delete(oldest.key); + } + } + } +} + /** The provisioned accepted key set, or undefined when the deploy never provisioned one (local/test — enforcement off). */ function acceptedKeys(): readonly string[] | undefined { const raw = process.env[RPC_ACCEPTED_KEYS_ENV]; @@ -108,6 +273,8 @@ function bearerToken(req: Request): string { return header?.startsWith(BEARER_PREFIX) ? header.slice(BEARER_PREFIX.length) : ''; } +const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; + /** * Flattens every exposed port's methods into one method → {schemas, handler} * table. RPC dispatch is flat (`/rpc/`), so a method name exposed by @@ -144,11 +311,14 @@ function methodTable( } /** - * Routes `POST /rpc/`: parses JSON, validates input, calls the - * handler with `service.load()`'s deps, validates the output, and responds - * JSON. An unknown method or invalid input is a 4xx; a handler (or output - * validation) failure is a 5xx — either way the process does not crash. - * `load()` is called exactly once, here, before the handler ever runs. + * Routes `POST /rpc/`: checks the service key, requires an + * Idempotency-Key, single-flights/replays through `IdempotencyStore`, and — + * per call — parses JSON within the body cap, validates input, calls the + * handler with `service.load()`'s deps plus `{ idempotencyKey }`, validates + * the output, and responds JSON. A handler or output-validation failure + * masks its message behind a generic 500 and logs the real error; an + * unknown method or invalid input is a 4xx. `load()` is called exactly + * once, here, before the handler ever runs. */ export function serve>( service: S, @@ -162,46 +332,82 @@ export function serve>( >(handlers), ); const deps = service.load(); + const idempotency = new IdempotencyStore(); return async (req: Request): Promise => { const accepted = acceptedKeys(); if (accepted !== undefined && !isAcceptedKey(bearerToken(req), accepted)) { - return jsonResponse({ error: 'Unauthorized: missing or invalid service key' }, 401); + return toResponse(outcome({ error: 'Unauthorized: missing or invalid service key' }, 401)); } const { pathname } = new URL(req.url); const methodName = /^\/rpc\/([^/]+)$/.exec(pathname)?.[1]; if (methodName === undefined) { - return jsonResponse({ error: `Not found: ${pathname}` }, 404); + return toResponse(outcome({ error: `Not found: ${pathname}` }, 404)); } const method = table.get(methodName); if (method === undefined) { - return jsonResponse({ error: `Unknown RPC method "${methodName}"` }, 404); + return toResponse(outcome({ error: `Unknown RPC method "${methodName}"` }, 404)); } if (req.method !== 'POST') { - return jsonResponse({ error: `Method "${methodName}" requires POST` }, 405); + return toResponse(outcome({ error: `Method "${methodName}" requires POST` }, 405)); } - let body: unknown; - try { - body = await req.json(); - } catch { - return jsonResponse({ error: 'Request body must be JSON' }, 400); + const idempotencyKey = req.headers.get(IDEMPOTENCY_KEY_HEADER.toLowerCase()); + if (idempotencyKey === null || idempotencyKey === '') { + return toResponse( + outcome({ error: `Missing required "${IDEMPOTENCY_KEY_HEADER}" header` }, 400), + ); } - let input: unknown; - try { - input = await standardValidate(method.input, body); - } catch (err) { - return jsonResponse({ error: err instanceof Error ? err.message : String(err) }, 400); - } + const ctx: RpcHandlerContext = { idempotencyKey }; - try { - const result = await method.handler(input, deps); - return jsonResponse(await standardValidate(method.output, result)); - } catch (err) { - return jsonResponse({ error: err instanceof Error ? err.message : String(err) }, 500); - } + const run = async (): Promise => { + let bodyText: string; + try { + bodyText = await readBoundedBody(req, MAX_BODY_BYTES); + } catch (err) { + if (err instanceof RequestBodyTooLargeError) { + return outcome({ error: `Request body exceeds the ${MAX_BODY_BYTES}-byte limit` }, 413); + } + throw err; + } + + let body: unknown; + try { + body = JSON.parse(bodyText); + } catch { + return outcome({ error: 'Request body must be JSON' }, 400); + } + + let input: unknown; + try { + input = await standardValidate(method.input, body); + } catch (err) { + return outcome({ error: err instanceof Error ? err.message : String(err) }, 400); + } + + try { + const result = await method.handler(input, deps, ctx); + let output: unknown; + try { + output = await standardValidate(method.output, result); + } catch (err) { + console.error( + `serve(): handler for "${methodName}" returned output that failed schema validation — this is a provider bug:`, + err, + ); + return outcome({ error: INTERNAL_ERROR_MESSAGE }, 500); + } + return outcome(output); + } catch (err) { + console.error(`serve(): handler for "${methodName}" threw:`, err); + return outcome({ error: INTERNAL_ERROR_MESSAGE }, 500); + } + }; + + const result = await idempotency.dispatch(methodName, idempotencyKey, run); + return toResponse(result); }; } From 8cd8b7330891847e69abca2cf4d3c843fa0da96a Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 16:19:17 +0200 Subject: [PATCH 07/18] fix(cron): give serve-schedule's test requests an idempotency key serve() now requires every request to carry an Idempotency-Key header; serveSchedule()'s production callers already get one for free since they go through makeClient (the trigger dep hydrates via rpc()), but this suite builds its trigger requests by hand and was failing 400 before reaching the behavior each test actually exercises. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../cron/src/__tests__/serve-schedule.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts b/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts index 403b35f7..80e7d5ff 100644 --- a/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts +++ b/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts @@ -52,7 +52,10 @@ const schedule = defineSchedule({ tick: '2s', mrr: '5s' }); function triggerRequest(jobId: string): Request { return new Request('http://cron.internal/rpc/trigger', { method: 'POST', - headers: { 'content-type': 'application/json' }, + // service-rpc's serve() requires every request to carry an idempotency + // key; production calls go through makeClient, which mints one, so only + // this hand-built test request needs to supply it. + headers: { 'content-type': 'application/json', 'idempotency-key': `test-${jobId}` }, body: JSON.stringify({ jobId }), }); } From a16e684046b96958c2bdd4997ef1333ad6a37081 Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 17:03:44 +0200 Subject: [PATCH 08/18] docs(adr-0037): service RPC calls carry an idempotency key Records the decision the slice implements: one key per logical call, reused across retries; the server requires it, runs one call per key, and replays completed answers but never 5xx. No per-method opt-in, because a flag would be a claim the framework cannot verify. States what the in-process store does not cover and why that residue is deliberate, calibrated against connection-contracts scope. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- ...vice-rpc-calls-carry-an-idempotency-key.md | 139 ++++++++++++++++++ docs/design/90-decisions/README.md | 1 + 2 files changed, 140 insertions(+) create mode 100644 docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md diff --git a/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md b/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md new file mode 100644 index 00000000..720737e8 --- /dev/null +++ b/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md @@ -0,0 +1,139 @@ +# ADR-0037: Service RPC calls carry an idempotency key, so every call is safely retryable + +## Status + +Accepted + +## Decision + +Every service RPC request carries an `Idempotency-Key` header. The client +mints one key per logical call and reuses it on every retry of that call; the +server requires it, runs one call per key, and replays a completed answer to a +repeat. Because duplicates are absorbed by the protocol rather than by a +promise from the method author, the client retries **every** method — there is +no per-method opt-in. + +``` +POST /rpc/placeOrder POST /rpc/placeOrder +Idempotency-Key: 9f2c…e1 Idempotency-Key: 9f2c…e1 ← same key, retry +Authorization: Bearer + server: this key already ran → + ✗ connection closed mid-boot replay its answer, do not + run the handler again +``` + +A handler may read the key when it wants a stronger guarantee than the +framework's own: + +```ts +serve(service, { + orders: { + // `ctx` is optional — existing two-argument handlers are unaffected. + placeOrder: async (input, deps, ctx) => { + await deps.db.insert({ ...input, requestKey: ctx.idempotencyKey }); + return { placed: true }; + }, + }, +}); +``` + +## Reasoning + +A service that has scaled to zero has to boot before it can answer, and a +caller's first request can be dropped while that happens — the connection is +closed during establishment, before any handler runs. The obvious repair is for +the client to retry, and the obvious objection is that a retried `POST` may +execute a write twice. + +The first design that suggests itself is to let each method say whether it is +safe to repeat — `rpc({ input, output, idempotent: true })` — and retry only +the methods that say yes. It should be rejected, and the reason generalizes: +idempotence is a property of what a handler does to state, so nothing at the +RPC layer can check the claim. The framework would be retrying on an assurance +it cannot verify, and the failure mode is silent — a method marked wrongly +duplicates writes with nothing to catch it. An unverifiable flag is also an +invitation to omission: the safe default is "not idempotent", so the common +outcome is that nobody marks anything and the mechanism protects nothing. + +An idempotency key inverts that. The client mints one identifier per logical +call — not per attempt — so every retry of that call is recognizable as the +same call, and two genuinely separate calls never collide. The server keys its +work on it: a duplicate that arrives while the first attempt is still running +waits for that attempt instead of starting a second one, and a duplicate that +arrives after an answer was produced receives that same answer without the +handler running again. Duplicate suppression becomes a mechanism the framework +enforces, so the method author is asked for nothing, and every method is +retryable. + +What counts as "an answer" matters. A `2xx` and a `4xx` are both conclusions — +the call succeeded, or it was rejected — and replaying either is correct. A +`5xx` or a thrown error is not a conclusion; it is the outcome a retry exists +to escape, so those are never remembered and a retry re-executes. The memory of +answers is bounded in time and in size: it exists to absorb the retries of a +call still in flight, not to serve as a general result cache, so it holds a +fixed number of recent answers for about a minute. + +Making the key required rather than optional is what keeps the guarantee +whole. A server that accepts keyless requests silently loses deduplication for +any caller that forgets one, and "required" enforced by documentation is not +enforced. A keyless request is therefore rejected outright, and the rejection +names the header so the fix is obvious to whoever hits it — including a person +with `curl`. + +Retrying is now permanent behavior of this kind, not a workaround for one +platform's cold starts. Keyed retries are correct on any transport that can +drop a request, so nothing here is written to be deleted later, even though +one platform's specific behavior is what made shipping it urgent. + +### What this guarantees, and what it does not + +[`connection-contracts.md`](../10-domains/connection-contracts.md) sets the +scope this calibrates against: a connection is internal because both ends +belong to one application's topology, not because they share a network, and +robustness is justified per edge against the named failure modes of the +targets carrying it. + +The named failure here is a request dropped while its target boots. That +happens before a handler runs, so nothing was applied and a retry cannot +duplicate work — the keys are not what makes that case safe. What the keys add +is the narrower case where a handler did run and its answer was lost on the way +back: the retry finds the recorded answer instead of running the work twice. + +The server's memory of answers lives in the serving process, so it cannot cover +a retry that reaches a *different* instance than the one that did the work — +after an instance dies mid-request, or on an edge whose retries may be routed +elsewhere. That residue is deliberate. Closing it would put a durable store +behind every provider, and no supported target's failure modes ask for that +today. An edge that does need it has the tool without the framework imposing +it: the handler receives the key and can record it inside its own transaction, +making its own work exactly-once by its own storage's guarantees. + +## Consequences + +- **Every method is retryable, and no contract declares anything.** Adding + retry behavior needs no change to a contract, a method, or a handler. +- **A keyless request is a `400`.** Any caller that is not the generated + client — a test fixture, a `curl` command — must send a key. +- **Handlers may take a third argument** carrying the key. Existing + two-argument handlers are unaffected; the argument is optional. +- **A provider holds a bounded amount of recent answers in memory**, sized to + absorb in-flight retries rather than to grow with traffic. +- **Cross-instance duplicate suppression is the handler's to add**, using the + key it is given, when its target's failure modes warrant it. + +## Alternatives considered + +- **Per-method `idempotent: true`, retry only marked methods.** Rejected: the + framework cannot verify the claim, a wrong mark duplicates writes silently, + and the safe default guarantees under-marking. The key replaces a promise + with a mechanism. +- **Retry everything without keys.** Rejected: it converts a visible failure + into a silent duplicate write, on the assumption that every handler is + repeat-safe — the same unverifiable claim, made on the author's behalf. +- **Durable, cross-instance deduplication in the framework.** Rejected here: + it puts a storage dependency behind every provider to close a window no + supported target's named failure modes call for. The handler can reach that + guarantee for the edges that need it, using the key this decision gives it. +- **Client-side retry only, with no server-side deduplication.** Rejected: it + is exactly retry-without-keys from the caller's side, and leaves the + ambiguous case — answer lost after the work was done — duplicating. diff --git a/docs/design/90-decisions/README.md b/docs/design/90-decisions/README.md index 616f939a..c05b706d 100644 --- a/docs/design/90-decisions/README.md +++ b/docs/design/90-decisions/README.md @@ -58,3 +58,4 @@ _Earlier drafts (ADR-0001, ADR-0002) were retired as the high-level design settl - [ADR-0034](ADR-0034-deploy-state-lives-in-the-stage-branch.md) — Deploy state lives in a framework-owned `prisma-composer-state` database in the stage's own Branch (production: the implicit default Branch). State has the environment's lifetime: platform-side Branch/Project deletion cleans it up with no framework involvement; the CLI deletes it last-among-members on destroy. Supersedes ADR-0009's workspace-level store. - [ADR-0035](ADR-0035-public-entrypoints-live-in-src-exports.md) — Public entrypoints live in `src/exports/` (one file per subpath; internals stay at the `src/` root); `@internal/tsdown-config` generates `package.json#exports` from object-named entries where safe, with two deliberate exceptions kept hand-maintained (the multi-pass `cron`/`storage`/`streams`, and the two published packages). Completes ADR-0028. - [ADR-0036](ADR-0036-the-rpc-kind-is-named-service-rpc.md) — The RPC kind is named **service RPC**: subpath `@prisma/composer/service-rpc`, unchanged call-site names (`rpc()`, `contract()`, `serve()`), kind brand stays `'rpc'`. Scope recorded in connection-contracts.md: edges internal to the application topology, agent-generatable by design — not an application API layer, not general distributed-systems infrastructure. +- [ADR-0037](ADR-0037-service-rpc-calls-carry-an-idempotency-key.md) — Every service RPC call carries an `Idempotency-Key`: the client mints one per logical call and reuses it across a bounded retry; the server requires it, runs one call per key, and replays completed 2xx/4xx answers (never 5xx) from a bounded in-process store. Retrying is permanent protocol behavior, not a platform workaround, and there is no per-method opt-in — a flag would be an unverifiable claim. Handlers may read the key via an optional third argument for their own durable exactly-once. From 442cc82f7bfb20c8e891b448891333adc36d1f6a Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 17:05:24 +0200 Subject: [PATCH 09/18] docs(connection-contracts): the network binding retries safely on a key Adds the idempotency-key protocol to the network-binding section as a property of the binding, not the contract, alongside the service key. Corrects the per-call check: the server validates both directions; the client no longer re-validates the response, since both ends are framework-generated. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../design/10-domains/connection-contracts.md | 39 +++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/docs/design/10-domains/connection-contracts.md b/docs/design/10-domains/connection-contracts.md index e1be19dc..aca02951 100644 --- a/docs/design/10-domains/connection-contracts.md +++ b/docs/design/10-domains/connection-contracts.md @@ -184,6 +184,33 @@ generalizes the mechanism — the `serviceKey` connection param declares an opaq provisioning *need* that the deploy target resolves through its own registry, so core mints nothing and knows nothing about RPC. +### The binding retries safely, so a dropped request is not a lost call + +A network-bound provider may have scaled to zero and has to boot before it can +answer; a caller's first request can be dropped while that happens. The generated +client absorbs this: every call carries an `Idempotency-Key` header, and a call +whose request is dropped — a thrown network error, a `429`, or any `5xx` — is +retried with a bounded, jittered backoff. Any other `4xx` is not retried: a +malformed or unauthorized request stays wrong on a second attempt. + +The key is what makes retrying safe rather than a way to double-execute a write. +The client mints one key per *logical call* and sends the same key on every retry +of it, so the server can tell a retry from a new call: it runs one call per key, +lets a duplicate that arrives mid-flight wait for the first, and replays a +completed answer (a `2xx` or a `4xx` — never a `5xx`, which is the outcome a retry +exists to escape) to a later duplicate. The key is required — the server rejects a +keyless request with `400` naming the header — so deduplication is never silently +lost to a caller that forgot one. + +This is a property of the binding, like the service key above: an in-memory or +mock binding has no network hop to drop a request, so it neither retries nor +deduplicates, and the contract says nothing about either. The retry is permanent +behavior of a network binding, not a workaround for one platform's cold starts; +[ADR-0037](../90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md) +decides it, including what its in-process deduplication does not cover — a retry +that reaches a different provider instance than the one that did the work — and why +that residue is left to a handler that needs to close it. + ## How compatibility is checked Enforcement happens in three places: @@ -196,9 +223,15 @@ Enforcement happens in three places: contract value), so a structurally-equivalent-but-distinct contract does not match. That is the RPC contract's current implementation, not a rule the framework or the Contract abstraction imposes. -3. **Run (per call) — validate input and output against the contract's schemas.** - Catches a provider that is typed-compatible but lies at runtime: a bug, drift, or - a legacy server wrapped in a Service that TypeScript never saw. +3. **Run (per call) — the server validates both directions against the contract's + schemas.** It validates the request against the method's input schema, rejecting a + malformed caller with `400` and a message describing what was wrong, and validates + its own handler's return against the output schema before responding — which + catches a provider that is typed-compatible but lies at runtime: a bug, drift, or + a legacy server wrapped in a Service that TypeScript never saw. The generated + client does not validate the response a second time; both ends of every connection + are framework-generated, so a second pass on every call would re-check what the + server already guaranteed. ### The compile-time check is plain TypeScript assignability From d021eaa40c51a9bea0673e65ddaa127b11ecfdff Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 17:19:53 +0200 Subject: [PATCH 10/18] chore(drive): the rpc cold-start canary (PRO-217), plus a known limitation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/rpc-cold-start-canary.ts + rpc-cold-start-canary-classify.ts (+ its test) mirror cold-start-canary.ts wholesale: create -> upload -> start -> race the promote call, >=60s spacing including sample #0, 14-confirmed-cold-start-hold bug-gone budget, first-close early exit, MAX_RUN_MS self-stop, requirable exits. The probe is a bare single-attempt fetch straight at auth.service's POST /rpc/verify with a manually minted Idempotency-Key, never through makeClient — this slice just gave every framework RPC edge an automatic retry, and a probe through that client would mask the raw platform behavior being tested. New rpc-cold-start-canary job in e2e-deploy.yml, modeled on cold-start-canary, same deploy-verify-destroy action and needs: serialization, not added to any required-checks list. auth.service never logged anything on boot, so there was no listening line to read; examples/storefront-auth/modules/auth/src/server.ts now logs one self-timestamped line right after Bun.serve() returns (Compute's log relay passes plain app stdout through with no platform-added timestamp, verified live, so the app has to stamp its own, the same role streams-server's console.log patch plays for the streams face). auth.service is wired to exactly one RPC consumer (storefront), so per ADR-0030/0031 it enforces a per-edge bearer key this script cannot obtain: the Management API's own contract says an environment variable's value "is stored encrypted and is not returned by subsequent reads," confirmed live (an unauthenticated touch against a warm instance got back exactly serve()'s documented 401). Sending no Authorization header is the honest choice, and the classifier treats that specific, known 401 the same as a 2xx for classification purposes, since PRO-217's close happens at the ingress before serve()'s accepted-key check ever runs. Known limitation, evidenced by a 14-sample live run: auth.service's boot (spark starting bun -> the app's own listening line) is consistently well under 2s, so no touch's lead time can cross the 2000ms cross-clock-skew margin cold-start-canary-classify.ts's technique requires before calling a touch confirmed-cold. Every touch still raced a real boot (positive, consistent 376-1185ms gaps, 14/14 samples) and the close-decisive path is completely unaffected (a close needs no boot evidence), so the canary can still catch bug-present, but it cannot currently earn a bug-gone verdict against this target. Full numbers and a recommendation are in this dispatch's report to the orchestrator. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .github/workflows/e2e-deploy.yml | 35 ++ .../modules/auth/src/server.ts | 13 + .../rpc-cold-start-canary-classify.test.ts | 236 ++++++++++ scripts/rpc-cold-start-canary-classify.ts | 331 ++++++++++++++ scripts/rpc-cold-start-canary.ts | 420 ++++++++++++++++++ 5 files changed, 1035 insertions(+) create mode 100644 scripts/rpc-cold-start-canary-classify.test.ts create mode 100644 scripts/rpc-cold-start-canary-classify.ts create mode 100644 scripts/rpc-cold-start-canary.ts diff --git a/.github/workflows/e2e-deploy.yml b/.github/workflows/e2e-deploy.yml index 37447fc8..60cec687 100644 --- a/.github/workflows/e2e-deploy.yml +++ b/.github/workflows/e2e-deploy.yml @@ -133,3 +133,38 @@ jobs: verify-command: bun "${{ github.workspace }}/scripts/cold-start-canary.ts" destroy-label: 'streams-canary ' sweep-prefixes: storefront-auth pn-widgets hello canary streams-canary + + rpc-cold-start-canary: + name: RPC cold-start canary (PRO-217) + runs-on: ubuntu-latest + # After the streams face's canary, for the same quota-serialization reason + # as the rest of this chain — only one real-cloud deploy round runs at a + # time. Fails when the ingress no longer closes first-touch connections to + # a booting service-RPC provider — the signal to retire this canary (never + # the Idempotency-Key protocol or the bounded retry in service-rpc, which + # are permanent protocol semantics for this kind, not a PRO-217 + # compensation). The bug being PRESENT is today's normal and passes. + # + # This face has no warmup phase and no durability wait (unlike the streams + # canary): the script itself is the caller, and auth.service keeps no + # local state to restore between samples, so its own budget is at most as + # large as cold-start-canary.ts's. NOT required until Will adds it. + needs: cold-start-canary + timeout-minutes: 30 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + env: + PRISMA_SERVICE_TOKEN: ${{ secrets.PRISMA_SERVICE_TOKEN }} + PRISMA_WORKSPACE_ID: ${{ vars.PRISMA_WORKSPACE_ID }} + AUTH_SIGNING_SECRET: sk_test_ci_storefront_auth + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - uses: ./.github/actions/deploy-verify-destroy + with: + working-directory: examples/storefront-auth + build-filter: '@prisma/example-storefront-auth...' + stack-name: storefront-auth-rpc-canary-ci-${{ github.run_id }} + verify-command: bun "${{ github.workspace }}/scripts/rpc-cold-start-canary.ts" + destroy-label: 'storefront-auth-rpc-canary ' + sweep-prefixes: storefront-auth pn-widgets hello canary streams-canary storefront-auth-rpc-canary diff --git a/examples/storefront-auth/modules/auth/src/server.ts b/examples/storefront-auth/modules/auth/src/server.ts index 9783043e..9ec38da2 100644 --- a/examples/storefront-auth/modules/auth/src/server.ts +++ b/examples/storefront-auth/modules/auth/src/server.ts @@ -56,3 +56,16 @@ export default handler; // Bind all interfaces — Compute routes external HTTP to the VM, so a // loopback-only listener would be unreachable. Bun.serve({ port, hostname: '0.0.0.0', fetch: handler }); +// The RPC cold-start canary (scripts/rpc-cold-start-canary.ts) reads this +// line out of the deployment's boot log to prove a touch was sent before the +// server could answer anything — the same technique +// scripts/cold-start-canary.ts already relies on for the streams module. +// Bun.serve binds synchronously, so logging immediately after it returns +// marks the moment this service is actually ready to accept connections. +// The timestamp is stamped here, in-process, rather than left to the +// platform: unlike @prisma/streams-server (which patches console.log to +// prepend one), Compute's log relay passes plain app stdout through +// unmodified — verified live, an unstamped line arrives with no timestamp +// at all — so a line with no timestamp of its own would be unusable +// evidence for classifyBootEvidence's cross-clock comparison. +console.log(`[${new Date().toISOString()}] [INFO] auth server listening on 0.0.0.0:${port}`); diff --git a/scripts/rpc-cold-start-canary-classify.test.ts b/scripts/rpc-cold-start-canary-classify.test.ts new file mode 100644 index 00000000..e1cb2475 --- /dev/null +++ b/scripts/rpc-cold-start-canary-classify.test.ts @@ -0,0 +1,236 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { + CLOCK_SKEW_MARGIN_MS, + classifyBootEvidence, + classifyRpcColdStartRun, + classifyRpcColdStartTouch, + findListeningTimestamp, + MAX_FALSE_CLEAN_PROBABILITY, + MIN_HELD_SAMPLES_FOR_BUG_GONE, + type RpcColdStartTouch, + stripAnsiCodes, + TARGET_CLOSE_RATE, +} from './rpc-cold-start-canary-classify.ts'; + +describe('classifyRpcColdStartTouch', () => { + it('a 200 confirmed cold → held (the edge carried the request through a real boot)', () => { + assert.equal(classifyRpcColdStartTouch(200, '{"ok":true}', false, 'confirmed-cold'), 'held'); + }); + + it("the canary's own known 401 (missing service key), confirmed cold → held — the ingress still carried it through", () => { + const body = '{"error":"Unauthorized: missing or invalid service key"}'; + assert.equal(classifyRpcColdStartTouch(401, body, false, 'confirmed-cold'), 'held'); + }); + + it('a 200 confirmed warm → no-cold-start (it proves nothing about PRO-217)', () => { + assert.equal( + classifyRpcColdStartTouch(200, '{"ok":true}', false, 'confirmed-warm'), + 'no-cold-start', + ); + }); + + it('the known 401, confirmed warm → no-cold-start', () => { + const body = '{"error":"Unauthorized: missing or invalid service key"}'; + assert.equal(classifyRpcColdStartTouch(401, body, false, 'confirmed-warm'), 'no-cold-start'); + }); + + it('a 200 with unknown boot evidence → other (log read failed or too close to call — not a guess)', () => { + assert.equal(classifyRpcColdStartTouch(200, '{"ok":true}', false, 'unknown'), 'other'); + }); + + it('a thrown fetch error naming the close → closed (the PRO-217 signal), regardless of boot evidence', () => { + const message = 'fetch failed: The socket connection was closed unexpectedly'; + assert.equal(classifyRpcColdStartTouch(0, message, true, 'confirmed-cold'), 'closed'); + assert.equal(classifyRpcColdStartTouch(0, message, true, 'unknown'), 'closed'); + }); + + it('reset/refused faces of the same close → closed, whether thrown or answered', () => { + for (const body of ['ECONNRESET while fetching', 'connect ECONNREFUSED', 'socket hang up']) { + assert.equal(classifyRpcColdStartTouch(0, body, true, 'unknown'), 'closed', body); + assert.equal(classifyRpcColdStartTouch(502, body, false, 'unknown'), 'closed', body); + } + }); + + it('an unrelated 401 (a real caller bug, not the missing-key message) → other, not held', () => { + const body = '{"error":"Unauthorized: something else entirely"}'; + assert.equal(classifyRpcColdStartTouch(401, body, false, 'confirmed-cold'), 'other'); + }); + + it('a 500 → other, regardless of boot evidence — a real app bug must not count as proof the platform is healthy', () => { + assert.equal( + classifyRpcColdStartTouch(500, '{"error":"Internal server error"}', false, 'confirmed-cold'), + 'other', + ); + }); + + it('any other unexpected status → other', () => { + assert.equal(classifyRpcColdStartTouch(404, 'not found', false, 'confirmed-cold'), 'other'); + assert.equal(classifyRpcColdStartTouch(413, 'too large', false, 'unknown'), 'other'); + }); + + it('a thrown error that does not name a close → other, not closed', () => { + assert.equal( + classifyRpcColdStartTouch(0, 'TimeoutError: signal timed out', true, 'unknown'), + 'other', + ); + }); +}); + +describe('stripAnsiCodes', () => { + it('removes SGR escape sequences from spark boot log lines', () => { + const colorized = `${String.fromCharCode(27)}[90m[${String.fromCharCode(27)}[0m2026-07-20T14:45:51Z ${String.fromCharCode(27)}[32mINFO ${String.fromCharCode(27)}[0m spark::app_source${String.fromCharCode(27)}[90m]${String.fromCharCode(27)}[0m compute.manifest.json not found`; + assert.equal( + stripAnsiCodes(colorized), + '[2026-07-20T14:45:51Z INFO spark::app_source] compute.manifest.json not found', + ); + }); + + it('leaves plain text untouched', () => { + assert.equal(stripAnsiCodes('[INFO] plain line, no escapes'), '[INFO] plain line, no escapes'); + }); +}); + +describe('findListeningTimestamp', () => { + it("reads auth's own listening line", () => { + const log = + 'spark: starting bun with entrypoint: bootstrap.js\r\n' + + '[2026-07-20T14:45:51.926Z] [INFO] auth server listening on 0.0.0.0:3000\r\n'; + const found = findListeningTimestamp(log); + assert.ok(found); + assert.equal(found?.toISOString(), '2026-07-20T14:45:51.926Z'); + }); + + it('returns undefined when the log never reached a listening line (e.g. read cut off mid-boot)', () => { + const log = + 'spark: starting bun with entrypoint: bootstrap.js\r\n' + + 'spark: time-sync maintenance child started\r\n'; + assert.equal(findListeningTimestamp(log), undefined); + }); + + it('returns undefined for an empty or unrelated log', () => { + assert.equal(findListeningTimestamp(''), undefined); + assert.equal(findListeningTimestamp('some other server started fine'), undefined); + }); + + it("does not match a differently-named service (e.g. the streams face's own listening line)", () => { + const log = + '[2026-07-20T14:45:51.926Z] [INFO] prisma-streams server listening on 0.0.0.0:3000\r\n'; + assert.equal(findListeningTimestamp(log), undefined); + }); +}); + +describe('classifyBootEvidence (margin-aware, cross-clock comparison)', () => { + const listeningAt = new Date('2026-07-20T14:45:51.926Z'); + + it('touch sent comfortably before listening (beyond the skew margin) → confirmed-cold', () => { + const touchSentAt = new Date(listeningAt.getTime() - CLOCK_SKEW_MARGIN_MS - 1); + assert.equal(classifyBootEvidence(touchSentAt, listeningAt), 'confirmed-cold'); + }); + + it('touch sent exactly at the margin boundary before listening → confirmed-cold (>=)', () => { + const touchSentAt = new Date(listeningAt.getTime() - CLOCK_SKEW_MARGIN_MS); + assert.equal(classifyBootEvidence(touchSentAt, listeningAt), 'confirmed-cold'); + }); + + it('touch sent comfortably after listening (beyond the skew margin) → confirmed-warm', () => { + const touchSentAt = new Date(listeningAt.getTime() + CLOCK_SKEW_MARGIN_MS + 1); + assert.equal(classifyBootEvidence(touchSentAt, listeningAt), 'confirmed-warm'); + }); + + it('touch sent within the skew margin on either side of listening → unknown (could be skew, not order)', () => { + const justBefore = new Date(listeningAt.getTime() - CLOCK_SKEW_MARGIN_MS + 1); + const justAfter = new Date(listeningAt.getTime() + CLOCK_SKEW_MARGIN_MS - 1); + assert.equal(classifyBootEvidence(justBefore, listeningAt), 'unknown'); + assert.equal(classifyBootEvidence(justAfter, listeningAt), 'unknown'); + }); + + it('no listening timestamp at all → unknown, not a guess', () => { + assert.equal(classifyBootEvidence(new Date(), undefined), 'unknown'); + }); +}); + +describe('the sample-budget arithmetic', () => { + it('MIN_HELD_SAMPLES_FOR_BUG_GONE is the smallest N keeping an all-held run at or under 5% chance at a 20% close rate', () => { + assert.equal(TARGET_CLOSE_RATE, 0.2); + assert.equal(MAX_FALSE_CLEAN_PROBABILITY, 0.05); + assert.equal(MIN_HELD_SAMPLES_FOR_BUG_GONE, 14); + const chance = (n: number) => (1 - TARGET_CLOSE_RATE) ** n; + assert.ok(chance(MIN_HELD_SAMPLES_FOR_BUG_GONE) <= MAX_FALSE_CLEAN_PROBABILITY); + assert.ok(chance(MIN_HELD_SAMPLES_FOR_BUG_GONE - 1) > MAX_FALSE_CLEAN_PROBABILITY); + }); +}); + +describe('classifyRpcColdStartRun (the three-exit mapping of a REQUIRED check)', () => { + const run = (...touches: RpcColdStartTouch[]) => classifyRpcColdStartRun(touches); + const heldTimes = (n: number): RpcColdStartTouch[] => Array.from({ length: n }, () => 'held'); + + it('no touches → inconclusive (broken canary; warn, do not block)', () => { + assert.equal(run().verdict, 'inconclusive'); + }); + + it("one close among holds → bug-present (exit 0; today's normal)", () => { + const result = run('held', 'closed', 'held', 'held'); + assert.equal(result.verdict, 'bug-present'); + assert.match(result.message, /1\/4 first touches closed/); + assert.match(result.message, /PRO-217 not fixed/); + assert.match(result.message, /not a compensation/); + }); + + it('a close is decisive even alongside touches that never went cold', () => { + const result = run('closed', 'no-cold-start', 'no-cold-start', 'no-cold-start'); + assert.equal(result.verdict, 'bug-present'); + }); + + it('a close is decisive even in a run large enough to otherwise reach the bug-gone budget', () => { + const result = classifyRpcColdStartRun([...heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE), 'closed']); + assert.equal(result.verdict, 'bug-present'); + }); + + it('all held but fewer than MIN_HELD_SAMPLES_FOR_BUG_GONE → inconclusive, not bug-gone (an all-held run this small is the expected outcome of an intermittent bug)', () => { + const result = classifyRpcColdStartRun(heldTimes(4)); + assert.equal(result.verdict, 'inconclusive'); + assert.match(result.message, /All 4 confirmed cold-start touches held/); + assert.match(result.message, /41\.0%/); // 0.8^4 + assert.match(result.message, new RegExp(String(MIN_HELD_SAMPLES_FOR_BUG_GONE))); + assert.match(result.message, /not blocking/i); + }); + + it('all held at exactly MIN_HELD_SAMPLES_FOR_BUG_GONE → bug-gone (exit 1 — the forcing signal), actionable for a cold reader', () => { + const result = classifyRpcColdStartRun(heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE)); + assert.equal(result.verdict, 'bug-gone'); + assert.match(result.message, /4\.4%/); // 0.8^14 + assert.match(result.message, /not because of your change/); + assert.match(result.message, /rpc-cold-start-canary\.ts/); + assert.match(result.message, /e2e-deploy\.yml/); + assert.match(result.message, /gotchas\.md/); + assert.match(result.message, /do NOT remove the Idempotency-Key protocol/); + }); + + it('one held short of the budget → inconclusive, not bug-gone', () => { + const result = classifyRpcColdStartRun(heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE - 1)); + assert.equal(result.verdict, 'inconclusive'); + }); + + it('any touch that never went cold makes the whole run inconclusive, even with no closes and plenty of holds', () => { + const result = classifyRpcColdStartRun([ + ...heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE), + 'no-cold-start', + ]); + assert.equal(result.verdict, 'inconclusive'); + assert.match(result.message, /failed to force a cold start/); + assert.match(result.message, /not blocking/); + }); + + it('all touches never going cold → inconclusive, not a clean bill of health', () => { + const result = run('no-cold-start', 'no-cold-start', 'no-cold-start', 'no-cold-start'); + assert.equal(result.verdict, 'inconclusive'); + assert.match(result.message, /4\/4 touches/); + }); + + it('an "other" (broken/ambiguous) touch also blocks a bug-gone verdict', () => { + const result = classifyRpcColdStartRun([...heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE), 'other']); + assert.equal(result.verdict, 'inconclusive'); + }); +}); diff --git a/scripts/rpc-cold-start-canary-classify.ts b/scripts/rpc-cold-start-canary-classify.ts new file mode 100644 index 00000000..7ccd10e8 --- /dev/null +++ b/scripts/rpc-cold-start-canary-classify.ts @@ -0,0 +1,331 @@ +/** + * Pass/fail logic for rpc-cold-start-canary.ts (PRO-217, service-RPC face) — + * the service-rpc sibling of cold-start-canary-classify.ts, same intermittent- + * bug arithmetic applied to a direct `POST /rpc/` touch instead + * of the streams module's `POST /jobs`. + * + * A touch against a freshly promoted `auth.service` instance lands on one of + * four outcomes: + * + * - the connection itself is reset mid-establishment (a thrown fetch error, + * or a response naming the close) — the bug reproduced. A close only + * happens during the boot window, so it alone proves the touch reached a + * cold start; no further evidence is needed. + * - the target answered (any real HTTP response, not a connection error) and + * the deployment's own boot log confirms the touch was sent before the + * server finished booting — the edge held the connection through a real + * cold start. Genuine evidence toward "fixed". + * - the target answered before there was anything left to boot through — no + * cold start happened, so the touch says nothing about the bug either way. + * - the log evidence can't place the touch on either side of the boot (read + * cut off, or within the clock-skew margin) — inconclusive, not guessed. + * + * "The target answered" deliberately covers more than a successful RPC + * result. This canary cannot supply the real per-edge Authorization bearer + * key ADR-0030/0031 mints for the storefront -> auth wiring: the Management + * API's own contract states plainly that an environment variable's value "is + * stored encrypted and is not returned by subsequent reads" — verified live + * against a real deploy, where an unauthenticated touch got back exactly + * `401 {"error":"Unauthorized: missing or invalid service key"}` from a warm + * instance. That 401 is `serve()`'s own documented rejection + * (packages/0-framework/2-authoring/service-rpc/src/serve.ts) for a request + * that never had a chance to supply a key this script cannot obtain — not a + * sign PRO-217 fired. The property this canary needs from a "held" touch is + * "the ingress carried the request through to the application", and a 401 + * proves exactly that as cleanly as a 2xx would: `serve()`'s accepted-key + * check runs after the connection is already established, so reaching it at + * all means the ingress did not reset the connection. Observed 401 latencies + * during this canary's build (983-1102ms) track normal boot-racing latency, + * not the ~400ms fast-fail PRO-217's close produces — further evidence this + * is the auth check running on an already-live connection, not the close. + * Any OTHER status or body is left `other` rather than folded in: a genuine + * application bug must not silently count as proof the platform is healthy. + */ + +/** + * A raw, unauthenticated single-attempt `fetch` hitting the RPC endpoint + * directly surfaces PRO-217 as a thrown error (Bun's `fetch` reports "The + * socket connection was closed unexpectedly" for a reset), not as a 502 — + * that shape only came from the streams canary's intermediary `jobs` caller + * catching the error itself and wrapping it into a response body. Both + * shapes are checked here for the same fragments regardless, since it costs + * nothing and guards against the platform's exact behavior differing across + * targets. Keep in sync with gotchas.md's PRO-217 entry. + */ +const CLOSE_FRAGMENTS = [ + 'socket connection was closed', + 'econnreset', + 'econnrefused', + 'socket hang up', +]; + +/** One first-touch outcome against a freshly promoted `auth.service` instance. */ +export type RpcColdStartTouch = 'held' | 'closed' | 'no-cold-start' | 'other'; + +/** + * The caller's answer to "did this touch actually race a boot?", decided by + * `classifyBootEvidence` from the deployment's own logs before + * `classifyRpcColdStartTouch` is called. Identical three-way split to + * cold-start-canary-classify.ts's `BootEvidence` — see that file for the + * full reasoning; duplicated here (not imported) so this canary and its + * gotchas paragraph can be deleted independently of the streams one. + */ +export type BootEvidence = 'confirmed-cold' | 'confirmed-warm' | 'unknown'; + +/** + * `auth`'s own documented rejection for a request that never presented (or + * presented the wrong) per-edge service key — see `serve()`'s accepted-keys + * check. Matched verbatim so a coincidental, unrelated 401 does not get + * folded into "the target answered". + */ +const UNAUTHORIZED_MISSING_KEY_MESSAGE = 'Unauthorized: missing or invalid service key'; + +/** + * Whether `status`/`body` is a genuine application response — proof the + * ingress carried the connection through to `serve()` — as opposed to a + * connection-level failure or an unrelated error this canary should not + * interpret either way. A real RPC success (2xx) counts; so does the + * specific, known 401 this canary's own missing service key always produces + * (see the module comment). Nothing else does — a 500 or an unexpected 4xx + * is left uninterpreted rather than assumed harmless. + */ +function targetAnswered(status: number, body: string): boolean { + if (status >= 200 && status < 300) return true; + return status === 401 && body.includes(UNAUTHORIZED_MISSING_KEY_MESSAGE); +} + +/** + * Classifies one first-touch outcome from the CALLER's seat. `body` is + * either a response body (fetch resolved) or the stringified error a thrown + * fetch rejection produced (connection-level failure) — the caller passes + * whichever it got, and `wasThrown` says which. + * + * A close is decisive regardless of `bootEvidence`: it only happens + * mid-boot, so it is its own proof. A response that reaches `serve()` + * (`targetAnswered`) only becomes `held` when `bootEvidence` is + * `confirmed-cold`; `confirmed-warm` makes it `no-cold-start` (the touch + * proves nothing, because nothing was booting when it landed), and `unknown` + * makes it `other`. A thrown error that isn't a close, or a response that + * isn't `targetAnswered`, is `other` either way. + */ +export function classifyRpcColdStartTouch( + status: number, + body: string, + wasThrown: boolean, + bootEvidence: BootEvidence, +): RpcColdStartTouch { + const lower = body.toLowerCase(); + if (CLOSE_FRAGMENTS.some((fragment) => lower.includes(fragment))) { + return 'closed'; + } + if (!wasThrown && targetAnswered(status, body)) { + if (bootEvidence === 'confirmed-cold') return 'held'; + if (bootEvidence === 'confirmed-warm') return 'no-cold-start'; + return 'other'; + } + return 'other'; +} + +/** + * Strips ANSI SGR color codes from spark's boot log lines. The `auth` + * service's own listening line (added for this canary — see + * examples/storefront-auth/modules/auth/src/server.ts) is not colorized, + * but spark's surrounding platform lines are, and stripping first keeps the + * timestamp regex robust regardless of what shares the log stream. Built + * from String.fromCharCode rather than a regex literal containing the raw + * ESC byte, which Biome's noControlCharactersInRegex rule (rightly) rejects. + */ +export function stripAnsiCodes(text: string): string { + const ESC = String.fromCharCode(27); + return text.split(new RegExp(`${ESC}\\[[0-9;]*m`, 'g')).join(''); +} + +/** + * `auth`'s own boot line — e.g. "[2026-07-20T14:45:51.926Z] [INFO] auth + * server listening on 0.0.0.0:3000" — read from a deployment's log history + * (`?from_start=true`). Compute's log relay passes plain app stdout through + * unmodified (verified live: an unstamped line arrives with no timestamp of + * its own), so the timestamp is the app's own — stamped in server.ts rather + * than left to the platform, unlike @prisma/streams-server's console.log + * patch. Returns the timestamp it logged, or undefined if the boot never + * reached it (or the log read didn't cover it). + */ +export function findListeningTimestamp(logText: string): Date | undefined { + const match = stripAnsiCodes(logText).match( + /\[(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z)]\s*\[INFO]\s*auth server listening/, + ); + const timestamp = match?.[1]; + return timestamp !== undefined ? new Date(timestamp) : undefined; +} + +/** + * `touchSentAt` is `new Date()` on the CI runner; `listeningAt` is parsed out + * of a timestamp `auth`'s own server.ts wrote with its own `new Date()` on a + * Prisma Compute VM. Nothing keeps those two clocks in lockstep, so a + * touch/listening gap of a few tens or hundreds of milliseconds is not by + * itself proof of ordering — it could be clock skew rather than a genuine + * race. Same margin and reasoning as cold-start-canary-classify.ts's + * CLOCK_SKEW_MARGIN_MS: two well-behaved NTP-synced hosts are not expected + * to disagree by more than a few hundred milliseconds, and 2 seconds is a + * comfortable multiple of that while staying small next to the 3-22s boot + * windows this family of canaries samples. + */ +export const CLOCK_SKEW_MARGIN_MS = 2_000; + +/** + * Decides, from the deployment's own boot log, which side of the boot a + * touch landed on. Returns `unknown` — not a guess — when the log never + * showed a `listening` line, or when the touch and `listening` are within + * CLOCK_SKEW_MARGIN_MS of each other and cross-clock skew could plausibly + * explain the gap either way. + */ +export function classifyBootEvidence( + touchSentAt: Date, + listeningAt: Date | undefined, +): BootEvidence { + if (listeningAt === undefined) return 'unknown'; + const touchBeforeListeningByMs = listeningAt.getTime() - touchSentAt.getTime(); + if (touchBeforeListeningByMs >= CLOCK_SKEW_MARGIN_MS) return 'confirmed-cold'; + if (touchBeforeListeningByMs <= -CLOCK_SKEW_MARGIN_MS) return 'confirmed-warm'; + return 'unknown'; +} + +/** + * The three exits a REQUIRED check needs (the job fails only on the + * conclusive forcing signal): + * - `bug-present` → exit 0 (a close occurred; today's normal), + * - `bug-gone` → exit 1 (enough touches reached a genuinely fresh, booting + * instance and every one of them held that an all-held result is strong + * evidence, not luck — the actionable removal message is the point of the + * failure), + * - `inconclusive` → exit 0 plus a CI warning annotation (loud, not blocking + * every PR on a deploy flake, a run that never managed to force a cold + * start, or a run too small to trust; a human should look). + */ +export type RpcColdStartVerdict = 'bug-present' | 'bug-gone' | 'inconclusive'; + +export interface RpcColdStartResult { + readonly verdict: RpcColdStartVerdict; + readonly message: string; +} + +/** + * Same target close rate as cold-start-canary-classify.ts's + * TARGET_CLOSE_RATE, and the same reasoning: deliberately conservative + * relative to the 60-100% close rates manual reproduction has actually + * observed for this family of bug, so the sample budget below stays + * trustworthy even if the RPC face's real defect rate is lower than the + * streams face's. + */ +export const TARGET_CLOSE_RATE = 0.2; + +/** + * The most a bug-gone verdict is allowed to be "all held by luck": if the + * true close rate were TARGET_CLOSE_RATE and the bug were still present, the + * chance of seeing every one of MIN_HELD_SAMPLES_FOR_BUG_GONE independent + * confirmed cold starts hold is at most this. + */ +export const MAX_FALSE_CLEAN_PROBABILITY = 0.05; + +/** + * The number of confirmed cold-start holds classifyRpcColdStartRun requires + * before it will say bug-gone. Identical arithmetic to + * cold-start-canary-classify.ts's MIN_HELD_SAMPLES_FOR_BUG_GONE: + * + * 0.8^13 ≈ 5.50% (not low enough) + * 0.8^14 ≈ 4.40% (first N at or below 5%) + * + * so N = 14. + */ +export const MIN_HELD_SAMPLES_FOR_BUG_GONE = Math.ceil( + Math.log(MAX_FALSE_CLEAN_PROBABILITY) / Math.log(1 - TARGET_CLOSE_RATE), +); + +function chanceAllHoldByLuck(heldCount: number): number { + return (1 - TARGET_CLOSE_RATE) ** heldCount; +} + +function asPercent(probability: number): string { + return `${(probability * 100).toFixed(1)}%`; +} + +/** + * Aggregates N first touches. A close anywhere is decisive on its own (rule: + * a close only happens mid-boot, so it needs no corroboration). Short of + * that, a touch that landed on an already-warm instance (`no-cold-start`) or + * came back some other inconclusive way (`other`) means the run never earned + * an opinion from that touch, so ANY of those makes the whole run + * `inconclusive` rather than mixing an uninformative touch into a "clean" + * verdict. And even a run where every touch held is only allowed to say + * "fixed" once it has collected MIN_HELD_SAMPLES_FOR_BUG_GONE confirmed + * cold-start holds — see that constant's comment for why a smaller all-held + * run is not evidence. + */ +export function classifyRpcColdStartRun(touches: readonly RpcColdStartTouch[]): RpcColdStartResult { + const n = touches.length; + if (n === 0) return { verdict: 'inconclusive', message: 'Canary made no touches — broken.' }; + const count = (t: RpcColdStartTouch) => touches.filter((x) => x === t).length; + const closed = count('closed'); + const held = count('held'); + const noColdStart = count('no-cold-start'); + const other = count('other'); + + if (closed > 0) { + return { + verdict: 'bug-present', + message: + `Cold-start close still present on the service-RPC edge (${closed}/${n} first touches ` + + `closed, ${held} held, ${noColdStart} never went cold) — PRO-217 not fixed; keep the ` + + 'bounded retry over keyed calls in service-rpc (client.ts/serve.ts) — that is permanent ' + + 'protocol semantics for this kind, not a compensation for this bug.', + }; + } + + if (noColdStart > 0 || other > 0) { + return { + verdict: 'inconclusive', + message: + `The canary failed to force a cold start on ${noColdStart + other}/${n} touches ` + + `(${noColdStart} landed on an already-warm instance, ${other} were otherwise ` + + 'inconclusive) — a run that never reaches a cold instance has no opinion to report on ' + + 'PRO-217. A human should look; not blocking.', + }; + } + + // Every touch reached a fresh, booting instance and held (noColdStart === 0, + // other === 0, closed === 0), so held === n here. Whether that is enough + // still depends on how many holds it actually is. + if (held < MIN_HELD_SAMPLES_FOR_BUG_GONE) { + return { + verdict: 'inconclusive', + message: + `All ${held} confirmed cold-start touches held, but PRO-217 is intermittent, so that is ` + + 'the outcome a too-small sample is expected to produce even with the bug fully present: ' + + 'even at a conservative 20% close rate (well below the 60-100% close rates seen in manual ' + + `reproduction against the streams face of this bug), the chance that ${held} independent ` + + `cold starts would all happen to hold is ${asPercent(chanceAllHoldByLuck(held))}. This run ` + + `needs at least ${MIN_HELD_SAMPLES_FOR_BUG_GONE} confirmed cold-start holds before an ` + + `all-held result drops that chance to ${asPercent(MAX_FALSE_CLEAN_PROBABILITY)} or below ` + + `(0.8^${MIN_HELD_SAMPLES_FOR_BUG_GONE} ≈ ` + + `${asPercent(chanceAllHoldByLuck(MIN_HELD_SAMPLES_FOR_BUG_GONE))}). Not blocking.`, + }; + } + + return { + verdict: 'bug-gone', + message: + `All ${n} first touches against genuinely fresh, still-booting instances were held to ` + + `success — ${held} confirmed cold-start holds with zero closes. Even at a conservative 20% ` + + 'close rate (well below the 60-100% close rates seen in manual reproduction against the ' + + 'streams face of this bug), the chance of that happening by luck alone is only ' + + `${asPercent(chanceAllHoldByLuck(held))}, so this counts as real evidence: the platform no ` + + "longer resets a service-RPC edge's first-touch connection during a cold start. To fix this " + + 'build (you are seeing it because the cleanup is now due, not because of your change): ' + + '1) remove scripts/rpc-cold-start-canary.ts, scripts/rpc-cold-start-canary-classify.ts (+ ' + + 'its test) and the "RPC cold-start canary (PRO-217)" job in .github/workflows/e2e-deploy.yml; ' + + "2) drop the service-RPC paragraph from gotchas.md's PRO-217 entry; 3) do NOT remove the " + + "Idempotency-Key protocol or the bounded retry in service-rpc's client.ts/serve.ts — those " + + 'are permanent protocol semantics for this kind (safe retries on every call), not a PRO-217 ' + + 'compensation, and stay regardless of this verdict.', + }; +} diff --git a/scripts/rpc-cold-start-canary.ts b/scripts/rpc-cold-start-canary.ts new file mode 100644 index 00000000..d62fb636 --- /dev/null +++ b/scripts/rpc-cold-start-canary.ts @@ -0,0 +1,420 @@ +#!/usr/bin/env bun +/** + * Canary for PRO-217 (the Compute ingress closing a first-touch connection + * while a scale-to-zero service boots) — the service-RPC sibling of + * cold-start-canary.ts, run as the VERIFY step of a deploy-verify-destroy + * round over examples/storefront-auth (the deploy and teardown are the + * action's; this script only samples). + * + * Shape: this script IS the caller — a bare, single-attempt `fetch` straight + * at `auth.service`'s own `POST /rpc/verify` endpoint, carrying a manually + * minted `Idempotency-Key` header. It deliberately does NOT go through + * `makeClient` (`@prisma/composer/service-rpc`) or any framework client: + * this slice just gave every framework RPC edge a bounded, automatic retry + * over the same idempotency key (packages/0-framework/2-authoring/service-rpc), + * so a probe built on that client would have PRO-217's raw first-touch + * behavior masked by the very retry this slice ships — the platform's actual + * behavior has to stay observable, not smoothed over by the client under + * test. Each sample forces a genuinely fresh `auth.service` instance (create + * a deployment, upload the artifact already built by this job's Deploy step, + * start it, promote it to the app's stable endpoint), fires ONE first-touch + * `POST /rpc/verify` the instant the promote call succeeds, then reads the + * deployment's own boot logs to confirm the touch actually raced the boot — + * not just that a fresh instance existed somewhere. + * + * This inherits cold-start-canary.ts's contract wholesale — the same 2026- + * 07-17 rebuild that fixed two ways the original streams canary reported + * "fixed" while PRO-217 was live: it never actually hit a cold start (it + * waited for `running` before touching, which flips ~1s after `start`, long + * before the app is listening), and its verdict rule treated "every touch + * happened to hold" as proof of absence for an intermittent bug. Read that + * file's own module comment for the full two-defect history; the fixes are + * the same fixes here: + * + * 1. Race the promote call itself (retrying immediately on its 409 "not + * running yet") instead of polling for `running` — see `sampleFreshStart` + * below. + * 2. Space samples at least SAMPLE_INTERVAL_MS apart, including before + * sample #0 — back-to-back promotions land on some kind of already-warm + * host resource and produce atypically short boots the close does not + * appear in (cold-start-canary.ts's module comment; gotchas.md's PRO-217 + * entry has the measured boot times). + * 3. Prove coldness from the deployment's own boot log + * (`/v1/deployments/{id}/logs?from_start=true`), margin-aware against + * cross-clock skew, rather than inferring it from latency — see + * rpc-cold-start-canary-classify.ts's `classifyBootEvidence`. + * 4. Require MIN_HELD_SAMPLES_FOR_BUG_GONE confirmed cold-start holds before + * a bug-gone verdict, since an intermittent bug's expected outcome from a + * too-small sample is "every touch happened to hold" even while the bug + * is fully present. + * + * What's different from the streams face, beyond the raw-fetch requirement + * above: + * + * - This canary has no "caller" to warm. cold-start-canary.ts warms the + * `jobs` service first so a cold `jobs` can't be mistaken for a cold + * `streams`; here, THIS SCRIPT is the caller (an ephemeral fetch from the + * CI runner, not a Compute service with its own cold start), so there is + * nothing upstream of the touch that needs warming. + * - No durability wait. The streams canary waits for a just-created stream + * to reach the object store before sampling, because a fresh streams + * instance restores its local state from there. `auth.service` keeps no + * local state to restore — its `verify` handler pings Postgres fresh on + * every call — so there is nothing to wait for between promoting and + * sampling beyond the inherited SAMPLE_INTERVAL_MS spacing itself. + * - No Authorization header. `auth.service` is wired to exactly one RPC + * consumer (`storefront`, via `deps: { auth: rpc(authContract) }` in + * examples/storefront-auth/modules/storefront/src/service.ts), so per + * ADR-0030/0031 it does enforce a per-edge bearer key. This script cannot + * obtain that key: it is minted once at deploy time and, per the + * Management API's own contract for environment variables, "is stored + * encrypted and is not returned by subsequent reads" — verified live + * against a real deploy, where an unauthenticated touch against a warm + * `auth.service` got back exactly `401 {"error":"Unauthorized: missing or + * invalid service key"}`. Sending no Authorization header (never a guessed + * one) is the honest choice, and rpc-cold-start-canary-classify.ts treats + * that specific, known 401 the same as a real success for classification + * purposes — see its module comment for why that is still sound evidence + * for PRO-217 specifically (the accepted-key check runs only after the + * ingress has already carried the connection through). + * - No listening-line source in the example as written. Unlike + * @prisma/streams-server, `auth.service`'s own server.ts never logged + * anything on boot, so cold-start-canary.ts's technique had nothing to + * read. examples/storefront-auth/modules/auth/src/server.ts now logs one + * self-timestamped line right after `Bun.serve()` returns (see that + * file's comment) — Compute's log relay passes plain app stdout through + * completely unmodified (verified live: no platform-added timestamp), so + * the timestamp has to come from the app's own clock, the same as + * streams-server's console.log patch does for the streams face. + * + * A REQUIRED check: any close → exit 0, bug still present (today's normal); + * enough touches reaching a genuine cold start AND holding → exit 1, the + * signal to retire this canary (never the Idempotency-Key protocol or the + * bounded retry — those are permanent protocol semantics for this kind, not + * a PRO-217 compensation); a run that never manages to force a cold start, + * or one whose log evidence can't place a touch on either side of the boot, + * or one too small to trust an all-held result from → exit 0 with a CI + * warning annotation. + */ +import { execSync } from 'node:child_process'; +import * as os from 'node:os'; +import { + classifyBootEvidence, + classifyRpcColdStartRun, + classifyRpcColdStartTouch, + findListeningTimestamp, + MIN_HELD_SAMPLES_FOR_BUG_GONE, + type RpcColdStartTouch, +} from './rpc-cold-start-canary-classify.ts'; + +const API = 'https://api.prisma.io/v1'; +/** + * MIN_HELD_SAMPLES_FOR_BUG_GONE confirmed cold-start holds are what a + * bug-gone verdict needs; sampling fewer than that can never produce one + * (classifyRpcColdStartRun reports inconclusive instead), so that count is + * the default budget. A close is decisive the moment it happens (see the + * early-exit in the sampling loop below), so a run against a stack where the + * bug is present typically finishes in far fewer samples than this. + */ +const SAMPLES = Number( + process.env['RPC_COLD_START_SAMPLES'] ?? String(MIN_HELD_SAMPLES_FOR_BUG_GONE), +); +/** + * The gap enforced before every sample, including the first — reproduces + * cold-start-canary.ts's SAMPLE_INTERVAL_MS spacing and its reasoning: + * back-to-back promotions land on some kind of already-warm host resource + * and produce atypically short boots the close does not appear in. Unlike + * the streams face, there is no separate durability wait stacked on top of + * this — auth.service restores no local state, so this is the only spacing + * a sample needs. + */ +const SAMPLE_INTERVAL_MS = Number(process.env['RPC_COLD_START_SAMPLE_INTERVAL_MS'] ?? '60000'); +/** + * How long to read a fresh deployment's boot logs before giving up on + * finding the `listening` line. Matches cold-start-canary.ts's + * LOG_READ_TIMEOUT_MS — manual probing of this same platform behavior has + * observed start->listening as long as 21.9s, so this sits comfortably above + * that. + */ +const LOG_READ_TIMEOUT_MS = Number(process.env['RPC_COLD_START_LOG_READ_TIMEOUT_MS'] ?? '30000'); +/** + * The run's own wall-clock budget — see cold-start-canary.ts's identical + * MAX_RUN_MS for the full reasoning (a job killed by the surrounding CI + * timeout never reaches classifyRpcColdStartRun, so it can't emit the + * inconclusive exit and warning annotation this script is supposed to use + * for a run that can't finish). This canary has no per-sample durability + * wait to budget for, so its worst case (MIN_HELD_SAMPLES_FOR_BUG_GONE + * samples at roughly SAMPLE_INTERVAL_MS plus a boot-and-touch each) fits + * comfortably inside the same 20-minute figure. + */ +const MAX_RUN_MS = Number(process.env['RPC_COLD_START_MAX_RUN_MS'] ?? '1200000'); +/** The HTTP port `auth.service` was deployed on — matches its compute() default (no explicit `port` param). */ +const AUTH_SERVICE_PORT = 3000; + +const token = process.env['PRISMA_SERVICE_TOKEN']; +const stackName = process.env['STACK_NAME']; +if (!token || !stackName) { + console.error('PRISMA_SERVICE_TOKEN and STACK_NAME are required'); + process.exit(1); +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +interface ApiResponse { + readonly status: number; + readonly data: unknown; +} + +/** POSTs/GETs the Management API, returning the status alongside the parsed `data` field — never throws on a non-2xx status. */ +async function apiCall(method: string, path: string, body?: unknown): Promise { + const init: RequestInit = { + method, + headers: { Authorization: `Bearer ${token}`, 'Content-Type': 'application/json' }, + }; + if (body !== undefined) init.body = JSON.stringify(body); + const res = await fetch(`${API}${path}`, init); + const text = await res.text(); + let json: unknown; + try { + json = text ? JSON.parse(text) : undefined; + } catch { + json = text; + } + return { status: res.status, data: isRecord(json) ? json['data'] : json }; +} + +/** Same as apiCall, but throws on a non-2xx status — for calls this script cannot proceed without. */ +async function apiData(method: string, path: string, body?: unknown): Promise { + const res = await apiCall(method, path, body); + if (res.status < 200 || res.status >= 300) { + throw new Error(`${method} ${path} failed: ${res.status} ${JSON.stringify(res.data)}`); + } + return res.data; +} + +function requireString(record: unknown, key: string): string { + if (!isRecord(record) || typeof record[key] !== 'string') { + throw new Error(`expected "${key}" to be a string`); + } + return record[key]; +} + +/** The per-run project shares the stack's name (`prisma-composer deploy --name`). */ +async function findProjectId(): Promise { + const projects = await apiData('GET', '/projects?limit=100'); + const list = Array.isArray(projects) ? projects : []; + const match = list.find((p) => isRecord(p) && p['name'] === stackName); + if (match === undefined) throw new Error(`no project named "${stackName}" — did the deploy run?`); + return requireString(match, 'id'); +} + +/** + * `auth.service`'s app id and its own base URL — the touch target. + * `/v1/apps` is the current Management API surface for what used to be + * `/v1/compute-services` (verified live — see cold-start-canary.ts's + * findApps and gotchas.md's PRO-217 entry). + */ +async function findAuthApp(projectId: string): Promise<{ authAppId: string; authUrl: string }> { + const apps = await apiData('GET', `/apps?projectId=${projectId}&limit=100`); + const list = Array.isArray(apps) ? apps : []; + for (const app of list) { + if (isRecord(app) && app['name'] === 'auth.service') { + return { + authAppId: requireString(app, 'id'), + authUrl: requireString(app, 'appEndpointDomain'), + }; + } + } + throw new Error(`stack "${stackName}" is missing the "auth.service" app`); +} + +/** + * The Deploy step that ran earlier in this job left the content-addressed + * `auth.service` artifact in the runner's temp dir (packageComputeArtifact) + * — reuse it so every promoted deployment is byte-identical to the deployed + * one. Mirrors cold-start-canary.ts's findStreamsArtifact. + */ +function findAuthArtifact(): string { + const dir = `${os.tmpdir()}/prisma-composer-compute-${os.userInfo().uid}`; + const found = execSync(`ls -t ${dir}/*/auth.service.tar.gz 2>/dev/null | head -1`, { + encoding: 'utf8', + }).trim(); + if (!found) throw new Error(`no auth.service.tar.gz under ${dir} — did the deploy build?`); + return found; +} + +/** + * Reads a deployment's boot log from the start, stopping as soon as + * `auth.service`'s own `listening` line has been seen (or + * LOG_READ_TIMEOUT_MS elapses, or the socket errors/closes). Returns the + * concatenated log text collected so far — `findListeningTimestamp` on the + * result may still be undefined if the line was never seen. Identical + * mechanism to cold-start-canary.ts's readDeploymentBootLog. + */ +function readDeploymentBootLog(deploymentId: string): Promise { + return new Promise((resolve) => { + const chunks: string[] = []; + let settled = false; + const ws = new WebSocket( + `wss://api.prisma.io/v1/deployments/${deploymentId}/logs?from_start=true`, + { headers: { Authorization: `Bearer ${token}` } }, + ); + const finish = () => { + if (settled) return; + settled = true; + clearTimeout(timer); + ws.close(); + resolve(chunks.join('')); + }; + const timer = setTimeout(finish, LOG_READ_TIMEOUT_MS); + ws.addEventListener('message', (event) => { + let parsed: unknown; + try { + parsed = JSON.parse(String(event.data)); + } catch { + return; + } + if (isRecord(parsed) && parsed['type'] === 'log' && typeof parsed['text'] === 'string') { + chunks.push(parsed['text']); + if (findListeningTimestamp(chunks.join('')) !== undefined) finish(); + } + }); + ws.addEventListener('error', finish); + ws.addEventListener('close', finish); + }); +} + +/** `err`'s message, plus its `cause` chain if it has one — a thrown fetch rejection's `cause` is where Bun puts the underlying socket error. */ +function errorText(err: unknown): string { + if (!(err instanceof Error)) return String(err); + const causeText = err.cause instanceof Error ? ` (cause: ${err.cause.message})` : ''; + return `${err.message}${causeText}`; +} + +/** + * One fresh `auth.service` deployment, touched once: create -> upload -> + * start -> race the promote call (retrying immediately on the "not running + * yet" 409 — NOT polling for `running` and then promoting, which is what let + * the boot window close in cold-start-canary.ts's original design; see this + * file's module comment) -> fire ONE bare `POST /rpc/verify` the instant + * promote succeeds -> confirm from the deployment's own boot log whether the + * touch actually landed before the app was listening. + */ +async function sampleFreshStart( + authAppId: string, + authUrl: string, + artifactPath: string, + index: number, +): Promise { + const created = await apiData('POST', `/apps/${authAppId}/deployments`, { + portMapping: { http: AUTH_SERVICE_PORT }, + }); + const deploymentId = requireString(created, 'id'); + const uploadUrl = requireString(created, 'uploadUrl'); + const artifact = await Bun.file(artifactPath).arrayBuffer(); + const uploaded = await fetch(uploadUrl, { method: 'PUT', body: artifact }); + if (!uploaded.ok) throw new Error(`artifact upload failed: ${uploaded.status}`); + + await apiData('POST', `/deployments/${deploymentId}/start`); + + const promoteDeadline = Date.now() + 30_000; + for (;;) { + const res = await apiCall('POST', `/apps/${authAppId}/promote`, { deploymentId }); + if (res.status === 200) break; + if (res.status !== 409 || Date.now() > promoteDeadline) { + throw new Error( + `promote never succeeded for deployment ${deploymentId}: ${res.status} ` + + JSON.stringify(res.data), + ); + } + // A short, deliberate courtesy delay — not a "wait for running" poll. + // Each retry is still racing to promote at the earliest legal moment; + // this just keeps a slow boot from hammering the API every few ms. + await sleep(200); + } + + const touchSentAt = new Date(); + const started = Date.now(); + let status = 0; + let body = ''; + let wasThrown = false; + try { + const res = await fetch(`${authUrl}/rpc/verify`, { + method: 'POST', + headers: { 'content-type': 'application/json', 'Idempotency-Key': crypto.randomUUID() }, + body: JSON.stringify({ token: `rpc-cold-start-canary-${index}` }), + signal: AbortSignal.timeout(60_000), + }); + status = res.status; + body = await res.text(); + } catch (err) { + wasThrown = true; + body = errorText(err); + } + const latencyMs = Date.now() - started; + + const logText = await readDeploymentBootLog(deploymentId); + const listeningAt = findListeningTimestamp(logText); + const bootEvidence = classifyBootEvidence(touchSentAt, listeningAt); + const evidence = + listeningAt !== undefined + ? `logs: listening ${listeningAt.toISOString()}, touch sent ${touchSentAt.toISOString()} (${bootEvidence})` + : `no listening line read within ${LOG_READ_TIMEOUT_MS}ms — boot evidence unknown, not guessed`; + + const touch = classifyRpcColdStartTouch(status, body, wasThrown, bootEvidence); + const statusLabel = wasThrown ? 'thrown' : String(status); + console.log( + ` sample #${index}: ${touch} (${statusLabel}, ${latencyMs}ms) [${evidence}] — ${body.slice(0, 160)}`, + ); + return touch; +} + +const projectId = await findProjectId(); +const { authAppId, authUrl } = await findAuthApp(projectId); +const artifactPath = findAuthArtifact(); +console.log(`Stack "${stackName}" (${projectId}); auth.service at ${authUrl}`); +console.log(`Sampling ${SAMPLES} fresh auth.service instances, ${SAMPLE_INTERVAL_MS}ms apart…`); + +const runStartedAt = Date.now(); +const touches: RpcColdStartTouch[] = []; +for (let i = 0; i < SAMPLES; i++) { + if (Date.now() - runStartedAt > MAX_RUN_MS) { + console.log( + ` stopping after ${i} sample(s): the run's own ${MAX_RUN_MS}ms wall-clock budget is used ` + + 'up — reporting the touches collected so far rather than risking a CI timeout kill.', + ); + break; + } + console.log(` waiting ${SAMPLE_INTERVAL_MS}ms before sample #${i}…`); + await sleep(SAMPLE_INTERVAL_MS); + const touch = await sampleFreshStart(authAppId, authUrl, artifactPath, i); + touches.push(touch); + // A close is decisive on its own (classifyRpcColdStartRun's rule) — the + // verdict is already bug-present, and running the rest of the budget only + // spends CI minutes without changing the answer. + if (touch === 'closed') { + console.log( + ` close observed on sample #${i}; bug-present is already decided — skipping the ` + + `remaining ${SAMPLES - i - 1} samples.`, + ); + break; + } +} + +const result = classifyRpcColdStartRun(touches); +console.log(result.message); +if (result.verdict === 'inconclusive') { + // A GitHub Actions warning annotation: loud on the run page without + // failing a required check over a deploy flake. Newlines must be %0A. + const detail = touches.map((touch, i) => `sample #${i}: ${touch}`).join('; '); + console.log( + `::warning title=RPC cold-start canary (PRO-217) inconclusive::${result.message} [${detail}]`, + ); +} +process.exitCode = result.verdict === 'bug-gone' ? 1 : 0; From a46abfa5099906dddc1fbb758309934f91fea67e Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 18:08:51 +0200 Subject: [PATCH 11/18] =?UTF-8?q?Revert=20"the=20rpc=20cold-start=20canary?= =?UTF-8?q?=20(PRO-217)"=20=E2=80=94=20the=20canary=20is=20not=20warranted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An RPC cold-start canary was dispatched but, on the evidence, dropped: 1. It cannot certify a cold start against examples/storefront-auth. The auth service restores no state, so it boots in under ~1.5s — narrower than the 2s clock-skew margin the coldness proof needs. The live run forced 14 real races (touch arrived 376-1185ms before listening, every time) yet certified none, correctly refusing to shrink the margin to fit. 2. More fundamentally, a cold-start canary exists to signal when a platform workaround can be deleted. The RPC idempotency retry is permanent protocol semantics (ADR-0037), not a workaround with an expiry — there is nothing for it to signal the removal of. And PRO-217 is a platform ingress bug, already watched by the streams cold-start canary, which detects it well precisely because streams has the long boot window RPC lacks. The slice ships the keyed protocol; the streams canary remains the platform-wide PRO-217 signal. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .github/workflows/e2e-deploy.yml | 35 -- .../modules/auth/src/server.ts | 13 - .../rpc-cold-start-canary-classify.test.ts | 236 ---------- scripts/rpc-cold-start-canary-classify.ts | 331 -------------- scripts/rpc-cold-start-canary.ts | 420 ------------------ 5 files changed, 1035 deletions(-) delete mode 100644 scripts/rpc-cold-start-canary-classify.test.ts delete mode 100644 scripts/rpc-cold-start-canary-classify.ts delete mode 100644 scripts/rpc-cold-start-canary.ts diff --git a/.github/workflows/e2e-deploy.yml b/.github/workflows/e2e-deploy.yml index 60cec687..37447fc8 100644 --- a/.github/workflows/e2e-deploy.yml +++ b/.github/workflows/e2e-deploy.yml @@ -133,38 +133,3 @@ jobs: verify-command: bun "${{ github.workspace }}/scripts/cold-start-canary.ts" destroy-label: 'streams-canary ' sweep-prefixes: storefront-auth pn-widgets hello canary streams-canary - - rpc-cold-start-canary: - name: RPC cold-start canary (PRO-217) - runs-on: ubuntu-latest - # After the streams face's canary, for the same quota-serialization reason - # as the rest of this chain — only one real-cloud deploy round runs at a - # time. Fails when the ingress no longer closes first-touch connections to - # a booting service-RPC provider — the signal to retire this canary (never - # the Idempotency-Key protocol or the bounded retry in service-rpc, which - # are permanent protocol semantics for this kind, not a PRO-217 - # compensation). The bug being PRESENT is today's normal and passes. - # - # This face has no warmup phase and no durability wait (unlike the streams - # canary): the script itself is the caller, and auth.service keeps no - # local state to restore between samples, so its own budget is at most as - # large as cold-start-canary.ts's. NOT required until Will adds it. - needs: cold-start-canary - timeout-minutes: 30 - if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository - env: - PRISMA_SERVICE_TOKEN: ${{ secrets.PRISMA_SERVICE_TOKEN }} - PRISMA_WORKSPACE_ID: ${{ vars.PRISMA_WORKSPACE_ID }} - AUTH_SIGNING_SECRET: sk_test_ci_storefront_auth - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - - uses: ./.github/actions/deploy-verify-destroy - with: - working-directory: examples/storefront-auth - build-filter: '@prisma/example-storefront-auth...' - stack-name: storefront-auth-rpc-canary-ci-${{ github.run_id }} - verify-command: bun "${{ github.workspace }}/scripts/rpc-cold-start-canary.ts" - destroy-label: 'storefront-auth-rpc-canary ' - sweep-prefixes: storefront-auth pn-widgets hello canary streams-canary storefront-auth-rpc-canary diff --git a/examples/storefront-auth/modules/auth/src/server.ts b/examples/storefront-auth/modules/auth/src/server.ts index 9ec38da2..9783043e 100644 --- a/examples/storefront-auth/modules/auth/src/server.ts +++ b/examples/storefront-auth/modules/auth/src/server.ts @@ -56,16 +56,3 @@ export default handler; // Bind all interfaces — Compute routes external HTTP to the VM, so a // loopback-only listener would be unreachable. Bun.serve({ port, hostname: '0.0.0.0', fetch: handler }); -// The RPC cold-start canary (scripts/rpc-cold-start-canary.ts) reads this -// line out of the deployment's boot log to prove a touch was sent before the -// server could answer anything — the same technique -// scripts/cold-start-canary.ts already relies on for the streams module. -// Bun.serve binds synchronously, so logging immediately after it returns -// marks the moment this service is actually ready to accept connections. -// The timestamp is stamped here, in-process, rather than left to the -// platform: unlike @prisma/streams-server (which patches console.log to -// prepend one), Compute's log relay passes plain app stdout through -// unmodified — verified live, an unstamped line arrives with no timestamp -// at all — so a line with no timestamp of its own would be unusable -// evidence for classifyBootEvidence's cross-clock comparison. -console.log(`[${new Date().toISOString()}] [INFO] auth server listening on 0.0.0.0:${port}`); diff --git a/scripts/rpc-cold-start-canary-classify.test.ts b/scripts/rpc-cold-start-canary-classify.test.ts deleted file mode 100644 index e1cb2475..00000000 --- a/scripts/rpc-cold-start-canary-classify.test.ts +++ /dev/null @@ -1,236 +0,0 @@ -import assert from 'node:assert/strict'; -import { describe, it } from 'node:test'; - -import { - CLOCK_SKEW_MARGIN_MS, - classifyBootEvidence, - classifyRpcColdStartRun, - classifyRpcColdStartTouch, - findListeningTimestamp, - MAX_FALSE_CLEAN_PROBABILITY, - MIN_HELD_SAMPLES_FOR_BUG_GONE, - type RpcColdStartTouch, - stripAnsiCodes, - TARGET_CLOSE_RATE, -} from './rpc-cold-start-canary-classify.ts'; - -describe('classifyRpcColdStartTouch', () => { - it('a 200 confirmed cold → held (the edge carried the request through a real boot)', () => { - assert.equal(classifyRpcColdStartTouch(200, '{"ok":true}', false, 'confirmed-cold'), 'held'); - }); - - it("the canary's own known 401 (missing service key), confirmed cold → held — the ingress still carried it through", () => { - const body = '{"error":"Unauthorized: missing or invalid service key"}'; - assert.equal(classifyRpcColdStartTouch(401, body, false, 'confirmed-cold'), 'held'); - }); - - it('a 200 confirmed warm → no-cold-start (it proves nothing about PRO-217)', () => { - assert.equal( - classifyRpcColdStartTouch(200, '{"ok":true}', false, 'confirmed-warm'), - 'no-cold-start', - ); - }); - - it('the known 401, confirmed warm → no-cold-start', () => { - const body = '{"error":"Unauthorized: missing or invalid service key"}'; - assert.equal(classifyRpcColdStartTouch(401, body, false, 'confirmed-warm'), 'no-cold-start'); - }); - - it('a 200 with unknown boot evidence → other (log read failed or too close to call — not a guess)', () => { - assert.equal(classifyRpcColdStartTouch(200, '{"ok":true}', false, 'unknown'), 'other'); - }); - - it('a thrown fetch error naming the close → closed (the PRO-217 signal), regardless of boot evidence', () => { - const message = 'fetch failed: The socket connection was closed unexpectedly'; - assert.equal(classifyRpcColdStartTouch(0, message, true, 'confirmed-cold'), 'closed'); - assert.equal(classifyRpcColdStartTouch(0, message, true, 'unknown'), 'closed'); - }); - - it('reset/refused faces of the same close → closed, whether thrown or answered', () => { - for (const body of ['ECONNRESET while fetching', 'connect ECONNREFUSED', 'socket hang up']) { - assert.equal(classifyRpcColdStartTouch(0, body, true, 'unknown'), 'closed', body); - assert.equal(classifyRpcColdStartTouch(502, body, false, 'unknown'), 'closed', body); - } - }); - - it('an unrelated 401 (a real caller bug, not the missing-key message) → other, not held', () => { - const body = '{"error":"Unauthorized: something else entirely"}'; - assert.equal(classifyRpcColdStartTouch(401, body, false, 'confirmed-cold'), 'other'); - }); - - it('a 500 → other, regardless of boot evidence — a real app bug must not count as proof the platform is healthy', () => { - assert.equal( - classifyRpcColdStartTouch(500, '{"error":"Internal server error"}', false, 'confirmed-cold'), - 'other', - ); - }); - - it('any other unexpected status → other', () => { - assert.equal(classifyRpcColdStartTouch(404, 'not found', false, 'confirmed-cold'), 'other'); - assert.equal(classifyRpcColdStartTouch(413, 'too large', false, 'unknown'), 'other'); - }); - - it('a thrown error that does not name a close → other, not closed', () => { - assert.equal( - classifyRpcColdStartTouch(0, 'TimeoutError: signal timed out', true, 'unknown'), - 'other', - ); - }); -}); - -describe('stripAnsiCodes', () => { - it('removes SGR escape sequences from spark boot log lines', () => { - const colorized = `${String.fromCharCode(27)}[90m[${String.fromCharCode(27)}[0m2026-07-20T14:45:51Z ${String.fromCharCode(27)}[32mINFO ${String.fromCharCode(27)}[0m spark::app_source${String.fromCharCode(27)}[90m]${String.fromCharCode(27)}[0m compute.manifest.json not found`; - assert.equal( - stripAnsiCodes(colorized), - '[2026-07-20T14:45:51Z INFO spark::app_source] compute.manifest.json not found', - ); - }); - - it('leaves plain text untouched', () => { - assert.equal(stripAnsiCodes('[INFO] plain line, no escapes'), '[INFO] plain line, no escapes'); - }); -}); - -describe('findListeningTimestamp', () => { - it("reads auth's own listening line", () => { - const log = - 'spark: starting bun with entrypoint: bootstrap.js\r\n' + - '[2026-07-20T14:45:51.926Z] [INFO] auth server listening on 0.0.0.0:3000\r\n'; - const found = findListeningTimestamp(log); - assert.ok(found); - assert.equal(found?.toISOString(), '2026-07-20T14:45:51.926Z'); - }); - - it('returns undefined when the log never reached a listening line (e.g. read cut off mid-boot)', () => { - const log = - 'spark: starting bun with entrypoint: bootstrap.js\r\n' + - 'spark: time-sync maintenance child started\r\n'; - assert.equal(findListeningTimestamp(log), undefined); - }); - - it('returns undefined for an empty or unrelated log', () => { - assert.equal(findListeningTimestamp(''), undefined); - assert.equal(findListeningTimestamp('some other server started fine'), undefined); - }); - - it("does not match a differently-named service (e.g. the streams face's own listening line)", () => { - const log = - '[2026-07-20T14:45:51.926Z] [INFO] prisma-streams server listening on 0.0.0.0:3000\r\n'; - assert.equal(findListeningTimestamp(log), undefined); - }); -}); - -describe('classifyBootEvidence (margin-aware, cross-clock comparison)', () => { - const listeningAt = new Date('2026-07-20T14:45:51.926Z'); - - it('touch sent comfortably before listening (beyond the skew margin) → confirmed-cold', () => { - const touchSentAt = new Date(listeningAt.getTime() - CLOCK_SKEW_MARGIN_MS - 1); - assert.equal(classifyBootEvidence(touchSentAt, listeningAt), 'confirmed-cold'); - }); - - it('touch sent exactly at the margin boundary before listening → confirmed-cold (>=)', () => { - const touchSentAt = new Date(listeningAt.getTime() - CLOCK_SKEW_MARGIN_MS); - assert.equal(classifyBootEvidence(touchSentAt, listeningAt), 'confirmed-cold'); - }); - - it('touch sent comfortably after listening (beyond the skew margin) → confirmed-warm', () => { - const touchSentAt = new Date(listeningAt.getTime() + CLOCK_SKEW_MARGIN_MS + 1); - assert.equal(classifyBootEvidence(touchSentAt, listeningAt), 'confirmed-warm'); - }); - - it('touch sent within the skew margin on either side of listening → unknown (could be skew, not order)', () => { - const justBefore = new Date(listeningAt.getTime() - CLOCK_SKEW_MARGIN_MS + 1); - const justAfter = new Date(listeningAt.getTime() + CLOCK_SKEW_MARGIN_MS - 1); - assert.equal(classifyBootEvidence(justBefore, listeningAt), 'unknown'); - assert.equal(classifyBootEvidence(justAfter, listeningAt), 'unknown'); - }); - - it('no listening timestamp at all → unknown, not a guess', () => { - assert.equal(classifyBootEvidence(new Date(), undefined), 'unknown'); - }); -}); - -describe('the sample-budget arithmetic', () => { - it('MIN_HELD_SAMPLES_FOR_BUG_GONE is the smallest N keeping an all-held run at or under 5% chance at a 20% close rate', () => { - assert.equal(TARGET_CLOSE_RATE, 0.2); - assert.equal(MAX_FALSE_CLEAN_PROBABILITY, 0.05); - assert.equal(MIN_HELD_SAMPLES_FOR_BUG_GONE, 14); - const chance = (n: number) => (1 - TARGET_CLOSE_RATE) ** n; - assert.ok(chance(MIN_HELD_SAMPLES_FOR_BUG_GONE) <= MAX_FALSE_CLEAN_PROBABILITY); - assert.ok(chance(MIN_HELD_SAMPLES_FOR_BUG_GONE - 1) > MAX_FALSE_CLEAN_PROBABILITY); - }); -}); - -describe('classifyRpcColdStartRun (the three-exit mapping of a REQUIRED check)', () => { - const run = (...touches: RpcColdStartTouch[]) => classifyRpcColdStartRun(touches); - const heldTimes = (n: number): RpcColdStartTouch[] => Array.from({ length: n }, () => 'held'); - - it('no touches → inconclusive (broken canary; warn, do not block)', () => { - assert.equal(run().verdict, 'inconclusive'); - }); - - it("one close among holds → bug-present (exit 0; today's normal)", () => { - const result = run('held', 'closed', 'held', 'held'); - assert.equal(result.verdict, 'bug-present'); - assert.match(result.message, /1\/4 first touches closed/); - assert.match(result.message, /PRO-217 not fixed/); - assert.match(result.message, /not a compensation/); - }); - - it('a close is decisive even alongside touches that never went cold', () => { - const result = run('closed', 'no-cold-start', 'no-cold-start', 'no-cold-start'); - assert.equal(result.verdict, 'bug-present'); - }); - - it('a close is decisive even in a run large enough to otherwise reach the bug-gone budget', () => { - const result = classifyRpcColdStartRun([...heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE), 'closed']); - assert.equal(result.verdict, 'bug-present'); - }); - - it('all held but fewer than MIN_HELD_SAMPLES_FOR_BUG_GONE → inconclusive, not bug-gone (an all-held run this small is the expected outcome of an intermittent bug)', () => { - const result = classifyRpcColdStartRun(heldTimes(4)); - assert.equal(result.verdict, 'inconclusive'); - assert.match(result.message, /All 4 confirmed cold-start touches held/); - assert.match(result.message, /41\.0%/); // 0.8^4 - assert.match(result.message, new RegExp(String(MIN_HELD_SAMPLES_FOR_BUG_GONE))); - assert.match(result.message, /not blocking/i); - }); - - it('all held at exactly MIN_HELD_SAMPLES_FOR_BUG_GONE → bug-gone (exit 1 — the forcing signal), actionable for a cold reader', () => { - const result = classifyRpcColdStartRun(heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE)); - assert.equal(result.verdict, 'bug-gone'); - assert.match(result.message, /4\.4%/); // 0.8^14 - assert.match(result.message, /not because of your change/); - assert.match(result.message, /rpc-cold-start-canary\.ts/); - assert.match(result.message, /e2e-deploy\.yml/); - assert.match(result.message, /gotchas\.md/); - assert.match(result.message, /do NOT remove the Idempotency-Key protocol/); - }); - - it('one held short of the budget → inconclusive, not bug-gone', () => { - const result = classifyRpcColdStartRun(heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE - 1)); - assert.equal(result.verdict, 'inconclusive'); - }); - - it('any touch that never went cold makes the whole run inconclusive, even with no closes and plenty of holds', () => { - const result = classifyRpcColdStartRun([ - ...heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE), - 'no-cold-start', - ]); - assert.equal(result.verdict, 'inconclusive'); - assert.match(result.message, /failed to force a cold start/); - assert.match(result.message, /not blocking/); - }); - - it('all touches never going cold → inconclusive, not a clean bill of health', () => { - const result = run('no-cold-start', 'no-cold-start', 'no-cold-start', 'no-cold-start'); - assert.equal(result.verdict, 'inconclusive'); - assert.match(result.message, /4\/4 touches/); - }); - - it('an "other" (broken/ambiguous) touch also blocks a bug-gone verdict', () => { - const result = classifyRpcColdStartRun([...heldTimes(MIN_HELD_SAMPLES_FOR_BUG_GONE), 'other']); - assert.equal(result.verdict, 'inconclusive'); - }); -}); diff --git a/scripts/rpc-cold-start-canary-classify.ts b/scripts/rpc-cold-start-canary-classify.ts deleted file mode 100644 index 7ccd10e8..00000000 --- a/scripts/rpc-cold-start-canary-classify.ts +++ /dev/null @@ -1,331 +0,0 @@ -/** - * Pass/fail logic for rpc-cold-start-canary.ts (PRO-217, service-RPC face) — - * the service-rpc sibling of cold-start-canary-classify.ts, same intermittent- - * bug arithmetic applied to a direct `POST /rpc/` touch instead - * of the streams module's `POST /jobs`. - * - * A touch against a freshly promoted `auth.service` instance lands on one of - * four outcomes: - * - * - the connection itself is reset mid-establishment (a thrown fetch error, - * or a response naming the close) — the bug reproduced. A close only - * happens during the boot window, so it alone proves the touch reached a - * cold start; no further evidence is needed. - * - the target answered (any real HTTP response, not a connection error) and - * the deployment's own boot log confirms the touch was sent before the - * server finished booting — the edge held the connection through a real - * cold start. Genuine evidence toward "fixed". - * - the target answered before there was anything left to boot through — no - * cold start happened, so the touch says nothing about the bug either way. - * - the log evidence can't place the touch on either side of the boot (read - * cut off, or within the clock-skew margin) — inconclusive, not guessed. - * - * "The target answered" deliberately covers more than a successful RPC - * result. This canary cannot supply the real per-edge Authorization bearer - * key ADR-0030/0031 mints for the storefront -> auth wiring: the Management - * API's own contract states plainly that an environment variable's value "is - * stored encrypted and is not returned by subsequent reads" — verified live - * against a real deploy, where an unauthenticated touch got back exactly - * `401 {"error":"Unauthorized: missing or invalid service key"}` from a warm - * instance. That 401 is `serve()`'s own documented rejection - * (packages/0-framework/2-authoring/service-rpc/src/serve.ts) for a request - * that never had a chance to supply a key this script cannot obtain — not a - * sign PRO-217 fired. The property this canary needs from a "held" touch is - * "the ingress carried the request through to the application", and a 401 - * proves exactly that as cleanly as a 2xx would: `serve()`'s accepted-key - * check runs after the connection is already established, so reaching it at - * all means the ingress did not reset the connection. Observed 401 latencies - * during this canary's build (983-1102ms) track normal boot-racing latency, - * not the ~400ms fast-fail PRO-217's close produces — further evidence this - * is the auth check running on an already-live connection, not the close. - * Any OTHER status or body is left `other` rather than folded in: a genuine - * application bug must not silently count as proof the platform is healthy. - */ - -/** - * A raw, unauthenticated single-attempt `fetch` hitting the RPC endpoint - * directly surfaces PRO-217 as a thrown error (Bun's `fetch` reports "The - * socket connection was closed unexpectedly" for a reset), not as a 502 — - * that shape only came from the streams canary's intermediary `jobs` caller - * catching the error itself and wrapping it into a response body. Both - * shapes are checked here for the same fragments regardless, since it costs - * nothing and guards against the platform's exact behavior differing across - * targets. Keep in sync with gotchas.md's PRO-217 entry. - */ -const CLOSE_FRAGMENTS = [ - 'socket connection was closed', - 'econnreset', - 'econnrefused', - 'socket hang up', -]; - -/** One first-touch outcome against a freshly promoted `auth.service` instance. */ -export type RpcColdStartTouch = 'held' | 'closed' | 'no-cold-start' | 'other'; - -/** - * The caller's answer to "did this touch actually race a boot?", decided by - * `classifyBootEvidence` from the deployment's own logs before - * `classifyRpcColdStartTouch` is called. Identical three-way split to - * cold-start-canary-classify.ts's `BootEvidence` — see that file for the - * full reasoning; duplicated here (not imported) so this canary and its - * gotchas paragraph can be deleted independently of the streams one. - */ -export type BootEvidence = 'confirmed-cold' | 'confirmed-warm' | 'unknown'; - -/** - * `auth`'s own documented rejection for a request that never presented (or - * presented the wrong) per-edge service key — see `serve()`'s accepted-keys - * check. Matched verbatim so a coincidental, unrelated 401 does not get - * folded into "the target answered". - */ -const UNAUTHORIZED_MISSING_KEY_MESSAGE = 'Unauthorized: missing or invalid service key'; - -/** - * Whether `status`/`body` is a genuine application response — proof the - * ingress carried the connection through to `serve()` — as opposed to a - * connection-level failure or an unrelated error this canary should not - * interpret either way. A real RPC success (2xx) counts; so does the - * specific, known 401 this canary's own missing service key always produces - * (see the module comment). Nothing else does — a 500 or an unexpected 4xx - * is left uninterpreted rather than assumed harmless. - */ -function targetAnswered(status: number, body: string): boolean { - if (status >= 200 && status < 300) return true; - return status === 401 && body.includes(UNAUTHORIZED_MISSING_KEY_MESSAGE); -} - -/** - * Classifies one first-touch outcome from the CALLER's seat. `body` is - * either a response body (fetch resolved) or the stringified error a thrown - * fetch rejection produced (connection-level failure) — the caller passes - * whichever it got, and `wasThrown` says which. - * - * A close is decisive regardless of `bootEvidence`: it only happens - * mid-boot, so it is its own proof. A response that reaches `serve()` - * (`targetAnswered`) only becomes `held` when `bootEvidence` is - * `confirmed-cold`; `confirmed-warm` makes it `no-cold-start` (the touch - * proves nothing, because nothing was booting when it landed), and `unknown` - * makes it `other`. A thrown error that isn't a close, or a response that - * isn't `targetAnswered`, is `other` either way. - */ -export function classifyRpcColdStartTouch( - status: number, - body: string, - wasThrown: boolean, - bootEvidence: BootEvidence, -): RpcColdStartTouch { - const lower = body.toLowerCase(); - if (CLOSE_FRAGMENTS.some((fragment) => lower.includes(fragment))) { - return 'closed'; - } - if (!wasThrown && targetAnswered(status, body)) { - if (bootEvidence === 'confirmed-cold') return 'held'; - if (bootEvidence === 'confirmed-warm') return 'no-cold-start'; - return 'other'; - } - return 'other'; -} - -/** - * Strips ANSI SGR color codes from spark's boot log lines. The `auth` - * service's own listening line (added for this canary — see - * examples/storefront-auth/modules/auth/src/server.ts) is not colorized, - * but spark's surrounding platform lines are, and stripping first keeps the - * timestamp regex robust regardless of what shares the log stream. Built - * from String.fromCharCode rather than a regex literal containing the raw - * ESC byte, which Biome's noControlCharactersInRegex rule (rightly) rejects. - */ -export function stripAnsiCodes(text: string): string { - const ESC = String.fromCharCode(27); - return text.split(new RegExp(`${ESC}\\[[0-9;]*m`, 'g')).join(''); -} - -/** - * `auth`'s own boot line — e.g. "[2026-07-20T14:45:51.926Z] [INFO] auth - * server listening on 0.0.0.0:3000" — read from a deployment's log history - * (`?from_start=true`). Compute's log relay passes plain app stdout through - * unmodified (verified live: an unstamped line arrives with no timestamp of - * its own), so the timestamp is the app's own — stamped in server.ts rather - * than left to the platform, unlike @prisma/streams-server's console.log - * patch. Returns the timestamp it logged, or undefined if the boot never - * reached it (or the log read didn't cover it). - */ -export function findListeningTimestamp(logText: string): Date | undefined { - const match = stripAnsiCodes(logText).match( - /\[(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z)]\s*\[INFO]\s*auth server listening/, - ); - const timestamp = match?.[1]; - return timestamp !== undefined ? new Date(timestamp) : undefined; -} - -/** - * `touchSentAt` is `new Date()` on the CI runner; `listeningAt` is parsed out - * of a timestamp `auth`'s own server.ts wrote with its own `new Date()` on a - * Prisma Compute VM. Nothing keeps those two clocks in lockstep, so a - * touch/listening gap of a few tens or hundreds of milliseconds is not by - * itself proof of ordering — it could be clock skew rather than a genuine - * race. Same margin and reasoning as cold-start-canary-classify.ts's - * CLOCK_SKEW_MARGIN_MS: two well-behaved NTP-synced hosts are not expected - * to disagree by more than a few hundred milliseconds, and 2 seconds is a - * comfortable multiple of that while staying small next to the 3-22s boot - * windows this family of canaries samples. - */ -export const CLOCK_SKEW_MARGIN_MS = 2_000; - -/** - * Decides, from the deployment's own boot log, which side of the boot a - * touch landed on. Returns `unknown` — not a guess — when the log never - * showed a `listening` line, or when the touch and `listening` are within - * CLOCK_SKEW_MARGIN_MS of each other and cross-clock skew could plausibly - * explain the gap either way. - */ -export function classifyBootEvidence( - touchSentAt: Date, - listeningAt: Date | undefined, -): BootEvidence { - if (listeningAt === undefined) return 'unknown'; - const touchBeforeListeningByMs = listeningAt.getTime() - touchSentAt.getTime(); - if (touchBeforeListeningByMs >= CLOCK_SKEW_MARGIN_MS) return 'confirmed-cold'; - if (touchBeforeListeningByMs <= -CLOCK_SKEW_MARGIN_MS) return 'confirmed-warm'; - return 'unknown'; -} - -/** - * The three exits a REQUIRED check needs (the job fails only on the - * conclusive forcing signal): - * - `bug-present` → exit 0 (a close occurred; today's normal), - * - `bug-gone` → exit 1 (enough touches reached a genuinely fresh, booting - * instance and every one of them held that an all-held result is strong - * evidence, not luck — the actionable removal message is the point of the - * failure), - * - `inconclusive` → exit 0 plus a CI warning annotation (loud, not blocking - * every PR on a deploy flake, a run that never managed to force a cold - * start, or a run too small to trust; a human should look). - */ -export type RpcColdStartVerdict = 'bug-present' | 'bug-gone' | 'inconclusive'; - -export interface RpcColdStartResult { - readonly verdict: RpcColdStartVerdict; - readonly message: string; -} - -/** - * Same target close rate as cold-start-canary-classify.ts's - * TARGET_CLOSE_RATE, and the same reasoning: deliberately conservative - * relative to the 60-100% close rates manual reproduction has actually - * observed for this family of bug, so the sample budget below stays - * trustworthy even if the RPC face's real defect rate is lower than the - * streams face's. - */ -export const TARGET_CLOSE_RATE = 0.2; - -/** - * The most a bug-gone verdict is allowed to be "all held by luck": if the - * true close rate were TARGET_CLOSE_RATE and the bug were still present, the - * chance of seeing every one of MIN_HELD_SAMPLES_FOR_BUG_GONE independent - * confirmed cold starts hold is at most this. - */ -export const MAX_FALSE_CLEAN_PROBABILITY = 0.05; - -/** - * The number of confirmed cold-start holds classifyRpcColdStartRun requires - * before it will say bug-gone. Identical arithmetic to - * cold-start-canary-classify.ts's MIN_HELD_SAMPLES_FOR_BUG_GONE: - * - * 0.8^13 ≈ 5.50% (not low enough) - * 0.8^14 ≈ 4.40% (first N at or below 5%) - * - * so N = 14. - */ -export const MIN_HELD_SAMPLES_FOR_BUG_GONE = Math.ceil( - Math.log(MAX_FALSE_CLEAN_PROBABILITY) / Math.log(1 - TARGET_CLOSE_RATE), -); - -function chanceAllHoldByLuck(heldCount: number): number { - return (1 - TARGET_CLOSE_RATE) ** heldCount; -} - -function asPercent(probability: number): string { - return `${(probability * 100).toFixed(1)}%`; -} - -/** - * Aggregates N first touches. A close anywhere is decisive on its own (rule: - * a close only happens mid-boot, so it needs no corroboration). Short of - * that, a touch that landed on an already-warm instance (`no-cold-start`) or - * came back some other inconclusive way (`other`) means the run never earned - * an opinion from that touch, so ANY of those makes the whole run - * `inconclusive` rather than mixing an uninformative touch into a "clean" - * verdict. And even a run where every touch held is only allowed to say - * "fixed" once it has collected MIN_HELD_SAMPLES_FOR_BUG_GONE confirmed - * cold-start holds — see that constant's comment for why a smaller all-held - * run is not evidence. - */ -export function classifyRpcColdStartRun(touches: readonly RpcColdStartTouch[]): RpcColdStartResult { - const n = touches.length; - if (n === 0) return { verdict: 'inconclusive', message: 'Canary made no touches — broken.' }; - const count = (t: RpcColdStartTouch) => touches.filter((x) => x === t).length; - const closed = count('closed'); - const held = count('held'); - const noColdStart = count('no-cold-start'); - const other = count('other'); - - if (closed > 0) { - return { - verdict: 'bug-present', - message: - `Cold-start close still present on the service-RPC edge (${closed}/${n} first touches ` + - `closed, ${held} held, ${noColdStart} never went cold) — PRO-217 not fixed; keep the ` + - 'bounded retry over keyed calls in service-rpc (client.ts/serve.ts) — that is permanent ' + - 'protocol semantics for this kind, not a compensation for this bug.', - }; - } - - if (noColdStart > 0 || other > 0) { - return { - verdict: 'inconclusive', - message: - `The canary failed to force a cold start on ${noColdStart + other}/${n} touches ` + - `(${noColdStart} landed on an already-warm instance, ${other} were otherwise ` + - 'inconclusive) — a run that never reaches a cold instance has no opinion to report on ' + - 'PRO-217. A human should look; not blocking.', - }; - } - - // Every touch reached a fresh, booting instance and held (noColdStart === 0, - // other === 0, closed === 0), so held === n here. Whether that is enough - // still depends on how many holds it actually is. - if (held < MIN_HELD_SAMPLES_FOR_BUG_GONE) { - return { - verdict: 'inconclusive', - message: - `All ${held} confirmed cold-start touches held, but PRO-217 is intermittent, so that is ` + - 'the outcome a too-small sample is expected to produce even with the bug fully present: ' + - 'even at a conservative 20% close rate (well below the 60-100% close rates seen in manual ' + - `reproduction against the streams face of this bug), the chance that ${held} independent ` + - `cold starts would all happen to hold is ${asPercent(chanceAllHoldByLuck(held))}. This run ` + - `needs at least ${MIN_HELD_SAMPLES_FOR_BUG_GONE} confirmed cold-start holds before an ` + - `all-held result drops that chance to ${asPercent(MAX_FALSE_CLEAN_PROBABILITY)} or below ` + - `(0.8^${MIN_HELD_SAMPLES_FOR_BUG_GONE} ≈ ` + - `${asPercent(chanceAllHoldByLuck(MIN_HELD_SAMPLES_FOR_BUG_GONE))}). Not blocking.`, - }; - } - - return { - verdict: 'bug-gone', - message: - `All ${n} first touches against genuinely fresh, still-booting instances were held to ` + - `success — ${held} confirmed cold-start holds with zero closes. Even at a conservative 20% ` + - 'close rate (well below the 60-100% close rates seen in manual reproduction against the ' + - 'streams face of this bug), the chance of that happening by luck alone is only ' + - `${asPercent(chanceAllHoldByLuck(held))}, so this counts as real evidence: the platform no ` + - "longer resets a service-RPC edge's first-touch connection during a cold start. To fix this " + - 'build (you are seeing it because the cleanup is now due, not because of your change): ' + - '1) remove scripts/rpc-cold-start-canary.ts, scripts/rpc-cold-start-canary-classify.ts (+ ' + - 'its test) and the "RPC cold-start canary (PRO-217)" job in .github/workflows/e2e-deploy.yml; ' + - "2) drop the service-RPC paragraph from gotchas.md's PRO-217 entry; 3) do NOT remove the " + - "Idempotency-Key protocol or the bounded retry in service-rpc's client.ts/serve.ts — those " + - 'are permanent protocol semantics for this kind (safe retries on every call), not a PRO-217 ' + - 'compensation, and stay regardless of this verdict.', - }; -} diff --git a/scripts/rpc-cold-start-canary.ts b/scripts/rpc-cold-start-canary.ts deleted file mode 100644 index d62fb636..00000000 --- a/scripts/rpc-cold-start-canary.ts +++ /dev/null @@ -1,420 +0,0 @@ -#!/usr/bin/env bun -/** - * Canary for PRO-217 (the Compute ingress closing a first-touch connection - * while a scale-to-zero service boots) — the service-RPC sibling of - * cold-start-canary.ts, run as the VERIFY step of a deploy-verify-destroy - * round over examples/storefront-auth (the deploy and teardown are the - * action's; this script only samples). - * - * Shape: this script IS the caller — a bare, single-attempt `fetch` straight - * at `auth.service`'s own `POST /rpc/verify` endpoint, carrying a manually - * minted `Idempotency-Key` header. It deliberately does NOT go through - * `makeClient` (`@prisma/composer/service-rpc`) or any framework client: - * this slice just gave every framework RPC edge a bounded, automatic retry - * over the same idempotency key (packages/0-framework/2-authoring/service-rpc), - * so a probe built on that client would have PRO-217's raw first-touch - * behavior masked by the very retry this slice ships — the platform's actual - * behavior has to stay observable, not smoothed over by the client under - * test. Each sample forces a genuinely fresh `auth.service` instance (create - * a deployment, upload the artifact already built by this job's Deploy step, - * start it, promote it to the app's stable endpoint), fires ONE first-touch - * `POST /rpc/verify` the instant the promote call succeeds, then reads the - * deployment's own boot logs to confirm the touch actually raced the boot — - * not just that a fresh instance existed somewhere. - * - * This inherits cold-start-canary.ts's contract wholesale — the same 2026- - * 07-17 rebuild that fixed two ways the original streams canary reported - * "fixed" while PRO-217 was live: it never actually hit a cold start (it - * waited for `running` before touching, which flips ~1s after `start`, long - * before the app is listening), and its verdict rule treated "every touch - * happened to hold" as proof of absence for an intermittent bug. Read that - * file's own module comment for the full two-defect history; the fixes are - * the same fixes here: - * - * 1. Race the promote call itself (retrying immediately on its 409 "not - * running yet") instead of polling for `running` — see `sampleFreshStart` - * below. - * 2. Space samples at least SAMPLE_INTERVAL_MS apart, including before - * sample #0 — back-to-back promotions land on some kind of already-warm - * host resource and produce atypically short boots the close does not - * appear in (cold-start-canary.ts's module comment; gotchas.md's PRO-217 - * entry has the measured boot times). - * 3. Prove coldness from the deployment's own boot log - * (`/v1/deployments/{id}/logs?from_start=true`), margin-aware against - * cross-clock skew, rather than inferring it from latency — see - * rpc-cold-start-canary-classify.ts's `classifyBootEvidence`. - * 4. Require MIN_HELD_SAMPLES_FOR_BUG_GONE confirmed cold-start holds before - * a bug-gone verdict, since an intermittent bug's expected outcome from a - * too-small sample is "every touch happened to hold" even while the bug - * is fully present. - * - * What's different from the streams face, beyond the raw-fetch requirement - * above: - * - * - This canary has no "caller" to warm. cold-start-canary.ts warms the - * `jobs` service first so a cold `jobs` can't be mistaken for a cold - * `streams`; here, THIS SCRIPT is the caller (an ephemeral fetch from the - * CI runner, not a Compute service with its own cold start), so there is - * nothing upstream of the touch that needs warming. - * - No durability wait. The streams canary waits for a just-created stream - * to reach the object store before sampling, because a fresh streams - * instance restores its local state from there. `auth.service` keeps no - * local state to restore — its `verify` handler pings Postgres fresh on - * every call — so there is nothing to wait for between promoting and - * sampling beyond the inherited SAMPLE_INTERVAL_MS spacing itself. - * - No Authorization header. `auth.service` is wired to exactly one RPC - * consumer (`storefront`, via `deps: { auth: rpc(authContract) }` in - * examples/storefront-auth/modules/storefront/src/service.ts), so per - * ADR-0030/0031 it does enforce a per-edge bearer key. This script cannot - * obtain that key: it is minted once at deploy time and, per the - * Management API's own contract for environment variables, "is stored - * encrypted and is not returned by subsequent reads" — verified live - * against a real deploy, where an unauthenticated touch against a warm - * `auth.service` got back exactly `401 {"error":"Unauthorized: missing or - * invalid service key"}`. Sending no Authorization header (never a guessed - * one) is the honest choice, and rpc-cold-start-canary-classify.ts treats - * that specific, known 401 the same as a real success for classification - * purposes — see its module comment for why that is still sound evidence - * for PRO-217 specifically (the accepted-key check runs only after the - * ingress has already carried the connection through). - * - No listening-line source in the example as written. Unlike - * @prisma/streams-server, `auth.service`'s own server.ts never logged - * anything on boot, so cold-start-canary.ts's technique had nothing to - * read. examples/storefront-auth/modules/auth/src/server.ts now logs one - * self-timestamped line right after `Bun.serve()` returns (see that - * file's comment) — Compute's log relay passes plain app stdout through - * completely unmodified (verified live: no platform-added timestamp), so - * the timestamp has to come from the app's own clock, the same as - * streams-server's console.log patch does for the streams face. - * - * A REQUIRED check: any close → exit 0, bug still present (today's normal); - * enough touches reaching a genuine cold start AND holding → exit 1, the - * signal to retire this canary (never the Idempotency-Key protocol or the - * bounded retry — those are permanent protocol semantics for this kind, not - * a PRO-217 compensation); a run that never manages to force a cold start, - * or one whose log evidence can't place a touch on either side of the boot, - * or one too small to trust an all-held result from → exit 0 with a CI - * warning annotation. - */ -import { execSync } from 'node:child_process'; -import * as os from 'node:os'; -import { - classifyBootEvidence, - classifyRpcColdStartRun, - classifyRpcColdStartTouch, - findListeningTimestamp, - MIN_HELD_SAMPLES_FOR_BUG_GONE, - type RpcColdStartTouch, -} from './rpc-cold-start-canary-classify.ts'; - -const API = 'https://api.prisma.io/v1'; -/** - * MIN_HELD_SAMPLES_FOR_BUG_GONE confirmed cold-start holds are what a - * bug-gone verdict needs; sampling fewer than that can never produce one - * (classifyRpcColdStartRun reports inconclusive instead), so that count is - * the default budget. A close is decisive the moment it happens (see the - * early-exit in the sampling loop below), so a run against a stack where the - * bug is present typically finishes in far fewer samples than this. - */ -const SAMPLES = Number( - process.env['RPC_COLD_START_SAMPLES'] ?? String(MIN_HELD_SAMPLES_FOR_BUG_GONE), -); -/** - * The gap enforced before every sample, including the first — reproduces - * cold-start-canary.ts's SAMPLE_INTERVAL_MS spacing and its reasoning: - * back-to-back promotions land on some kind of already-warm host resource - * and produce atypically short boots the close does not appear in. Unlike - * the streams face, there is no separate durability wait stacked on top of - * this — auth.service restores no local state, so this is the only spacing - * a sample needs. - */ -const SAMPLE_INTERVAL_MS = Number(process.env['RPC_COLD_START_SAMPLE_INTERVAL_MS'] ?? '60000'); -/** - * How long to read a fresh deployment's boot logs before giving up on - * finding the `listening` line. Matches cold-start-canary.ts's - * LOG_READ_TIMEOUT_MS — manual probing of this same platform behavior has - * observed start->listening as long as 21.9s, so this sits comfortably above - * that. - */ -const LOG_READ_TIMEOUT_MS = Number(process.env['RPC_COLD_START_LOG_READ_TIMEOUT_MS'] ?? '30000'); -/** - * The run's own wall-clock budget — see cold-start-canary.ts's identical - * MAX_RUN_MS for the full reasoning (a job killed by the surrounding CI - * timeout never reaches classifyRpcColdStartRun, so it can't emit the - * inconclusive exit and warning annotation this script is supposed to use - * for a run that can't finish). This canary has no per-sample durability - * wait to budget for, so its worst case (MIN_HELD_SAMPLES_FOR_BUG_GONE - * samples at roughly SAMPLE_INTERVAL_MS plus a boot-and-touch each) fits - * comfortably inside the same 20-minute figure. - */ -const MAX_RUN_MS = Number(process.env['RPC_COLD_START_MAX_RUN_MS'] ?? '1200000'); -/** The HTTP port `auth.service` was deployed on — matches its compute() default (no explicit `port` param). */ -const AUTH_SERVICE_PORT = 3000; - -const token = process.env['PRISMA_SERVICE_TOKEN']; -const stackName = process.env['STACK_NAME']; -if (!token || !stackName) { - console.error('PRISMA_SERVICE_TOKEN and STACK_NAME are required'); - process.exit(1); -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null; -} - -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - -interface ApiResponse { - readonly status: number; - readonly data: unknown; -} - -/** POSTs/GETs the Management API, returning the status alongside the parsed `data` field — never throws on a non-2xx status. */ -async function apiCall(method: string, path: string, body?: unknown): Promise { - const init: RequestInit = { - method, - headers: { Authorization: `Bearer ${token}`, 'Content-Type': 'application/json' }, - }; - if (body !== undefined) init.body = JSON.stringify(body); - const res = await fetch(`${API}${path}`, init); - const text = await res.text(); - let json: unknown; - try { - json = text ? JSON.parse(text) : undefined; - } catch { - json = text; - } - return { status: res.status, data: isRecord(json) ? json['data'] : json }; -} - -/** Same as apiCall, but throws on a non-2xx status — for calls this script cannot proceed without. */ -async function apiData(method: string, path: string, body?: unknown): Promise { - const res = await apiCall(method, path, body); - if (res.status < 200 || res.status >= 300) { - throw new Error(`${method} ${path} failed: ${res.status} ${JSON.stringify(res.data)}`); - } - return res.data; -} - -function requireString(record: unknown, key: string): string { - if (!isRecord(record) || typeof record[key] !== 'string') { - throw new Error(`expected "${key}" to be a string`); - } - return record[key]; -} - -/** The per-run project shares the stack's name (`prisma-composer deploy --name`). */ -async function findProjectId(): Promise { - const projects = await apiData('GET', '/projects?limit=100'); - const list = Array.isArray(projects) ? projects : []; - const match = list.find((p) => isRecord(p) && p['name'] === stackName); - if (match === undefined) throw new Error(`no project named "${stackName}" — did the deploy run?`); - return requireString(match, 'id'); -} - -/** - * `auth.service`'s app id and its own base URL — the touch target. - * `/v1/apps` is the current Management API surface for what used to be - * `/v1/compute-services` (verified live — see cold-start-canary.ts's - * findApps and gotchas.md's PRO-217 entry). - */ -async function findAuthApp(projectId: string): Promise<{ authAppId: string; authUrl: string }> { - const apps = await apiData('GET', `/apps?projectId=${projectId}&limit=100`); - const list = Array.isArray(apps) ? apps : []; - for (const app of list) { - if (isRecord(app) && app['name'] === 'auth.service') { - return { - authAppId: requireString(app, 'id'), - authUrl: requireString(app, 'appEndpointDomain'), - }; - } - } - throw new Error(`stack "${stackName}" is missing the "auth.service" app`); -} - -/** - * The Deploy step that ran earlier in this job left the content-addressed - * `auth.service` artifact in the runner's temp dir (packageComputeArtifact) - * — reuse it so every promoted deployment is byte-identical to the deployed - * one. Mirrors cold-start-canary.ts's findStreamsArtifact. - */ -function findAuthArtifact(): string { - const dir = `${os.tmpdir()}/prisma-composer-compute-${os.userInfo().uid}`; - const found = execSync(`ls -t ${dir}/*/auth.service.tar.gz 2>/dev/null | head -1`, { - encoding: 'utf8', - }).trim(); - if (!found) throw new Error(`no auth.service.tar.gz under ${dir} — did the deploy build?`); - return found; -} - -/** - * Reads a deployment's boot log from the start, stopping as soon as - * `auth.service`'s own `listening` line has been seen (or - * LOG_READ_TIMEOUT_MS elapses, or the socket errors/closes). Returns the - * concatenated log text collected so far — `findListeningTimestamp` on the - * result may still be undefined if the line was never seen. Identical - * mechanism to cold-start-canary.ts's readDeploymentBootLog. - */ -function readDeploymentBootLog(deploymentId: string): Promise { - return new Promise((resolve) => { - const chunks: string[] = []; - let settled = false; - const ws = new WebSocket( - `wss://api.prisma.io/v1/deployments/${deploymentId}/logs?from_start=true`, - { headers: { Authorization: `Bearer ${token}` } }, - ); - const finish = () => { - if (settled) return; - settled = true; - clearTimeout(timer); - ws.close(); - resolve(chunks.join('')); - }; - const timer = setTimeout(finish, LOG_READ_TIMEOUT_MS); - ws.addEventListener('message', (event) => { - let parsed: unknown; - try { - parsed = JSON.parse(String(event.data)); - } catch { - return; - } - if (isRecord(parsed) && parsed['type'] === 'log' && typeof parsed['text'] === 'string') { - chunks.push(parsed['text']); - if (findListeningTimestamp(chunks.join('')) !== undefined) finish(); - } - }); - ws.addEventListener('error', finish); - ws.addEventListener('close', finish); - }); -} - -/** `err`'s message, plus its `cause` chain if it has one — a thrown fetch rejection's `cause` is where Bun puts the underlying socket error. */ -function errorText(err: unknown): string { - if (!(err instanceof Error)) return String(err); - const causeText = err.cause instanceof Error ? ` (cause: ${err.cause.message})` : ''; - return `${err.message}${causeText}`; -} - -/** - * One fresh `auth.service` deployment, touched once: create -> upload -> - * start -> race the promote call (retrying immediately on the "not running - * yet" 409 — NOT polling for `running` and then promoting, which is what let - * the boot window close in cold-start-canary.ts's original design; see this - * file's module comment) -> fire ONE bare `POST /rpc/verify` the instant - * promote succeeds -> confirm from the deployment's own boot log whether the - * touch actually landed before the app was listening. - */ -async function sampleFreshStart( - authAppId: string, - authUrl: string, - artifactPath: string, - index: number, -): Promise { - const created = await apiData('POST', `/apps/${authAppId}/deployments`, { - portMapping: { http: AUTH_SERVICE_PORT }, - }); - const deploymentId = requireString(created, 'id'); - const uploadUrl = requireString(created, 'uploadUrl'); - const artifact = await Bun.file(artifactPath).arrayBuffer(); - const uploaded = await fetch(uploadUrl, { method: 'PUT', body: artifact }); - if (!uploaded.ok) throw new Error(`artifact upload failed: ${uploaded.status}`); - - await apiData('POST', `/deployments/${deploymentId}/start`); - - const promoteDeadline = Date.now() + 30_000; - for (;;) { - const res = await apiCall('POST', `/apps/${authAppId}/promote`, { deploymentId }); - if (res.status === 200) break; - if (res.status !== 409 || Date.now() > promoteDeadline) { - throw new Error( - `promote never succeeded for deployment ${deploymentId}: ${res.status} ` + - JSON.stringify(res.data), - ); - } - // A short, deliberate courtesy delay — not a "wait for running" poll. - // Each retry is still racing to promote at the earliest legal moment; - // this just keeps a slow boot from hammering the API every few ms. - await sleep(200); - } - - const touchSentAt = new Date(); - const started = Date.now(); - let status = 0; - let body = ''; - let wasThrown = false; - try { - const res = await fetch(`${authUrl}/rpc/verify`, { - method: 'POST', - headers: { 'content-type': 'application/json', 'Idempotency-Key': crypto.randomUUID() }, - body: JSON.stringify({ token: `rpc-cold-start-canary-${index}` }), - signal: AbortSignal.timeout(60_000), - }); - status = res.status; - body = await res.text(); - } catch (err) { - wasThrown = true; - body = errorText(err); - } - const latencyMs = Date.now() - started; - - const logText = await readDeploymentBootLog(deploymentId); - const listeningAt = findListeningTimestamp(logText); - const bootEvidence = classifyBootEvidence(touchSentAt, listeningAt); - const evidence = - listeningAt !== undefined - ? `logs: listening ${listeningAt.toISOString()}, touch sent ${touchSentAt.toISOString()} (${bootEvidence})` - : `no listening line read within ${LOG_READ_TIMEOUT_MS}ms — boot evidence unknown, not guessed`; - - const touch = classifyRpcColdStartTouch(status, body, wasThrown, bootEvidence); - const statusLabel = wasThrown ? 'thrown' : String(status); - console.log( - ` sample #${index}: ${touch} (${statusLabel}, ${latencyMs}ms) [${evidence}] — ${body.slice(0, 160)}`, - ); - return touch; -} - -const projectId = await findProjectId(); -const { authAppId, authUrl } = await findAuthApp(projectId); -const artifactPath = findAuthArtifact(); -console.log(`Stack "${stackName}" (${projectId}); auth.service at ${authUrl}`); -console.log(`Sampling ${SAMPLES} fresh auth.service instances, ${SAMPLE_INTERVAL_MS}ms apart…`); - -const runStartedAt = Date.now(); -const touches: RpcColdStartTouch[] = []; -for (let i = 0; i < SAMPLES; i++) { - if (Date.now() - runStartedAt > MAX_RUN_MS) { - console.log( - ` stopping after ${i} sample(s): the run's own ${MAX_RUN_MS}ms wall-clock budget is used ` + - 'up — reporting the touches collected so far rather than risking a CI timeout kill.', - ); - break; - } - console.log(` waiting ${SAMPLE_INTERVAL_MS}ms before sample #${i}…`); - await sleep(SAMPLE_INTERVAL_MS); - const touch = await sampleFreshStart(authAppId, authUrl, artifactPath, i); - touches.push(touch); - // A close is decisive on its own (classifyRpcColdStartRun's rule) — the - // verdict is already bug-present, and running the rest of the budget only - // spends CI minutes without changing the answer. - if (touch === 'closed') { - console.log( - ` close observed on sample #${i}; bug-present is already decided — skipping the ` + - `remaining ${SAMPLES - i - 1} samples.`, - ); - break; - } -} - -const result = classifyRpcColdStartRun(touches); -console.log(result.message); -if (result.verdict === 'inconclusive') { - // A GitHub Actions warning annotation: loud on the run page without - // failing a required check over a deploy flake. Newlines must be %0A. - const detail = touches.map((touch, i) => `sample #${i}: ${touch}`).join('; '); - console.log( - `::warning title=RPC cold-start canary (PRO-217) inconclusive::${result.message} [${detail}]`, - ); -} -process.exitCode = result.verdict === 'bug-gone' ? 1 : 0; From 161aa92e8b7844bca76477ff10069413f167610f Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 18:09:29 +0200 Subject: [PATCH 12/18] chore(drive): record that the rpc canary was dropped on the evidence Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../forcing-function-apps/design-notes.md | 12 ++++++++ .../slices/rpc-cold-start/plan.md | 28 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/.drive/projects/forcing-function-apps/design-notes.md b/.drive/projects/forcing-function-apps/design-notes.md index 49b3bd69..657bdf7c 100644 --- a/.drive/projects/forcing-function-apps/design-notes.md +++ b/.drive/projects/forcing-function-apps/design-notes.md @@ -233,3 +233,15 @@ flight; the branch is rebased and all paths follow the new package and the ADR-0037 is the number for the keyed protocol (0033–0036 taken); it ships with the implementation in this slice's PR, never as a docs-only change. + +### RPC cold-start canary dropped (2026-07-20, Will accepted) + +The slice's own canary was built, run live, and dropped on the evidence +(revert `a46abfa`). The auth service boots in under ~1.5s (no state to +restore), narrower than the 2s coldness-proof margin, so 14 forced races +certified as cold zero times. The decisive reason is not the margin, though: +the RPC idempotency retry is permanent protocol semantics (ADR-0037), so +there is no workaround for a canary to time the removal of — and PRO-217, a +platform ingress bug, is already watched by the streams canary, which catches +it well because streams has the long boot window RPC lacks. The slice ships +the keyed protocol; the streams canary stays the platform-wide PRO-217 signal. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md index ebdc36f6..3b6d13e1 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md @@ -32,7 +32,33 @@ attempts, fresh-key-per-call, 5xx-not-cached, and exception-message-not- returned); both RPC-consuming example suites pass unchanged; repo checks green with casts delta ≤ 0; committed with DCO dual sign-off. -## D2 — the canary (scripts + CI) +## D2 — the canary (scripts + CI) — ATTEMPTED, THEN DROPPED (2026-07-20) + +**Outcome: no RPC canary ships.** The canary was built and run live; the +evidence retired it (reverted in `a46abfa`). Two reasons, the second +decisive: + +1. It cannot certify a cold start against `examples/storefront-auth`. The + auth service restores no state and boots in under ~1.5s — narrower than + the 2s clock-skew margin the coldness proof requires. The live run forced + 14 genuine races (touch landed 376–1185ms before the listening line, + every sample) but certified none, correctly refusing to shrink the margin + to make its own numbers pass. +2. A cold-start canary exists to signal when a platform workaround can be + deleted. The RPC idempotency retry is **permanent protocol semantics** + (ADR-0037), not a workaround — there is nothing for a canary to time the + removal of. PRO-217 itself is a platform ingress bug already watched by + the streams cold-start canary, which catches it well precisely because + streams has the long boot window RPC lacks. + +Superseded original intent (kept for the record): a sibling of the streams +cold-start canary probing the auth service's rpc endpoint with a bare +single-attempt `fetch`. If a future RPC edge with a real restore-from-store +boot exists, and only if the retry ever becomes removable, revisit with an +independently measured skew margin rather than the streams canary's borrowed +constant. + +### Original D2 outcome (not shipped) **Outcome:** `scripts/rpc-cold-start-canary.ts` + `-classify.ts` + unit tests, inheriting the cold-start canary's proven contract wholesale From f01973ab54f2ebc3b9a5f94e5b1135dfba197eff Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 18:10:07 +0200 Subject: [PATCH 13/18] =?UTF-8?q?docs(gotchas):=20PRO-217=20gains=20the=20?= =?UTF-8?q?service-RPC=20face=20=E2=80=94=20keyed=20retry,=20no=20canary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RPC client absorbs the cold-start close with a keyed, dedup-safe retry that is permanent protocol semantics, so there is no workaround to retire and no RPC canary; PRO-217 stays watched by the streams canary, whose long boot window RPC lacks. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- gotchas.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gotchas.md b/gotchas.md index 70921c56..9a818502 100644 --- a/gotchas.md +++ b/gotchas.md @@ -396,6 +396,8 @@ The window is measurable, and much wider than first recorded. In `examples/strea **Removal guard.** The CI "Cold-start canary (PRO-217)" job (`scripts/cold-start-canary.ts`, in the E2E deploy workflow) touches freshly promoted instances each run, 60 s apart, and confirms from the deployment's own boot log that each touch was sent before the server's "listening" line — a touch that cannot be placed on one side of the boot counts for nothing rather than being guessed. It fails only when enough touches reached a genuine cold start **and** every one of them held; because this bug is intermittent, "every touch held" in a small run is the expected outcome of a run that is too small, so the job requires 14 confirmed cold-start holds before it will claim the bug is gone (at a conservative 20% close rate, 0.8^14 ≈ 4.4% — the chance of being fooled by luck). An inconclusive run passes with a warning annotation. That failure is the signal to remove the streams client's `IDEMPOTENT_BACKOFF` (the PRO-219 compensation) and the canary itself, the same contract as FT-5226's cold-connect canary. +**The service-RPC face, and why it has no canary of its own.** The same ingress closes a service-to-service RPC call's first connection while the target cold-starts. The generated `service-rpc` client absorbs it: every call carries an idempotency key and is retried with a bounded backoff, and the server dedupes on the key so a retry cannot double-execute (ADR-0037). Two things follow that differ from the streams face. First, this retry is **permanent protocol semantics, not a compensation** — it is correct on any transport that can drop a request, so nothing here is ever removed when PRO-217 is fixed; there is no workaround to time the removal of, and therefore no RPC-specific canary. Second, an RPC canary was built and dropped on the evidence anyway: an RPC service that restores no state (`examples/storefront-auth`'s `auth`) boots in under ~1.5 s — narrower than the 2 s clock-skew margin the coldness proof needs — so a probe cannot certify a cold start even though it reliably forces one (touch measured 376–1185 ms ahead of the listening line across 14 samples). PRO-217 stays watched by the streams canary above, which catches it because streams has the long boot window RPC lacks; when that canary reports the platform healed, the RPC keys and retry still stay, because they were never a workaround. + **Reproduction.** 1. Deploy two Compute services, A calling B over HTTP on each request to A. From 6f282e517653d9f34f8733d01204bbbc12716461 Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 18:11:31 +0200 Subject: [PATCH 14/18] docs(guides): document the idempotency-key protocol on the RPC surface building-an-app gains a "Calls retry safely for you" section (the key, the safe retry, the keyless-400, the optional ctx third arg); getting-started and SKILL get terse pointers to it. Matches the new serve()/makeClient behavior from ADR-0037. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- docs/guides/building-an-app.md | 24 ++++++++++++++++++++++-- docs/guides/getting-started.md | 7 +++++++ skills/prisma-composer/SKILL.md | 12 ++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/docs/guides/building-an-app.md b/docs/guides/building-an-app.md index cd98d25c..ffe46d3d 100644 --- a/docs/guides/building-an-app.md +++ b/docs/guides/building-an-app.md @@ -47,8 +47,9 @@ export const authContract = contract({ On the producer, `serve()` turns the service's `expose` into a fetch handler. The handler map must cover every method — a missing or wrong-shaped handler -doesn't compile. Each handler receives the validated input (and the service's -own loaded deps as a second argument): +doesn't compile. Each handler receives the validated input, the service's own +loaded deps as a second argument, and an optional third argument carrying the +call's idempotency key (see below — most handlers ignore it): ```ts const handler = serve(service, { @@ -88,6 +89,25 @@ run it locally. service you run in a terminal, a fake, and `bootstrapService` all accept every call, and there's no key for you to supply. +### Calls retry safely for you + +You don't do anything for this either. A provider that has scaled to zero has +to boot before it answers, and a first call can be dropped mid-connection +while it does. The client absorbs that: every call carries an **idempotency +key**, a dropped call is retried with a backoff, and the provider runs one +call per key — a retry that arrives after the first already ran gets the first +answer back instead of running your handler twice. So `await auth.verify(...)` +just works across a cold start, and it works whether or not the call changes +state. You write nothing; the key is on the request and the deduplication is +in `serve()`. + +The one visible edge, again from `curl`: a request without an `Idempotency-Key` +header is answered with `400`. The generated client always sends one, so this +only bites a hand-rolled request. If a handler needs a stronger guarantee than +one instance's memory — surviving a crash mid-call — its optional third +argument carries the same key, to write into its own transaction; most +handlers never need it. + Two limits worth knowing: - **A key opens the whole service, not one method.** Any valid key reaches diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index 2405686d..7d25dd30 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -329,6 +329,13 @@ a deploy creates them. [Building an app](building-an-app.md#calls-are-authenticated-for-you) has the details. +Two more things the generated client does for you, both invisible in your +code: every call carries an idempotency key and retries safely if the target +was still cold-starting, and the provider deduplicates on that key so a retry +never runs your handler twice. (A hand-rolled request without the key is +answered `400` — the client always sends one.) +[Building an app](building-an-app.md#calls-retry-safely-for-you) has these too. + Re-deploying is idempotent — it updates the same Project. For an isolated copy of the whole app (own services, own config), deploy a **stage**, and tear it down when you're done: diff --git a/skills/prisma-composer/SKILL.md b/skills/prisma-composer/SKILL.md index b2fbba9c..44d40632 100644 --- a/skills/prisma-composer/SKILL.md +++ b/skills/prisma-composer/SKILL.md @@ -157,6 +157,18 @@ deployed `/rpc/` to check it works** — an unwired caller always gets `401`, which looks like a broken deploy and isn't. Debug through a consumer, or locally. +**Calls carry an idempotency key and retry safely for you.** Every call the +generated client makes carries an `Idempotency-Key`; a call dropped while the +target cold-starts is retried with a backoff, and `serve()` runs one call per +key — a retry that arrives after the first completed replays that answer +instead of re-running the handler. So every method is safely retryable and no +contract declares anything about it (do not add an "is this idempotent" flag — +the framework does not have one). Two consequences for you: a handler may take +an **optional third argument** `(input, deps, ctx)` and read `ctx.idempotencyKey` +if it needs exactly-once beyond one instance's memory (most don't); and a +request without the header is answered `400`, so any hand-rolled probe must +send one — another reason not to `curl` these endpoints. + | | | | --- | --- | | Locally / in tests | nothing is provisioned, so `serve()` passes every call through — never supply a key in `inputs` | From ed897cfde176961dfd52b5271ec0099d64503a17 Mon Sep 17 00:00:00 2001 From: willbot Date: Mon, 20 Jul 2026 18:29:42 +0200 Subject: [PATCH 15/18] fix(service-rpc): close the review findings on the keyed protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A request body that errors mid-read now masks and logs like every other internal failure, instead of rejecting out of serve() (which broke its Response contract and skipped the operator log). New test, mutation-checked. - Add the missing test for output-validation masking (a handler returning schema-violating output → masked 500 + provider-bug log), mutation-checked. - Document why six retry attempts cover a boot up to ~22s (a held connection blocks until the server is listening; the budget only governs actively closed attempts) and that maxDelayMs is an unreached ceiling at maxRetries:5. - standard-schema.ts no longer claims the client re-validates responses. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../service-rpc/src/__tests__/serve.test.ts | 70 +++++++++++++++++++ .../2-authoring/service-rpc/src/client.ts | 13 ++++ .../2-authoring/service-rpc/src/serve.ts | 7 +- .../service-rpc/src/standard-schema.ts | 5 +- 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts index a15ddb82..b61433d4 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts @@ -142,6 +142,76 @@ describe('serve()', () => { } }); + test('a handler returning schema-violating output is a masked 500, and logs it as a provider bug', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { + rpc: { + // `ok` must be a boolean; return a secret-bearing string instead. + // JSON.parse is typed `any`, so this defeats the output type at + // authoring without a cast — the runtime value is what serve()'s + // output validation must catch. + verify: async () => JSON.parse('{"ok":"leaked-secret-value"}'), + }, + }); + + const logged: unknown[][] = []; + const originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + logged.push(args); + }; + try { + const res = await handler(verifyRequest({ token: 't' })); + + expect(res.status).toBe(500); + const bodyText = await res.text(); + expect(bodyText).not.toContain('leaked-secret-value'); + expect( + logged.some((args) => + args.some((a) => typeof a === 'string' && a.includes('provider bug')), + ), + ).toBe(true); + } finally { + console.error = originalConsoleError; + } + }); + + test('a request body that errors mid-read is a masked 500, not a rejected promise or a leaked error', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + // A body stream that fails partway through with a secret-bearing error. + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"tok')); + controller.error(new Error('socket truncated: secret-xyz')); + }, + }); + const req = new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': 'body-err-key' }, + body, + duplex: 'half', + }); + + const logged: unknown[][] = []; + const originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + logged.push(args); + }; + try { + const res = await handler(req); + expect(res.status).toBe(500); + expect(await res.text()).not.toContain('secret-xyz'); + expect( + logged.some((args) => + args.some((a) => a instanceof Error && a.message.includes('secret-xyz')), + ), + ).toBe(true); + } finally { + console.error = originalConsoleError; + } + }); + test('the wrong HTTP verb on a known method is a 405', async () => { const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); diff --git a/packages/0-framework/2-authoring/service-rpc/src/client.ts b/packages/0-framework/2-authoring/service-rpc/src/client.ts index f344698d..351677b5 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/client.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/client.ts @@ -35,6 +35,19 @@ export type Transport = (req: Request) => Promise; * because service-rpc is framework-layer and must not depend on * prisma-cloud. `maxRetries` counts retries after the first attempt, so a * persistently failing call sends 6 requests in total before giving up. + * + * Why 6 attempts covers a boot that can take up to ~22s even though the + * backoff sums to only ~8s: the close is intermittent. On a cold start the + * ingress usually HOLDS the connection and the request simply blocks until + * the server is listening (there is no client-side request timeout here), so + * a held attempt rides out the whole boot and succeeds on its own — the + * backoff budget only governs the case where an attempt is actively closed, + * and every close so far has been the fast (~400ms) kind. Spending six + * attempts against that means the call fails only if it is closed six times + * in a row, which the observed close rate makes rare. `maxDelayMs` is the + * ceiling the doubling would be clamped to; at maxRetries:5 the last wait is + * 4s, so the clamp never actually binds — it is kept so raising maxRetries + * stays safe. */ const RETRY = { initialDelayMs: 250, diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index 6dc31383..9662f7e2 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/serve.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/serve.ts @@ -371,7 +371,12 @@ export function serve>( if (err instanceof RequestBodyTooLargeError) { return outcome({ error: `Request body exceeds the ${MAX_BODY_BYTES}-byte limit` }, 413); } - throw err; + // A stream I/O error mid-read (a truncated or reset request body): mask + // and log it like any other internal failure rather than letting it + // reject out of the handler, which would both break serve()'s + // Response contract and skip the console.error operators rely on. + console.error(`serve(): reading the request body for "${methodName}" failed:`, err); + return outcome({ error: INTERNAL_ERROR_MESSAGE }, 500); } let body: unknown; diff --git a/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts b/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts index 0c34791e..1d203fc7 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts @@ -1,8 +1,9 @@ /** * Runs a Standard Schema validator over an unknown value, returning the * parsed output — or throwing with the validator's own issues on failure. - * Shared by the RPC client (validating a response) and serve() (validating a - * request and a handler's return). + * Used by serve() to validate a request's input and its handler's return; + * the client does not re-validate the response, since serve() already + * guaranteed it against the same schema. */ import type { StandardSchemaV1 } from '@standard-schema/spec'; From 6b83992c3dd27309f3b22ad9df8b4b26a784d982 Mon Sep 17 00:00:00 2001 From: willbot Date: Tue, 21 Jul 2026 11:08:51 +0200 Subject: [PATCH 16/18] refactor(service-rpc): a keyless request opts out of dedup, not a 400 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A request without an Idempotency-Key is now served once, without dedup or replay, instead of being rejected. The safe path is the generated client, which always pairs a key with the retry, so a keyless request never came from it — accepting it adds no unsafe retry path and lets curl and older callers work transparently, degrading gracefully instead of hard-breaking. ctx.idempotencyKey becomes string | undefined. Also: trim the oversized code comments down to what earns its place, and rewrite ADR-0037 to build the accepted mechanism directly (the rejected flag moves wholly to Alternatives) and to drop project-temporal framing. The keyless-400 wording is updated across connection-contracts, the guides, the skill, the ADR index, and the slice spec/plan. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../slices/rpc-cold-start/plan.md | 6 +- .../slices/rpc-cold-start/spec.md | 10 +- .../design/10-domains/connection-contracts.md | 7 +- ...vice-rpc-calls-carry-an-idempotency-key.md | 180 +++++++++--------- docs/design/90-decisions/README.md | 2 +- docs/guides/building-an-app.md | 12 +- docs/guides/getting-started.md | 4 +- .../src/__tests__/serve-handlers.test-d.ts | 6 +- .../service-rpc/src/__tests__/serve.test.ts | 50 ++++- .../2-authoring/service-rpc/src/client.ts | 52 +---- .../2-authoring/service-rpc/src/serve.ts | Bin 16258 -> 15043 bytes .../cron/src/__tests__/serve-schedule.test.ts | 8 +- skills/prisma-composer/SKILL.md | 7 +- 13 files changed, 174 insertions(+), 170 deletions(-) diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md index 3b6d13e1..d0de8663 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md @@ -16,7 +16,7 @@ believed. `makeClient` mints one `Idempotency-Key` per logical call and reuses it across a bounded retry (250 ms / ×2 / 5 s cap / 5 attempts / jitter; retry network errors + 5xx + 429, never other 4xx). `serve()` enforces the key -(keyless → loud 400), single-flights per in-flight key, replays completed +(keyless requests opt out — served once, no dedup), single-flights per in-flight key, replays completed 2xx/4xx for ~60 s under a justified LRU bound, never caches 5xx/throws, and passes `ctx.idempotencyKey` as an optional third handler argument after `deps`. Absorbed in the same pass: the request body size limit (413, @@ -79,8 +79,8 @@ report, do not ship); workspace left clean with project counts; committed. **Outcome:** reviewer pass over D1+D2. Attack priorities: a repeated key can never double-execute within an instance; a replay can never leak one logical -call's answer to another, across methods or callers; keyless rejection -cannot be bypassed; the body cap holds without a truthful `content-length`; +call's answer to another, across methods or callers; a keyless request opts +out cleanly (served once, no dedup); the body cap holds without a truthful `content-length`; no handler exception text reaches a caller while operators keep it; the canary cannot be masked by the retry machinery; every reported number is real. Findings closed. Full live round (deploy storefront-auth, canary diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md index 4f7b5c76..baa796e5 100644 --- a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md @@ -30,8 +30,12 @@ be rebased onto main at or after `6ec2625` before any code is written. UUID minted **once per logical call** and reused **byte-identically across every retry of that call**. Two separate logical calls never share a key. `crypto.randomUUID()` — no new dependency. -- The key is REQUIRED: `serve()` rejects a keyless request with a loud 400 - naming the header. "Requires" is enforced, not suggested. +- The generated client always sends the key. A request that arrives without + one (a hand-rolled or older caller) is served once, with no dedup or replay, + rather than rejected — it has opted out. `ctx.idempotencyKey` is `undefined` + for such a call. (Superseded the earlier "keyless → 400" rule, on the + grounds that the safe path is the always-keyed generated client, so keyless + requests never came from it and opt-out degrades transparently.) ### Client (`makeClient`) @@ -160,7 +164,7 @@ Server tests: - Repeated key after completion → handler ran **once**, response replayed byte-identically. - Concurrent same-key → **one** execution (single-flight). -- Keyless → 400 naming the header. +- Keyless → served once, no dedup (handler runs each time); `ctx.idempotencyKey` is `undefined`. - 5xx not cached → a same-key retry re-executes. - LRU bound evicts. - A replay can never answer a different method. diff --git a/docs/design/10-domains/connection-contracts.md b/docs/design/10-domains/connection-contracts.md index aca02951..2df3310f 100644 --- a/docs/design/10-domains/connection-contracts.md +++ b/docs/design/10-domains/connection-contracts.md @@ -198,9 +198,10 @@ The client mints one key per *logical call* and sends the same key on every retr of it, so the server can tell a retry from a new call: it runs one call per key, lets a duplicate that arrives mid-flight wait for the first, and replays a completed answer (a `2xx` or a `4xx` — never a `5xx`, which is the outcome a retry -exists to escape) to a later duplicate. The key is required — the server rejects a -keyless request with `400` naming the header — so deduplication is never silently -lost to a caller that forgot one. +exists to escape) to a later duplicate. The generated client always sends a key, +so every framework call is deduplicated; a request that arrives without one — a +hand-rolled or older caller — is served once, with no deduplication, rather than +rejected. This is a property of the binding, like the service key above: an in-memory or mock binding has no network hop to drop a request, so it neither retries nor diff --git a/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md b/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md index 720737e8..91d0401b 100644 --- a/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md +++ b/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md @@ -1,4 +1,4 @@ -# ADR-0037: Service RPC calls carry an idempotency key, so every call is safely retryable +# ADR-0037: Service RPC calls carry an idempotency key, so every call is safe to retry ## Status @@ -6,30 +6,31 @@ Accepted ## Decision -Every service RPC request carries an `Idempotency-Key` header. The client -mints one key per logical call and reuses it on every retry of that call; the -server requires it, runs one call per key, and replays a completed answer to a -repeat. Because duplicates are absorbed by the protocol rather than by a -promise from the method author, the client retries **every** method — there is -no per-method opt-in. +Every call the generated service-RPC client makes carries an `Idempotency-Key` +header: one key per logical call, reused on every retry of that call. The +provider runs one call per key — a duplicate that arrives while the first is +still running waits for it, and a duplicate that arrives after an answer +replays that answer without running the handler again. Because duplicates are +absorbed by the protocol, **every method is retryable and no method declares +anything** — there is no per-method "is this safe to retry" flag. ``` -POST /rpc/placeOrder POST /rpc/placeOrder -Idempotency-Key: 9f2c…e1 Idempotency-Key: 9f2c…e1 ← same key, retry +POST /rpc/placeOrder POST /rpc/placeOrder +Idempotency-Key: 9f2c…e1 Idempotency-Key: 9f2c…e1 ← same key, a retry Authorization: Bearer - server: this key already ran → - ✗ connection closed mid-boot replay its answer, do not - run the handler again + provider: this key already ran → + ✗ connection dropped mid-boot replay its answer, don't + run the handler again ``` -A handler may read the key when it wants a stronger guarantee than the -framework's own: +A handler that needs a stronger guarantee than the provider's own memory reads +the key from an optional third argument; handlers that don't are unchanged: ```ts serve(service, { orders: { - // `ctx` is optional — existing two-argument handlers are unaffected. placeOrder: async (input, deps, ctx) => { + // ctx.idempotencyKey lets a handler make its own work exactly-once. await deps.db.insert({ ...input, requestKey: ctx.idempotencyKey }); return { placed: true }; }, @@ -39,101 +40,98 @@ serve(service, { ## Reasoning -A service that has scaled to zero has to boot before it can answer, and a -caller's first request can be dropped while that happens — the connection is -closed during establishment, before any handler runs. The obvious repair is for -the client to retry, and the obvious objection is that a retried `POST` may -execute a write twice. - -The first design that suggests itself is to let each method say whether it is -safe to repeat — `rpc({ input, output, idempotent: true })` — and retry only -the methods that say yes. It should be rejected, and the reason generalizes: -idempotence is a property of what a handler does to state, so nothing at the -RPC layer can check the claim. The framework would be retrying on an assurance -it cannot verify, and the failure mode is silent — a method marked wrongly -duplicates writes with nothing to catch it. An unverifiable flag is also an -invitation to omission: the safe default is "not idempotent", so the common -outcome is that nobody marks anything and the mechanism protects nothing. - -An idempotency key inverts that. The client mints one identifier per logical -call — not per attempt — so every retry of that call is recognizable as the -same call, and two genuinely separate calls never collide. The server keys its -work on it: a duplicate that arrives while the first attempt is still running -waits for that attempt instead of starting a second one, and a duplicate that -arrives after an answer was produced receives that same answer without the -handler running again. Duplicate suppression becomes a mechanism the framework -enforces, so the method author is asked for nothing, and every method is -retryable. - -What counts as "an answer" matters. A `2xx` and a `4xx` are both conclusions — -the call succeeded, or it was rejected — and replaying either is correct. A -`5xx` or a thrown error is not a conclusion; it is the outcome a retry exists -to escape, so those are never remembered and a retry re-executes. The memory of -answers is bounded in time and in size: it exists to absorb the retries of a -call still in flight, not to serve as a general result cache, so it holds a -fixed number of recent answers for about a minute. - -Making the key required rather than optional is what keeps the guarantee -whole. A server that accepts keyless requests silently loses deduplication for -any caller that forgets one, and "required" enforced by documentation is not -enforced. A keyless request is therefore rejected outright, and the rejection -names the header so the fix is obvious to whoever hits it — including a person -with `curl`. - -Retrying is now permanent behavior of this kind, not a workaround for one -platform's cold starts. Keyed retries are correct on any transport that can -drop a request, so nothing here is written to be deleted later, even though -one platform's specific behavior is what made shipping it urgent. +A service that has scaled to zero must boot before it can answer, and the +caller's first request can be dropped while it does — the connection is closed +during establishment, before any handler runs. The repair is for the client to +retry; the objection is that a retried `POST` might run a write twice. The key +is what turns the retry from dangerous into safe. + +The client generates one identifier per logical call — not per attempt — and +sends it on every attempt of that call. So a retry is always recognizable as +the same call, and two genuinely separate calls never collide. The provider +keys its work on it: a second request for a key still in flight waits for the +first instead of starting a second execution, and a second request after the +first has answered gets that same answer back. Duplicate suppression is a +mechanism the framework runs, so the method author is asked for nothing and +every method is retryable. + +What counts as "an answer" is the load-bearing distinction. A `2xx` and a `4xx` +are both conclusions — the call succeeded, or it was rejected — so replaying +either is correct. A `5xx` or a thrown error is not a conclusion; it is the +outcome a retry exists to escape, so it is never remembered and a retry +re-executes. The memory of answers exists only to absorb the retries of a call +still in flight, not to serve as a general result cache, so it holds a fixed +number of recent answers for about a minute. + +The key is what makes retrying safe, and it lives entirely in the generated +client, which pairs "mint a key" with "retry" inseparably and always sends the +key. A request that arrives *without* a key therefore never came from that +client — it is a hand-rolled or older caller — so it is run once, with no +deduplication, rather than rejected. This keeps the safe path automatic (the +generated client is always keyed) while letting an out-of-band caller, or a +`curl` during debugging, work transparently. The handler sees the key as +`undefined` for such a call and decides for itself whether that is acceptable. + +Retrying is permanent behavior of this kind, not a workaround for one platform. +Keyed retries are correct on any transport that can drop a request, so nothing +here is written to be removed later. ### What this guarantees, and what it does not [`connection-contracts.md`](../10-domains/connection-contracts.md) sets the scope this calibrates against: a connection is internal because both ends belong to one application's topology, not because they share a network, and -robustness is justified per edge against the named failure modes of the -targets carrying it. +robustness is justified per edge against the named failure modes of the targets +carrying it. -The named failure here is a request dropped while its target boots. That +The failure this guards is a request dropped while its target boots. That happens before a handler runs, so nothing was applied and a retry cannot -duplicate work — the keys are not what makes that case safe. What the keys add +duplicate work — the keys are not what make *that* case safe. What the keys add is the narrower case where a handler did run and its answer was lost on the way back: the retry finds the recorded answer instead of running the work twice. -The server's memory of answers lives in the serving process, so it cannot cover -a retry that reaches a *different* instance than the one that did the work — -after an instance dies mid-request, or on an edge whose retries may be routed -elsewhere. That residue is deliberate. Closing it would put a durable store -behind every provider, and no supported target's failure modes ask for that -today. An edge that does need it has the tool without the framework imposing -it: the handler receives the key and can record it inside its own transaction, -making its own work exactly-once by its own storage's guarantees. +The provider's memory of answers lives in the serving process, so it cannot +cover a retry that reaches a *different* instance than the one that did the work +— after an instance dies mid-request, or on an edge whose retries route +elsewhere. That residue is deliberate: closing it would put a durable store +behind every provider, and no supported target's failure modes call for that. +An edge that does need it has the tool without the framework imposing it — the +handler receives the key and can record it inside its own transaction, making +its own work exactly-once by its own storage's guarantees. ## Consequences -- **Every method is retryable, and no contract declares anything.** Adding - retry behavior needs no change to a contract, a method, or a handler. -- **A keyless request is a `400`.** Any caller that is not the generated - client — a test fixture, a `curl` command — must send a key. +- **Every method is retryable, and nothing declares it.** Retry behavior needs + no change to a contract, a method, or a handler. +- **The generated client always sends a key**, so every framework-to-framework + call is deduplicated. +- **A keyless request is served once, without deduplication.** A hand-rolled + request or a `curl` works transparently; a handler's `ctx.idempotencyKey` is + `undefined` for it. - **Handlers may take a third argument** carrying the key. Existing two-argument handlers are unaffected; the argument is optional. -- **A provider holds a bounded amount of recent answers in memory**, sized to +- **A provider holds a bounded number of recent answers in memory**, sized to absorb in-flight retries rather than to grow with traffic. - **Cross-instance duplicate suppression is the handler's to add**, using the key it is given, when its target's failure modes warrant it. ## Alternatives considered -- **Per-method `idempotent: true`, retry only marked methods.** Rejected: the - framework cannot verify the claim, a wrong mark duplicates writes silently, - and the safe default guarantees under-marking. The key replaces a promise - with a mechanism. -- **Retry everything without keys.** Rejected: it converts a visible failure - into a silent duplicate write, on the assumption that every handler is - repeat-safe — the same unverifiable claim, made on the author's behalf. -- **Durable, cross-instance deduplication in the framework.** Rejected here: - it puts a storage dependency behind every provider to close a window no - supported target's named failure modes call for. The handler can reach that - guarantee for the edges that need it, using the key this decision gives it. -- **Client-side retry only, with no server-side deduplication.** Rejected: it - is exactly retry-without-keys from the caller's side, and leaves the - ambiguous case — answer lost after the work was done — duplicating. +- **A per-method `idempotent: true` flag, retrying only marked methods.** + Rejected: idempotence is a property of what a handler does to state, so + nothing at the RPC layer can verify the claim. The framework would retry on + an assurance it cannot check, and a method marked wrongly would duplicate + writes with nothing to catch it; the safe default ("not idempotent") also + guarantees under-marking, so the mechanism would protect almost nothing. The + key replaces a promise with a mechanism. +- **Retrying without keys.** Rejected: it turns a visible failure into a silent + duplicate write, on the assumption that every handler is repeat-safe — the + same unverifiable claim, made on the author's behalf. +- **Rejecting a keyless request outright.** Rejected: the only caller that can + omit a key is one outside the generated client, for which "no key, no + deduplication" is the honest and expected behavior; rejecting it breaks + `curl` and older clients to guard a case the safe path already covers. +- **Durable, cross-instance deduplication in the framework.** Rejected: it puts + a storage dependency behind every provider to close a window no supported + target's failure modes call for. A handler that needs it has the key to do it + itself. diff --git a/docs/design/90-decisions/README.md b/docs/design/90-decisions/README.md index c05b706d..1244a208 100644 --- a/docs/design/90-decisions/README.md +++ b/docs/design/90-decisions/README.md @@ -58,4 +58,4 @@ _Earlier drafts (ADR-0001, ADR-0002) were retired as the high-level design settl - [ADR-0034](ADR-0034-deploy-state-lives-in-the-stage-branch.md) — Deploy state lives in a framework-owned `prisma-composer-state` database in the stage's own Branch (production: the implicit default Branch). State has the environment's lifetime: platform-side Branch/Project deletion cleans it up with no framework involvement; the CLI deletes it last-among-members on destroy. Supersedes ADR-0009's workspace-level store. - [ADR-0035](ADR-0035-public-entrypoints-live-in-src-exports.md) — Public entrypoints live in `src/exports/` (one file per subpath; internals stay at the `src/` root); `@internal/tsdown-config` generates `package.json#exports` from object-named entries where safe, with two deliberate exceptions kept hand-maintained (the multi-pass `cron`/`storage`/`streams`, and the two published packages). Completes ADR-0028. - [ADR-0036](ADR-0036-the-rpc-kind-is-named-service-rpc.md) — The RPC kind is named **service RPC**: subpath `@prisma/composer/service-rpc`, unchanged call-site names (`rpc()`, `contract()`, `serve()`), kind brand stays `'rpc'`. Scope recorded in connection-contracts.md: edges internal to the application topology, agent-generatable by design — not an application API layer, not general distributed-systems infrastructure. -- [ADR-0037](ADR-0037-service-rpc-calls-carry-an-idempotency-key.md) — Every service RPC call carries an `Idempotency-Key`: the client mints one per logical call and reuses it across a bounded retry; the server requires it, runs one call per key, and replays completed 2xx/4xx answers (never 5xx) from a bounded in-process store. Retrying is permanent protocol behavior, not a platform workaround, and there is no per-method opt-in — a flag would be an unverifiable claim. Handlers may read the key via an optional third argument for their own durable exactly-once. +- [ADR-0037](ADR-0037-service-rpc-calls-carry-an-idempotency-key.md) — The generated service RPC client carries an `Idempotency-Key` on every call — one per logical call, reused across a bounded retry — and the provider deduplicates on it: one call per key, replaying completed 2xx/4xx answers (never 5xx) from a bounded in-process store. A keyless request (a hand-rolled or older caller) is served once without deduplication rather than rejected. Retrying is permanent protocol behavior, not a platform workaround, and there is no per-method opt-in — a flag would be an unverifiable claim. Handlers may read the key via an optional third argument for their own durable exactly-once. diff --git a/docs/guides/building-an-app.md b/docs/guides/building-an-app.md index 97bfc569..082e30f0 100644 --- a/docs/guides/building-an-app.md +++ b/docs/guides/building-an-app.md @@ -101,12 +101,12 @@ just works across a cold start, and it works whether or not the call changes state. You write nothing; the key is on the request and the deduplication is in `serve()`. -The one visible edge, again from `curl`: a request without an `Idempotency-Key` -header is answered with `400`. The generated client always sends one, so this -only bites a hand-rolled request. If a handler needs a stronger guarantee than -one instance's memory — surviving a crash mid-call — its optional third -argument carries the same key, to write into its own transaction; most -handlers never need it. +The deduplication rides on the key, so a request sent *without* one — a `curl`, +any hand-rolled request — simply isn't deduplicated: it runs once, which is what +you'd expect. The generated client always sends a key, so your service-to-service +calls always get it. If a handler needs a stronger guarantee than one instance's +memory — surviving a crash mid-call — its optional third argument carries the key +to write into its own transaction; most handlers never need it. Two limits worth knowing: diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index 7d25dd30..ce83f1b4 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -332,8 +332,8 @@ details. Two more things the generated client does for you, both invisible in your code: every call carries an idempotency key and retries safely if the target was still cold-starting, and the provider deduplicates on that key so a retry -never runs your handler twice. (A hand-rolled request without the key is -answered `400` — the client always sends one.) +never runs your handler twice. (A hand-rolled request without a key still +works — it just isn't deduplicated.) [Building an app](building-an-app.md#calls-retry-safely-for-you) has these too. Re-deploying is idempotent — it updates the same Project. For an isolated diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts index 9e4d27c1..0052702b 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts @@ -60,12 +60,12 @@ test('the exhaustive, correctly-typed handler map is accepted', () => { }); }); -test('ctx.idempotencyKey is typed on the three-argument handler form', () => { +test('ctx.idempotencyKey is typed string | undefined on the three-argument handler form', () => { serve(authService, { rpc: { verify: async ({ token }, { db }, ctx) => { - const key: string = ctx.idempotencyKey; - return { ok: token.length > 0 && db.validTokens.length >= 0 && key.length >= 0 }; + const key: string | undefined = ctx.idempotencyKey; + return { ok: token.length > 0 && db.validTokens.length >= 0 && key !== undefined }; }, }, }); diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts index b61433d4..e7cef7a9 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts @@ -256,25 +256,57 @@ describe('serve()', () => { }); }); -describe('serve() — idempotency key requirement', () => { - test('a request without the Idempotency-Key header is rejected with 400 naming the header', async () => { +describe('serve() — a keyless request opts out of deduplication', () => { + test('a request without the Idempotency-Key header succeeds, running the handler each time (no dedup)', async () => { + let handlerCalls = 0; const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); - const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); - const res = await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: null })); + const req = () => verifyRequest({ token: 'good-token' }, { idempotencyKey: null }); + const first = await handler(req()); + const second = await handler(req()); - expect(res.status).toBe(400); - const body = (await res.json()) as { error: string }; - expect(body.error).toContain('Idempotency-Key'); + expect(first.status).toBe(200); + expect(await first.json()).toEqual({ ok: true }); + // Two keyless requests each ran the handler — no replay, because the caller + // sent no key to deduplicate on. + expect(handlerCalls).toBe(2); + expect(second.status).toBe(200); }); - test('an empty Idempotency-Key header is treated as missing', async () => { + test('an empty Idempotency-Key header is treated as no key (opts out, does not error)', async () => { const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); const res = await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: '' })); - expect(res.status).toBe(400); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + test('the handler sees ctx.idempotencyKey === undefined for a keyless call, and the key value for a keyed one', async () => { + const seen: Array = []; + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { + rpc: { + verify: async (_input, _deps, ctx) => { + seen.push(ctx.idempotencyKey); + return { ok: true }; + }, + }, + }); + + await handler(verifyRequest({ token: 't' }, { idempotencyKey: null })); + await handler(verifyRequest({ token: 't' }, { idempotencyKey: 'k-123' })); + + expect(seen).toEqual([undefined, 'k-123']); }); }); diff --git a/packages/0-framework/2-authoring/service-rpc/src/client.ts b/packages/0-framework/2-authoring/service-rpc/src/client.ts index 351677b5..7a6d8237 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/client.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/client.ts @@ -1,17 +1,10 @@ /** - * The RPC kind's network adapter — the client `rpc(contract)` hydrates to. - * POSTs JSON to `/rpc/` for each contract method read off - * `contract.__cmp`. Every request carries an `Idempotency-Key` header: one - * `crypto.randomUUID()` minted per logical call and reused byte-identically - * across every retry of that call — never shared between two separate - * calls. A thrown network error, a 429, or any 5xx is retried with a - * bounded, jittered backoff; any other 4xx is not, since a malformed or - * unauthorized request stays wrong on a second try. `serve()` already - * validates a handler's output against the method schema before responding, - * so the client trusts that and does not re-validate — both ends of every - * edge are framework-provisioned. When a `serviceKey` is supplied - * (ADR-0030), every request also carries `Authorization: Bearer - * `. + * The client `rpc(contract)` hydrates to. POSTs JSON to `/rpc/` + * per contract method. Each call carries one `Idempotency-Key` + * (`crypto.randomUUID()`, reused across that call's retries) and, when a + * `serviceKey` is given, `Authorization: Bearer`. A thrown error, a 429, or a + * 5xx is retried; any other 4xx is not. `serve()` validates the response, so + * the client does not re-validate. */ import type { Contract } from '@internal/core'; @@ -25,30 +18,7 @@ import type { Client, RpcFns } from './rpc.ts'; */ export type Transport = (req: Request) => Promise; -/** - * The Prisma Compute ingress can close a service's first-touch connection - * while a scale-to-zero target boots (PRO-217), so every call is retried - * with a bounded, jittered exponential backoff — permanent protocol - * semantics for this kind, not a compensation that goes away once that - * platform behavior is fixed. These numbers mirror the streams module's - * IDEMPOTENT_BACKOFF; they are reimplemented here rather than imported - * because service-rpc is framework-layer and must not depend on - * prisma-cloud. `maxRetries` counts retries after the first attempt, so a - * persistently failing call sends 6 requests in total before giving up. - * - * Why 6 attempts covers a boot that can take up to ~22s even though the - * backoff sums to only ~8s: the close is intermittent. On a cold start the - * ingress usually HOLDS the connection and the request simply blocks until - * the server is listening (there is no client-side request timeout here), so - * a held attempt rides out the whole boot and succeeds on its own — the - * backoff budget only governs the case where an attempt is actively closed, - * and every close so far has been the fast (~400ms) kind. Spending six - * attempts against that means the call fails only if it is closed six times - * in a row, which the observed close rate makes rare. `maxDelayMs` is the - * ceiling the doubling would be clamped to; at maxRetries:5 the last wait is - * 4s, so the clamp never actually binds — it is kept so raising maxRetries - * stays safe. - */ +/** Bounded jittered backoff for retrying a dropped call. `maxRetries` is retries after the first attempt. */ const RETRY = { initialDelayMs: 250, multiplier: 2, @@ -86,11 +56,9 @@ function methodUrl(base: string, method: string): string { } /** - * Sends one logical call over `send`, retrying a thrown error, a 429, or a - * 5xx with full-jitter backoff (a random wait between 0 and the current - * delay, which then grows by `RETRY.multiplier` up to `RETRY.maxDelayMs`). - * `buildRequest` is called fresh for each attempt but always carries the - * same idempotency key — only the transport call is repeated, not the key. + * Sends one call over `send`, retrying a thrown error, 429, or 5xx with + * full-jitter backoff. `buildRequest` runs per attempt but carries the same + * idempotency key each time — only the transport call repeats, not the key. */ async function callWithRetry( send: Transport, diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index 9662f7e2ad374fe53b5b20aa14e66f23782b44e8..be6179731a303224b3f6bb152752f6317eb58519 100644 GIT binary patch delta 1560 zcmaJ>O^YN&5M`WQbp~}*7#2k&@SxL6f1rrUw8PM&2#YT2hzE6%&aR5?PNyof%*^WU zf`wYW_ybDtC+Jb=n^$k%JnGStf55vhvTA15i*>56eDUJNi}!8)Yx(m}yY++|e_gT< zlqlyaZ5E^qP4A?f7dEg-s=@nQO-Xx7IW3D!Cn|7C){qW#Q_AJ>!ze6*sGjLCC++$3?9{j z)9ibFvW}*uNqMdfr&F5aSzXeEQd!0h{i*N}89Q$a59#)3pMa%OYqF}V+WfZnbo2M4 zPi+6(o7{Ys&O6a(Hm!hH44W4Z=xrQItV?}A_71VFO8uB#rMvo!W)_4zX!y*o@n@VG zy2?4Xt{OJ~9L#Fq;zU$FB15gfo6sjUvo6SR3x!cnbG13E0{ba}^-S&{LnzT#6^Tc= zr-?OiKp13f7GW7Z(6&DCWdSE}+9z+RG{Kb~GZ}8_nb#kw=^dLF8OxBA@oRRTmYk(g zM8`#@Dw;7AW%}U!Q{t6Qg0^Nz@99m4D9$O^4GL#h7@C7Z30}byQ3L?DO}9D)r{;5= z3_QpQiL*nq^Onr_+OQFSLRbDJcwMhVk6fn%-f&V12NrV!?@S9T5AWI2mgLqO%HIVN0~HoyQ~~n4E|*Bv>`WD>T>& zg)ga|&)LbQw8sQ$w4Y=O7QI}`=8NY(|5CJrS8&z}Kysxy&Wef0bXOImjb{uV+uHsM z(jsoAJ1QXE{UTZbNel+XkrLS5GssNNwE%~-5lA`?!$4!?KT739pn>$ zhRx=UkDsFZ_hFN6oyObm-uz*w7K09az@FrEXY?VlbJiVGuKYz@?948}^8yQMDzadW z;_#z0D#LoU7YNRk&PsSm0s^LsCPz@T!9iZ1U5LIwMi4Td00z@K0MTGD=+O-|ak&Xz_!~&P>RH>%#~$qY=&r7M@71gMrTTjE^G^$t z?L&5Ba{RnvA1JTzM;UJ^Q4Y^lL7jUk=cNr?B;(c&k2}<7m9nEhUVl%D20S+G{UTjI z38FnNzv3itN<$s`ShcaXwn9((EXyf%`j9=9>{|H&hrUglvR&@uYfh_@+t`KwMkgvT zAsKzBiyjU8+Asw~%CtkSi86+8pby#@iNYgJs(_`_4=%qqQGvY5`S__VnkXrq4=sC6 zqN^;RFh%R5(dx!%gtdN%2aZdl#-4Pb?l^Eur(6UiX3Ur}D;v7DDpF1*SkO7I&|_vG z_wFflkArpGp)MOcJbHch$A!0^_K}^MHqKo;7tz-2r}xk8DJe5C8b>VQ$kPZBJ2@`& zQLwQ+$)E;6s1QtIt%u4~Otv77)Y<|!8FCj^f|FAg&VPJ#^Zczh7m)At+Y2;ll27`a zs}}a^6o98#n^>3nCKWaSYr|3-_^>MVk@_37tG6hzIZ_I$3n&tc13f?!Js~wYx#lav zBXuE3i8Hj|u9Yaylyh!B5v7sG0&wdA5cb!4Od!C%H7VJW3#mzB>cR(AB)qaX{qsWi zZ3(y!NqXvFTW85n_by(USS+9%nBz=Qajr&N;Lbz7Dymf_p^6N48KIFd4#~0_#(>*A zR#^y$6dd|O^6w20+X9VZ5g-8X7C1p41 z!TuL5KGd-TBM@0-j5J_OQsUskvDXt(G60>0+TXh^K}u&2Av`A+37Q-7uu(vfM*)z6 zjYUBjBTn%?mp)WE+5k0XkySUASWba+(JV;lIaC@QAvl#~I%!Yn#@gB%Aqf#?M4VBl zs3AtY|M<~EG*>#N(o~VQuC;>!b17_y?B@lTiO5lmqKt>sM@u?jxwF-zAtSv~!7yeD z1dmDO+@vnQl^NaZhuC3BUBjHtq#$Ik!MrP=YoK%jQ7jQ+*U(382%b?0BnklM=^N@I zwZj#xUtQ!;iJn2~#nCsHI%m3bO+M#hXN2MoFe#85{qWfK z{%MA?55(r$Ln$CuVt}0#(j`|FkktsifqoX{ND`;A{Fg5_R3X7?vDQwZAgkcU@B_F` z6|j|;@ksphlK+o;PCJ*^CpTfV$oTkFNoTf`l!T7~@fr|E-8pX6CJzIk>#{<;RrR8% z0TU?ij-fyqEHyYysj-l!b4Ux>QvU>UF%VT%N+>~awiV_E)@=bKq6uVRkhb8-c8GqN zQ>@f11)>TG;o%nW78lJ$7l|rsG(M60VJZFWJ67spt!-*#15bBF<<#lX<>eBAq=#pXv#bE#klm;e&T=wg6{ z(mAn_LF+7Z#77FtF;Hac9{u*t`K#0S*B`Vc2NH}Lb4>1F>E`Ap6)-t^`Es7*<5x>d z)7`bJaNF5$Ca}f}V&|-DQ)gFu89>1Hd8NfRu6^|4-kt5;y+=>BAKpHAwzGZk?C$oh zJKOuOuFbxHI`pb3UpC8_0`a$8r}t_x-lM(c10{^wv|lE7(P2m5t-n5V0DZ`GxwrbL zKAdu$w&MJ0D48@+j35Zg;m{mnMnZzv;Bp%}@Pm^&fTPS9d<+1l>FUOn31v!~>JmtDc7<7l+SYCq8Ps8l9FHtIs ypM@I^-Vt;116{_Tf`c`@5|Nfc4h>-b Date: Tue, 21 Jul 2026 11:10:36 +0200 Subject: [PATCH 17/18] docs(service-rpc): trim the comments flagged in review Remove the idempotency paragraph from serve()'s module header (the IdempotencyStore doc and the dispatch code already carry it) and tighten client.ts's header to its essentials. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../2-authoring/service-rpc/src/client.ts | 10 ++++------ .../2-authoring/service-rpc/src/serve.ts | Bin 15043 -> 14819 bytes 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/0-framework/2-authoring/service-rpc/src/client.ts b/packages/0-framework/2-authoring/service-rpc/src/client.ts index 7a6d8237..031ea689 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/client.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/client.ts @@ -1,10 +1,8 @@ /** - * The client `rpc(contract)` hydrates to. POSTs JSON to `/rpc/` - * per contract method. Each call carries one `Idempotency-Key` - * (`crypto.randomUUID()`, reused across that call's retries) and, when a - * `serviceKey` is given, `Authorization: Bearer`. A thrown error, a 429, or a - * 5xx is retried; any other 4xx is not. `serve()` validates the response, so - * the client does not re-validate. + * The typed client `rpc(contract)` hydrates to: POSTs JSON to + * `/rpc/` per method, carrying an idempotency key (and the + * service key when given) and retrying per `callWithRetry`. It does not + * re-validate the response — `serve()` already did. */ import type { Contract } from '@internal/core'; diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index be6179731a303224b3f6bb152752f6317eb58519..d08ea271ef7f51ef72a95b59a89af8438f8371ff 100644 GIT binary patch delta 12 TcmX?H`nY(*66VbC`FSTk$ppU8CXzyTBvAaMl8&~Xv7n@E)J z<>mX{$3MgUmaSXVqbWHY0z8ux&P8oDBn;(qg?(iiM#jMcF(>UwL4~T+QZN)*Lz=1M zftvEaU$S+`yA$F9=hwKsvIfKC*&&wjzMlV-32H2-xP`2t@6)BG7Q&OQ8cX1GN0AFA TRYsyuN6m#zb@S$N&1R!7p4m$} From 808a8cec17abc404c066865d889294d0a0569d09 Mon Sep 17 00:00:00 2001 From: willbot Date: Tue, 21 Jul 2026 11:21:42 +0200 Subject: [PATCH 18/18] fix(service-rpc): the LRU key separator was a literal NUL byte MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit serve.ts embedded a raw NUL (0x00) as the method/key separator in the idempotency store's LRU key. A NUL makes git classify the whole file as binary, so GitHub rendered "Binary file not shown" and hid the entire diff. Written as the `\0` escape instead: identical runtime separator (still collision-proof — neither a method name nor a UUID contains NUL), text source. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../2-authoring/service-rpc/src/serve.ts | Bin 14819 -> 14820 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index d08ea271ef7f51ef72a95b59a89af8438f8371ff..d1deab4ffae71fa61c4eba8943e5ed919de5c628 100644 GIT binary patch delta 15 WcmaD{{G@n8y%bZ7!R7|3WO)EQbOuxa delta 14 VcmaD-{J3~Sy%ZzE=0>Sxc>pxl1;_vZ