fix: round-3 review — deliberate pauses, settle ordering, inert-option honesty

1. A restart lifted deliberate operator pauses. `stop()` stamped
   `pauseReason: "shutdown"` on every configured cursor, including one
   already marked `operator` — so an operator pause survived until the
   Gateway restarted, at which point it was reclassified as a breadcrumb
   and the next tunnel connect resumed a pipeline someone had stopped on
   purpose. The round-2 test only covered a reconnect with no restart in
   between. Operator pauses are now skipped in `stop()`, which also saves
   re-pausing an already-paused connection against the shutdown budget.

2. The new catch path in settleAfterRun used the settle/retry ordering
   that the success path fifty lines above explains is wrong. Same file,
   opposite orderings, each with a comment claiming necessity. It settles
   first now, matching, and corrects to `exhausted` if the redelivery
   could not be requested.

3. The startup warnings claimed to name every inert option and did not.
   Because zod applies defaults, per-field warnings would either fire
   always or never, so there is now one warning per agent route stating
   that turns are fire-and-forget, naming maxAgentRetries and
   syncTimeoutSeconds as never taking effect, saying that a crash mid-run
   is not re-queued, and listing whichever of deliver/lane/sync were set.

Also: a failed operator pause no longer erases an underlying shutdown
breadcrumb, and resuming clears the reason rather than resetting it to
"shutdown"; `transportNote` reaches the caller instead of being computed
and dropped; form and JSON content types are both matched on the parsed
mime rather than one by substring; recovery settles on any permanent 4xx
rather than 404 alone; a disabled route is no longer paused at shutdown,
since in CLI mode nothing would ever resume it; the readiness test
imports the pattern rather than restating it; and the outcome.ts contract
text and the README's shutdown-budget claim now match what the code does.

Tests for what was previously unpinned: recovery settling behaviour
across permanent, transient and successful retries; a rejecting waitFor
after the 202; template bracket indexes; the fire-and-forget warnings;
the deliberate-pause-across-restart case; and a new suite for the
config-error path, which had none — covering the 503 hold, basePath
sanitising, and the status tool that stays registered.

613 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: garethx

README.md

@@ -98,7 +98,7 @@ You do not need to configure destination auth either. CLI destinations default t
 | `busyRetryAfterSeconds` | `10` | `Retry-After` sent when deferring at capacity. |
 | `deferAttemptLimit` | `5` | Deferrals of the same event before the short `Retry-After` is dropped and exponential backoff takes over. Capacity that has not recovered after this many attempts is not the transient condition a short interval assumes. |
 | `pause.onShutdown` | `true` | Pause the connection before stopping the listener, so events are held rather than discarded. |
-| `pause.shutdownTimeoutMs` | `5000` | Budget for the whole teardown: pausing, draining and stopping children. |
+| `pause.shutdownTimeoutMs` | `5000` | Budget for pausing connections at shutdown. Stopping the CLI children is bounded separately, by a SIGTERM grace before SIGKILL. |
 | `catchUp.enabled` | `true` | After a reconnect, replay requests that arrived while nothing was listening. |
 | `catchUp.minGapSeconds` | `30` | Below this, an outage is not worth a bulk replay. |
 | `dedupe.ttlHours` | `168` | Ledger retention, matching Hookdeck's one-week retry ceiling. Raise it if you extend retries beyond a week. |
