fix(porch-store): never replay a cached transient 5xx

- Miss path only marks a response a durable replay target (outcome 1)
  when status is 2xx/3xx; a 4xx/5xx gets outcome 3 instead
- Outcome 3's row is a one-shot relay: the scalar reply still can't carry
  a Resp (WO-E226), so the row exists only to hand the exact response
  back once, then idempotent.wo deletes it -- a retry with the same key
  is a genuine miss and re-executes, instead of caching a 500 for the
  24h default TTL
- Reviewer finding: caching any status meant a transient failure was
  replayed verbatim until TTL expiry, worse than no idempotency at all
- Gate leg 18d: FlakyHandler fails once then succeeds; same key twice
  must answer 500 then 200 -- confirmed failing (500, 500) before the
  fix, passing after

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit e61015f2065a7c6aec6f5c2439e78b53f796bab3)

docs/examples/porch/middleware/idempotent.wo

@@ -51,7 +51,8 @@ pub class Idempotent {
       return r;
     }
 
-    if raw / 1_000_000_000 == 2 {
+    let outcome = raw / 1_000_000_000;
+    if outcome == 2 {
       -- Same key, a different request: refuse rather than serve the
       -- other request's response.
       let r = Resp { status: 422, headers: {}, body: "{\"error\":\"idempotency key reused with a different request\"}" };
@@ -59,11 +60,13 @@ pub class Idempotent {
       return r;
     }
 
-    -- Stored (fresh miss or matched replay): the actor already committed
-    -- this row before returning, so it is there to read.
+    -- outcome 1 (2xx/3xx, a durable replay target) or 3 (4xx/5xx, a
+    -- one-read relay only): either way the actor already committed this
+    -- row before returning, so it is there to read right now.
     let hits = from k in IdempotencyKey where k.key == key take 1 select k;
     if len(hits) == 0 { return server_error(); }
-    let stored = json.decode(hits[0].response) as IdempotentStoredResp;
+    let row = hits[0];
+    let stored = json.decode(row.response) as IdempotentStoredResp;
     if stored == nil { return server_error(); }
     -- `.. ""` forces a fresh, independently-owned Text for every key/value
     -- copied out of the decoded record: json.decode's Text values do not
@@ -72,7 +75,15 @@ pub class Idempotent {
     -- otherwise) — concat is documented to always allocate new owned text.
     let hdrs: map<Text, Text> = {};
     for k, v in stored.headers { hdrs[k .. ""] = v .. ""; }
-    return Resp { status: stored.status, headers: hdrs, body: stored.body .. "" };
+    let result = Resp { status: stored.status, headers: hdrs, body: stored.body .. "" };
+    if outcome == 3 {
+      -- Never a replay target: delete now so the next attempt with this
+      -- key (an immediate retry, most likely) is a genuine miss and
+      -- re-executes the handler, instead of caching a transient failure
+      -- for the TTL.
+      delete row;
+    }
+    return result;
   }
 }
 

docs/examples/porch/middleware/keypool.wo

@@ -128,14 +128,24 @@ class KeyActor {
       -- unguarded, same as kind 1's own insert: this actor is the only
       -- writer for this key (messages are processed one at a time), so a
       -- @unique violation here would mean something is genuinely wrong,
-      -- not a race to paper over. A swallowed failure would answer
-      -- outcome 1 (stored) for a response that was never actually
+      -- not a race to paper over. A swallowed failure would answer a
+      -- stored/ephemeral outcome for a response that was never actually
       -- written -- let it trap instead, so a saturated-looking 503 is
       -- what the middleware answers, never a false success.
       insert IdempotencyKey {
         key: msg.key, response: stored_json, created_at: now, digest: msg.digest
       };
-      return pool_pack(1, 0);
+      if resp.status >= 200 and resp.status < 400 {
+        -- 2xx/3xx: a real answer worth replaying for the TTL.
+        return pool_pack(1, 0);
+      }
+      -- 4xx/5xx: the row above exists only so the scalar-only reply can
+      -- still hand the caller its exact response (WO-E226 -- a Resp
+      -- cannot ride the mailbox). It must NOT survive to answer a later
+      -- retry: caching a transient 500 for the TTL (default 24h) would
+      -- make every retry fail until it expires, worse than no idempotency
+      -- at all. idempotent.wo reads this row once and deletes it.
+      return pool_pack(3, 0);
     }
 
     -- kind 1: count.
