serve maps every statusless failure to 502, launders downstream 4xx to the caller, and silently discards a body that matches no slot

[rejifald]

Scenario: webhook-ack-deadline
Proofs: docs/scenarios/proofs/webhook-ack-deadline/ (8 scripts, 154 checks)

Found while measuring the receiving side of a webhook. serve is a control-plane front door
(POST /stitch/:name), not a webhook endpoint, and most of that scenario is out of its scope — the
findings below are the ones that are not, and §§1–3 apply to any caller at all.

First, what works

• StitchStore.increment is a genuine atomic first-one-wins. Two simultaneous deliveries of one
  id ran the work 2× under get-then-set and 1× under increment; the get-then-set race
  reproduced 50/50 on unforced trials, so it is not a barrier artefact. increment is required
  on the interface and the shipped Redis / deno-kv / cloudflare-kv adapters implement it, so the
  atomicity survives leaving one process. This is the load-bearing primitive for the whole scenario.
• The body cap is the right control for an unauthenticated POST target — 2 MiB, tunable,
  enforced before anything buffers (body: '8kb' → 413 on 16 KB, 200 on 1 KB).
• close() drains honestly — it does not abort the in-flight run, the caller gets its true
  status, and a new request during the drain is refused at connect rather than lied to.

1. One line decides the caller's next move, and it is wrong in both directions

serve.ts:192-193:

const status = failure.status && failure.status >= 400 ? failure.status : 502;

Measured through a real node:http caller implementing retry-on-5xx/timeout, stop-on-2xx/4xx:

cause	HTTP returned	caller attempts
output violation / input violation / thrown TypeError / cancellation / timeout	502 ×5	4, 4, 4, 4, 4
downstream 401 / 403 / 404 / 429	passed through	1, 1, 1, 1
downstream 500 / 503	500 / 503	4, 4

Upward: every statusless failure — including permanently-bad input that will fail identically
forever — becomes 502, the one class that means "come back". Scenario 51 measured that status is
undefined for every non-HTTP failure, so this is the whole category.

Downward: a downstream 401/403/404 is passed straight to the caller, which stops after one
attempt. An expired credential on the server's side is reported to the client as "you were
unauthorized" — a diagnosis of the wrong party, and for a client that will not retry a 4xx, the work
is simply lost.

And inconsistently: the output-violation case answers HTTP 502 while the JSON body it
writes reports "status": 200 — correctly, since the upstream did answer 200 and validation failed
afterwards. The envelope and its contents disagree.

Ask: distinguish the three kinds. A validation/contract failure is the server's own permanent
error and should be a 500 at most, arguably a 422 — never 502, which specifically claims an
upstream is at fault. A downstream 4xx is this server's upstream problem and should be a 502,
which is exactly the inverse of the current mapping. A cancellation is neither.

2. A request body that matches no slot is discarded silently, with a 200

A JSON body POSTed to a registered /stitch/:name is read as a StitchInput envelope. If its
top-level keys match no slot (body, query, params, headers, …), every field is dropped and
the stitch runs with body: undefined — measured with a realistic payload: HTTP 200, 0 error
events, 0 warnings. The only surviving copy is the trace start frame.

This is not webhook-specific: any caller that posts {"customerId": 7} instead of
{"body":{"customerId":7}} gets a 200 and a run against nothing.

Ask: reject or warn on a body whose top-level keys match no known slot. The slot names are known
at that point, and a 400 naming the expected envelope would turn a silent data loss into a one-line
fix at the call site.

3. close() can hang the process, and there are no signal handlers

With a keep-alive client — which every real caller pool uses — close() was still pending after
the run completed, the client was answered and the socket went idle. It resolved only when the
client destroyed its agent. A SIGTERM handler that awaits handle.close() therefore hangs until
the orchestrator's grace period expires and SIGKILL lands — killing the in-flight work the drain
existed to protect.

serve installs 0 SIGTERM and 0 SIGINT listeners, and ServeHandle is exactly
{ close, port, server, url } — no drain deadline, no force-close. server.closeIdleConnections()
is reachable through the exposed handle.server but is not wired up.

Ask: call closeIdleConnections() inside close(), or accept a drain deadline on it. This is a
known node:http sharp edge and the fix is one line.

4. ?stream=1 changes the response contract, not just its framing

streamSse writes 200 OK (serve.ts:144-148) before iterating, so the status no longer
reflects the outcome: the identical permanently-failing run answered 502 in JSON mode and 200 in
SSE mode, with the failure delivered as an event: error frame inside the 2xx.

It is also not usable as an early acknowledgement, which is the one thing an early 200 would be good
for: a client that takes the 2xx and hangs up triggers the disconnect handler (serve.ts:262-266)
and aborts the run — measured 1 run, 1 abort, 0 completions. That teardown is correct for a
streaming consumer and surprising for anyone reading the 200 as an ack.

Ask: documentation, mainly — say that the SSE status is unconditional and that the outcome is in
the frames. The abort-on-disconnect is right; it just needs to be stated next to the early 200.

5. redactEventForTransport's list is the outbound credential set

It scrubs authorization, proxy-authorization, cookie, set-cookie and x-api-key
(trace.ts:61-67) and deep-scrubs client_secret from a body — but passes stripe-signature,
x-hub-signature-256 and svix-signature through verbatim onto the (unauthenticated) SSE stream.

Fair caveat: this only bites if you forward inbound signature headers into input.headers. It is
worth a line in the redaction docs either way, since the set is presented as "credential headers"
rather than "credential headers a caller sends".