@@ -358,7 +358,7 @@ npm test
 npm run typecheck
 ```
 
-586 tests, no Gateway or Hookdeck account required. Signature vectors are computed independently with `openssl`, `test/http-integration.test.ts` exercises the pipeline over a real socket including multi-byte UTF-8 and multi-chunk bodies, the store suites inject write failures at an exact call to prove the degradation rule, and `test/store-io.test.ts` runs against a real filesystem because that is the only place durability actually lives.
+613 tests, no Gateway or Hookdeck account required. Signature vectors are computed independently with `openssl`, `test/http-integration.test.ts` exercises the pipeline over a real socket including multi-byte UTF-8 and multi-chunk bodies, the store suites inject write failures at an exact call to prove the degradation rule, and `test/store-io.test.ts` runs against a real filesystem because that is the only place durability actually lives.
 
 ## Shared reliability contract
 

scripts/agent-smoke.sh

@@ -164,8 +164,9 @@ if [ -n "$HOOKDECK_KEY" ]; then
     exit 2
   fi
   if [ -n "$ALLOW_MUTATIONS" ]; then
-    # Scoped to one connection on purpose. "Acknowledge the oldest issue" would
-    # let the model pick anything in the project.
+    # Scoped to one connection on purpose: "acknowledge the oldest issue" would
+    # let the model pick anything in the project. The connection is required at
+    # startup, so it is set by the time we get here.
     ask "Acknowledge the oldest open Hookdeck issue for the $MUTATION_CONNECTION connection, then confirm what changed and what did NOT change."
   else
     # The refusal is the point: the correct answer names tools.allowMutations.

src/dispatch/agent.ts

@@ -173,16 +173,23 @@ export function createAgentDispatcher(
       const message = err instanceof Error ? err.message : String(err);
       deps.logger.warn(`background settle failed for ${eventId}: ${message}`);
 
+      // Settled BEFORE the retry, matching the success path above: Hookdeck
+      // can redeliver within milliseconds, and the redelivery's `begin` opens
+      // the next run's row — which a late settle would stamp `failed`.
+      // `failed` is the optimistic guess; it is corrected to `exhausted` below
+      // if the redelivery could not be requested.
+      await deps.ledger.settle(eventId, "failed").catch(() => {});
+
       const requeued =
         deps.client !== undefined &&
         (await deps.client
           .retryEvent(eventId)
           .then((r) => r.ok)
           .catch(() => false));
 
-      await deps.ledger
-        .settle(eventId, requeued ? "failed" : "exhausted")
-        .catch(() => {});
+      if (!requeued) {
+        await deps.ledger.settle(eventId, "exhausted").catch(() => {});
+      }
 
       if (!requeued) {
         await deps.deadLetter

src/hookdeck/client.ts

@@ -262,10 +262,24 @@ export function createHookdeckClient(
           };
         }
 
+        // `permanent` means a retry of this exact request cannot succeed: the
+        // event is gone, or the request is malformed or unauthorised. Callers
+        // use it to decide whether to keep a row for the next attempt or give
+        // up on it. 429 is excluded deliberately — it is the retryable 4xx.
+        const permanent =
+          response.status >= 400 &&
+          response.status < 500 &&
+          response.status !== 429;
+
         return {
           ok: false,
           status: response.status,
-          code: response.status === 404 ? "not_found" : "api_error",
+          code:
+            response.status === 404
+              ? "not_found"
+              : permanent
+                ? "permanent_error"
+                : "api_error",
           message,
         };
       }

src/ingress/handler.ts

@@ -96,24 +96,28 @@ function parseFormUrlEncoded(body: string): Record<string, unknown> {
   return out;
 }
 
-function contentTypeIsFormEncoded(
+/** The mime type alone, lower-cased, with any parameters dropped. */
+function mimeType(
   headers: Record<string, string | string[] | undefined>,
-): boolean {
+): string | undefined {
   const raw = headers["content-type"];
   const value = Array.isArray(raw) ? raw[0] : raw;
-  return (
-    typeof value === "string" &&
-    value.toLowerCase().includes("application/x-www-form-urlencoded")
-  );
+  if (typeof value !== "string") return undefined;
+  return value.split(";")[0]?.trim().toLowerCase();
+}
+
+function contentTypeIsFormEncoded(
+  headers: Record<string, string | string[] | undefined>,
+): boolean {
+  // The mime, not a substring search over the whole header: a parameter that
+  // happens to contain this string would otherwise reclassify a JSON body.
+  return mimeType(headers) === "application/x-www-form-urlencoded";
 }
 
 function contentTypeIsJson(
   headers: Record<string, string | string[] | undefined>,
 ): boolean {
-  const raw = headers["content-type"];
-  const value = Array.isArray(raw) ? raw[0] : raw;
-  if (typeof value !== "string") return false;
-  const mime = value.split(";", 1)[0]?.trim().toLowerCase() ?? "";
+  const mime = mimeType(headers) ?? "";
   return mime === "application/json" || mime.endsWith("+json");
 }
 

src/plugin/config-parse.ts

@@ -278,31 +278,27 @@ export function parseHookdeckConfig(raw: unknown): ConfigParseResult {
     }
 
     if (route.dispatch.mode === "agent") {
-      // Both are carried in the config and neither reaches the runner: agent
-      // turns go through TaskFlow `run_task`, which takes a goal and nothing
-      // else. Saying so is better than an operator setting `deliver: false` and
-      // believing it is doing something.
-      if (route.dispatch.deliver) {
-        warnings.push({
-          path: `routes.${routeId}.dispatch.deliver`,
-          message:
-            "deliver has no effect on the TaskFlow transport and is not passed to the agent turn; it is recorded for a future transport that can carry it",
-        });
-      }
-      if (route.dispatch.lane !== undefined) {
-        warnings.push({
-          path: `routes.${routeId}.dispatch.lane`,
-          message:
-            "lane has no effect on the TaskFlow transport and is not passed to the agent turn",
-        });
-      }
-      if (route.dispatch.ackMode === "sync") {
-        warnings.push({
-          path: `routes.${routeId}.dispatch.ackMode`,
-          message:
-            "sync needs a completion signal the TaskFlow transport does not provide; this route will behave as async_retry",
-        });
-      }
+      // Agent turns go through TaskFlow `run_task`, which takes a goal and
+      // exposes flow state rather than a completion signal. Several dispatch
+      // options therefore describe behaviour this transport cannot produce.
+      // One warning per route, naming them: silence would let an operator
+      // believe a safety setting was in force.
+      const inert: string[] = [];
+      if (route.dispatch.deliver) inert.push("deliver");
+      if (route.dispatch.lane !== undefined) inert.push("lane");
+      if (route.dispatch.ackMode === "sync") inert.push('ackMode: "sync"');
+
+      warnings.push({
+        path: `routes.${routeId}.dispatch`,
+        message:
+          "agent turns are fire-and-forget on the TaskFlow transport: the delivery is acknowledged as " +
+          "soon as the run STARTS, so nothing waits for it and no run failure is ever observed. " +
+          "maxAgentRetries and syncTimeoutSeconds therefore never take effect, and a crash mid-run is " +
+          "not re-queued by recovery — run durability belongs to the flow record" +
+          (inert.length > 0
+            ? `. ${inert.join(", ")} ${inert.length === 1 ? "is" : "are"} recorded but not passed to the turn`
+            : ""),
+      });
     }
 
     routes[routeId] = {

src/protocol/outcome.ts

@@ -142,8 +142,14 @@ export function retryable(
  * must name a reason from the allowlist, and each emission is logged and
  * counted by the caller.
  *
- * Always dead-letters. If we are telling Hookdeck to stop trying, the payload
- * has to survive locally or it is simply lost.
+ * Dead-letters everything past signature verification. If we are telling
+ * Hookdeck to stop trying, the payload has to survive locally or it is simply
+ * lost.
+ *
+ * Cancellations BEFORE verification — wrong method, wrong content type,
+ * oversized — are deliberately not recorded. The ingress is public and the log
+ * is bounded, so recording unauthenticated traffic would let a scanner evict
+ * the failures that matter.
  */
 export function cancelRetries(
   reason: CancelReason,

src/recovery.ts

@@ -126,23 +126,23 @@ export async function reconcileOrphans(
       attemptCount: row.attempt,
     });
 
-    // A 404 means the event has aged out of Hookdeck's retention: no future
-    // boot will do better, so the row is settled. Anything else may be
-    // transient — a network fault, a rate limit — and is left `running` for the
-    // next boot to retry.
+    // A permanent failure — the event aged out of retention, the request was
+    // rejected — will not go better on the next boot, so the row is settled.
+    // Transient failures (network, rate limit, 5xx) leave it `running` to be
+    // retried then.
     //
-    // Leaving a permanently dead row running would make it an orphan forever:
-    // one duplicate dead-letter per boot, and since orphans are recovered
-    // oldest-first it would consume the budget ahead of events that could
-    // actually be recovered.
-    if (result.code === "not_found") {
-      await ledger.settle(row.eventId, "failed");
-    }
+    // Leaving a permanently dead row running makes it an orphan forever: one
+    // duplicate dead-letter per boot, and since orphans are recovered
+    // oldest-first it consumes the budget ahead of events that could actually
+    // be recovered.
+    const permanent =
+      result.code === "not_found" || result.code === "permanent_error";
+    if (permanent) await ledger.settle(row.eventId, "failed");
 
     logger.warn(
       `could not re-queue interrupted event ${row.eventId}: ${result.message}` +
-        (result.code === "not_found"
-          ? " (aged out of retention; not retried again)"
+        (permanent
+          ? " (permanent; not retried again)"
           : " (will be retried on the next start)"),
     );
   }

src/tools/pause.ts

@@ -76,9 +76,14 @@ export async function pauseHandler(
     });
     const result = await client.pauseConnection(cursor.connectionId);
     if (!result.ok) {
+      // Restore what was there, rather than asserting "not paused": a
+      // shutdown breadcrumb underneath this must survive, or the connection is
+      // left paused with nothing to lift it.
       await deps.cursors.patch(params.routeId, {
-        pausedByUs: false,
-        pauseReason: "shutdown",
+        pausedByUs: cursor.pausedByUs === true,
+        ...(cursor.pauseReason !== undefined
+          ? { pauseReason: cursor.pauseReason }
+          : {}),
       });
       return { ok: false, note: result.message };
     }
@@ -110,10 +115,10 @@ export async function pauseHandler(
   cancelPendingAutoResume(params.routeId);
   const result = await client.unpauseConnection(cursor.connectionId);
   if (!result.ok) return { ok: false, note: result.message };
-  await deps.cursors.patch(params.routeId, {
-    pausedByUs: false,
-    pauseReason: "shutdown",
-  });
+  // Cleared, not reset to a value: "resumed" has no pause reason, and leaving
+  // `shutdown` there would be a breadcrumb for something that already happened.
+  await deps.cursors.patch(params.routeId, { pausedByUs: false });
+  await deps.cursors.clear(params.routeId, "pauseReason");
   return {
     ok: true,
     paused: false,

src/tools/replay.ts

@@ -149,6 +149,7 @@ export async function replayHandler(
     return {
       ok: false,
       dryRun: true,
+      ...(transportNote !== undefined ? { transportNote } : {}),
       note:
         `Would replay requests for route '${params.routeId}' from the last ${params.sinceMinutes} minute(s) ` +
         `that produced no CLI event. Re-run with confirm: true to execute.`,
@@ -175,6 +176,7 @@ export async function replayHandler(
         ...(beyondShortestRetention
           ? { retentionWarning: RETENTION_NOTE }
           : {}),
+        ...(transportNote !== undefined ? { transportNote } : {}),
       }
     : { ok: false, note: `replay did not run: ${result.reason}` };
 }