@@ -235,10 +245,12 @@ pub fn pool_count(pool: Pool, key: Text, limit: Int, window: Int) -> Verdict {
 }
 
 -- The begin accessor idempotent.wo calls: hides pool_select/call the same
--- way pool_count does. Returns the raw packed outcome — 1 means the
--- response is now in IdempotencyKey (fresh store or matched replay), 2
--- means a digest mismatch (422, nothing to read). Never 0: idempotent.wo's
--- own `try ... catch (e) nil` cannot tell a literal 0 reply apart from a
+-- way pool_count does. Returns the raw packed outcome — 1 means a 2xx/3xx
+-- response is durably in IdempotencyKey (fresh store or matched replay);
+-- 2 means a digest mismatch (422, nothing to read); 3 means a 4xx/5xx
+-- miss whose row is a one-read-then-delete relay only, never a replay
+-- target (see the kind-2 arm above). Never 0: idempotent.wo's own
+-- `try ... catch (e) nil` cannot tell a literal 0 reply apart from a
 -- trapped call, so the encoding avoids it on purpose. A trapped call (a
 -- saturated mailbox) propagates to the caller uncaught, same as
 -- pool_count -- the middleware's own try/catch answers 503.

scripts/web-app-accept.sh

@@ -778,10 +778,27 @@ class ExecCount {
   }
 }
 
+@table(name: "flaky_marks")
+class FlakyMark {
+  n: Int
+}
+
+-- reviewer finding, task 4 follow-up: a transient 5xx must not be cached
+-- for the TTL -- fails on the first call, succeeds on every call after.
+class FlakyHandler {
+  fn handle(req: Req) -> Resp {
+    let n = len(from f in FlakyMark select f);
+    insert FlakyMark { n: 1 };
+    if n == 0 { return server_error(); }
+    return ok_json("{\"ok\":true}");
+  }
+}
+
 fn build_app(slot: actor PoolMsg) -> App {
   let app = App { middleware: [], routes: [] };
   let p = Pool { actors: [PoolSlot { a: slot }] };
   app.post("/create", Idempotent { key_header: "idempotency-key", pool: p, inner: SlowHandler {} });
+  app.post("/flaky", Idempotent { key_header: "idempotency-key", pool: p, inner: FlakyHandler {} });
   app.get("/execs", ExecCount {});
   return app;
 }
@@ -905,6 +922,22 @@ if ip_out="$("$WOC" --emit "$IP" -o "$IP/idempotent_check.wob" 2>&1)"; then
     && ok "idempotent concurrency: exactly one execution despite 2 parallel duplicates (ExecMark row count = 3)" \
     || bad "idempotent-cc-execs" "ExecMark count=$ec want 3"
 
+  # ---- 18d. gate leg: a transient 5xx is never replayed (reviewer finding) --
+  # The miss path must persist only a 2xx/3xx response. FlakyHandler fails
+  # on its first-ever call and succeeds on every call after; hit twice with
+  # the SAME idempotency key, the answer must be 500 then 200 -- caching the
+  # 500 would make every retry fail for the rest of the TTL (default 24h),
+  # a worse outcome than no idempotency at all.
+  f1="$(curl -s -o "$W/i11a.body" -w '%{http_code}' --max-time 5 -X POST \
+    -H "Host: a" -H "Idempotency-Key: leg11-key" -H "Content-Type: text/plain" \
+    --data-binary "x" "http://127.0.0.1:$IPORT/flaky")"
+  f2="$(curl -s -o "$W/i11b.body" -w '%{http_code}' --max-time 5 -X POST \
+    -H "Host: a" -H "Idempotency-Key: leg11-key" -H "Content-Type: text/plain" \
+    --data-binary "x" "http://127.0.0.1:$IPORT/flaky")"
+  [[ "$f1" == "500" && "$f2" == "200" ]] \
+    && ok "idempotent: a transient 5xx is not replayed -- retry re-executes (500 then 200)" \
+    || bad "idempotent-5xx-not-cached" "first=$f1 second=$f2 want 500 then 200"
+
   kill -TERM "$SRV" 2>/dev/null
   for _ in $(seq 1 30); do kill -0 "$SRV" 2>/dev/null || break; sleep 0.1; done
   SRV=""