Related, and minor: GET / is unauthenticated and enumerates every registered stitch name.

6. reserve reads like a claim primitive and is spacing

Confirmed three ways: three reserves on one key granted now+0, now+1h, now+2h — a duplicate is
deferred, never rejected; its TTL refreshes on every call (store.ts:82-85) where increment
binds expiry to the first write (store.ts:66-69), so the two verbs give a key opposite lifetimes;
and a first-wins reading of it breaks on clock skew — spacing 1 min, one worker 2 min ahead, and
both callers tested as first. It is also optional on the interface where increment is required.

Ask: the name invites the wrong reading. A sentence on reserve saying it paces rather than
claims, and pointing at increment for first-one-wins, would be enough.

Reproduction

npx tsx docs/scenarios/proofs/webhook-ack-deadline/c1-status-is-a-protocol.ts
npx tsx docs/scenarios/proofs/webhook-ack-deadline/c3-atomic-dedupe.ts

A real node:http sender implementing a retry policy with a full attempt ledger, so "the client
retried" is observed rather than assumed. Held handlers are released by an explicit promise, never a
timer; the one place a real deadline is under test asserts on attempt counts, never on elapsed
time.

One flake, recorded rather than hidden: c7 reported 15/17 on the first full batch run and has
since passed 17/17 ten consecutive times, standalone and in the same ordering. It contains no
timing, sleep or concurrency construct, so the cause is external — most likely socket reuse from
c6, which deliberately leaves a close() pending against a keep-alive agent. None of C7's claims
depend on it.

Which engine

serve.ts, store.ts and trace.ts are byte-identical to origin/main (git diff empty), so
every citation above holds unchanged on both trees. types.ts/engine.ts/stitch.ts have drifted;
the proofs carry both line numbers where they cite those.

Source references (verified against origin/main)

• serve.ts:192-193 — the status mapping
• serve.ts:194-197 — the JSON body { error, status }
• serve.ts:144-148 — streamSse's unconditional writeHead(200, …)
• serve.ts:262-266 — the disconnect handler that aborts the run
• serve.ts:23-40 — ServeOptions = { port, host, body } · :52-57 — ServeHandle
• serve.ts:66 — MAX_REQUEST_BODY_BYTES
• store.ts:61-69 — increment and its first-write expiry
• store.ts:75-88 — reserve(key, spacing, at, ttl) and its refreshing TTL
• store.ts:108-111 — memoryStore().close()
• trace.ts:61-67 — the redaction header set

---

Found while writing scenario 53 ("the webhook you acknowledged before you understood it") for the docs. Every claim is backed by a runnable offline proof in docs/scenarios/proofs/webhook-ack-deadline/.

[rejifald]

Triage — §3's SIGTERM claim understates it, and §1 has a shared root cause.

Re-verified against origin/main @ 44a9fcc. All six findings confirmed live. Two refinements:

§3 — "serve installs 0 SIGTERM listeners" is true of serve.ts and misses the sharper fact.
The shipped CLI wires precisely the hanging pattern at cli.ts:637-641:

const stop = () => handle.close().then(resolve);
process.once('SIGINT', stop);
process.once('SIGTERM', stop);

Combined with serve.ts:305-310's close() — which calls server.close() and never
closeIdleConnections() (zero occurrences anywhere in packages/core/src) — stitch serve under an
orchestrator hangs to SIGKILL against any keep-alive client. So the fix is one line inside close(),
and the caller that needs it already exists.

§1's 502 mapping is not a local serve bug — it shares a root cause with the circuit cluster.
serve.ts:192-193 maps every statusless failure to 502 because nothing upstream classifies a failure
by kind, only by whether it carries an HTTP status. The same gap produces #696 (a transport throw
retried on attempt count alone), #705 (a cancellation counted as a circuit failure), #690 (a
credential 401 opening the breaker) and #702 (a 413 opening it). A shared
classifyFailure(err) → permanent | transient | caller-fault | cancelled, read by the retry loop, the
breaker and serve, fixes all of them; patching serve.ts:192 alone would be a fifth private
guess.

Information flow, stated precisely. The downstream status does reach the untrusted caller
verbatim — serve.ts:193 passes anything >= 400 through unchanged, so a vendor's 401/403/404 is
served to an unauthenticated remote caller as this server's own verdict. The downstream body does
not leak on the default HTTP surface: the message is the constant HTTP ${status}
(surface.ts:149). But downstream-derived text does reach the caller verbatim in two other
spellings, both landing at serve.ts:195 (error: failure.message):

• the graphql surface joins the vendor's own error strings — surface.ts:319
• verdict.flag embeds a JSON-stringified downstream body value — surface.ts:186

On an unauthenticated endpoint that is a real outbound information flow. GET / also enumerates
every mounted stitch unauthenticated (serve.ts:219-224).

§2 meets the P0 bar on its own — HTTP 200 for a run against body: undefined, because
serve.ts:268-271 spreads parsed JSON straight into stitch.stream() and StitchInput
(types.ts:36-54) declares only seven slots, none of which are checked. It is graded P1 here only
because serve is loopback-by-default and documented as a control-plane front door. If serve is
treated as supported ingress, re-grade §2 to P0 — the slot names are already known at serve.ts:246,
so the fix is small.

Posted by an automated triage pass over #677–#709; every claim above was checked at the cited
file:line on origin/main.