test: cover the gaps a sibling plugin's live-test inventory exposed

Their framing is the useful part: a suite that fakes the transport agrees
with itself by construction, and every defect they found survived a green
suite. Working through their inventory against ours.

Three gaps were real and are now closed.

The published artifact was never loaded. `npm pack` honours the `files`
field, so a file needed at runtime that nobody listed is invisible to
every other test here — they all read the source tree, where it is
present. `npm run test:package` packs, extracts OUTSIDE the repo (inside,
the working tree shadows the install and the test passes without loading
what shipped), boots a real Gateway from the extracted package, and
requires 8 tools and a verified delivery. It passes: 104K, no test
directory, nothing missing.

A burst past the concurrency cap was untested. Admission defers rather
than queues and a deferred event is held nowhere, so the surviving burst
is maxConcurrent x the retry count. Now pinned: six concurrent deliveries
against a cap of two admit exactly two, defer four with the configured
Retry-After, and record a ledger row for none of the deferred ones —
recording one would make Hookdeck's redelivery look like a duplicate and
the event would vanish.

A deliberate operator retry was untested. Admission is attempt-count
based and does not read the trigger, which is right — the trigger arrives
in an unsigned header, so honouring it would let anyone able to replay a
body bypass deduplication. What matters is that Hookdeck's own increment
admits a MANUAL retry, including of an event that already succeeded. All
three cases now have tests.

Added from their finding about provider verification: a source's type
does not enable it, and a source with a secret is byte-identical to one
without over the API — so `hookdeck_doctor` now reads `verified` on
recent inbound requests, which is the only evidence there is.

Already covered here: malformed and non-ASCII signatures (their 500 case
— ours refuses cleanly, checked last round), duplicate suppression,
crash-recovery orphan reconciliation, and driving the tools with a real
model, which is how this plugin's tool surface was found broken twice.

659 tests.

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

Author: garethx

README.md

@@ -146,7 +146,8 @@ The README covers the common path. Everything else lives in [`docs/`](docs/):
 
 ```bash
 openclaw plugins install --link ./hookdeck-openclaw
-npm test   # no Gateway or Hookdeck account required
+npm test           # no Gateway or Hookdeck account required
+npm run test:package   # loads the packed tarball in a real Gateway
 ```
 
 ## Learn more

docs/agent-tools.md

@@ -93,3 +93,12 @@ A status tool that returns a page and lets the reader infer a total is worse tha
 - `hookdeck_status.openIssues` and `hookdeck_issues`' `total` come from Hookdeck's count endpoint, not from the length of a page.
 - `hookdeck_recent_deliveries` returns `openIssuesTotal` beside the page it shows, and an `openIssuesTruncated` note whenever the two differ. The local records get the same treatment via `localTruncated`.
 - `hookdeck_status.deadLetters` is the local log's true size, but the log evicts oldest-first at its cap — so once it is full, `deadLettersIsAtLeast: true` says the number is a floor. A floor reported as a floor beats a ceiling reported as a total.
+
+## What `hookdeck_doctor` checks
+
+Beyond the obvious config validation:
+
+- **Provider verification is actually in force.** Setting a source's *type* to STRIPE or GITHUB does not enable signature verification — the provider's signing secret has to be set on the source as well. A source with one is byte-identical to a source without it over the API, because the secret is never returned. So the only evidence is whether the requests that arrived were verified, and that is what this check reads.
+- **The retry rule still covers every status the plugin emits.** A rule narrower than the emitted codes turns admission control into silent data loss.
+- **The CLI and the API key point at the same project.** See [Transport](transport.md#the-two-projects-problem).
+- **The burst each route can absorb**, from `maxConcurrent` and the connection's retry count.

package.json

@@ -47,7 +47,8 @@
     "test:watch": "vitest --exclude \"test/live/**\"",
     "typecheck": "tsc --noEmit",
     "test:live": "vitest run test/live",
-    "test:agent": "bash scripts/agent-smoke.sh"
+    "test:agent": "bash scripts/agent-smoke.sh",
+    "test:package": "bash scripts/test-package.sh"
   },
   "homepage": "https://github.com/hookdeck/hookdeck-openclaw#readme",
   "bugs": {

scripts/test-package.sh

@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+# Loads the PACKAGED plugin in a real Gateway, not the working tree.
+#
+# `npm pack` honours the `files` field, so a file the plugin needs at runtime
+# but nobody listed is invisible to every other test in this repo — they all
+# read the source directory, where it is present.
+#
+# Everything runs from OUTSIDE the repo. Run it from inside and the working
+# tree shadows the extracted package, and the test passes without ever loading
+# what was shipped.
+set -uo pipefail
+cd "$(dirname "$0")/.."
+REPO="$(pwd)"
+
+ROOT="${TMPDIR:-/tmp}/hookdeck-openclaw-package"
+rm -rf "$ROOT"; mkdir -p "$ROOT/state"
+
+echo "==> packing"
+TARBALL="$(npm pack --silent --pack-destination "$ROOT")" || exit 1
+echo "    $TARBALL"
+
+echo "==> extracting outside the repo"
+tar -xzf "$ROOT/$TARBALL" -C "$ROOT"
+PKG="$ROOT/package"
+
+# The runtime dependencies are not in the tarball; link the ones the repo
+# already resolved rather than hitting the network.
+mkdir -p "$PKG/node_modules"
+for dep in typebox zod openclaw; do
+  [ -d "$REPO/node_modules/$dep" ] && ln -sfn "$REPO/node_modules/$dep" "$PKG/node_modules/$dep"
+done
+
+echo "==> what shipped"
+( cd "$PKG" && find . -name node_modules -prune -o -type f -print | sed 's|^\./||' | sort )
+
+for required in openclaw.plugin.json index.ts LICENSE README.md; do
+  if [ ! -f "$PKG/$required" ]; then
+    echo "MISSING from the package: $required"
+    exit 1
+  fi
+done
+
+if [ -d "$PKG/test" ]; then
+  echo "The test suite shipped to users. Check the files field."
+  exit 1
+fi
+
+cat > "$ROOT/openclaw.json" <<JSON
+{
+  "gateway": { "mode": "local", "bind": "loopback", "port": 18851 },
+  "plugins": {
+    "load": { "paths": ["$PKG"] },
+    "entries": { "hookdeck": { "enabled": true, "config": {
+      "signingSecret": "whsec_package_test",
+      "ingress": { "basePath": "/hookdeck" },
+      "routes": { "stripe": { "source": "stripe",
+        "dispatch": { "mode": "wake", "sessionKey": "main" } } }
+    } } }
+  }
+}
+JSON
+
+echo "==> booting a Gateway from the extracted package"
+cd "$ROOT"   # outside the repo, so nothing shadows the install
+OPENCLAW_CONFIG_PATH="$ROOT/openclaw.json" OPENCLAW_STATE_DIR="$ROOT/state" \
+  "$REPO/node_modules/.bin/openclaw" gateway --allow-unconfigured > "$ROOT/gw.log" 2>&1 &
+GW=$!
+trap 'kill $GW 2>/dev/null' EXIT
+sleep 14
+
+fail() { echo "FAIL: $1"; echo "--- log ---"; tail -20 "$ROOT/gw.log"; exit 1; }
+
+grep -q "ingress ready" "$ROOT/gw.log" || fail "the packaged plugin did not start"
+TOOLS=$(grep -o "declared [0-9]* tool(s)" "$ROOT/gw.log" | head -1 | grep -o "[0-9]*")
+[ "${TOOLS:-0}" = "8" ] || fail "expected 8 tools from the package, saw '${TOOLS:-none}'"
+
+BODY='{"type":"invoice.paid"}'
+SIG=$(printf '%s' "$BODY" | openssl dgst -sha256 -hmac 'whsec_package_test' -binary | base64)
+CODE=$(curl -s -o /dev/null -w "%{http_code}" -X POST http://127.0.0.1:18851/hookdeck/stripe \
+  -H 'content-type: application/json' -H "x-hookdeck-signature: $SIG" \
+  -H 'x-hookdeck-eventid: evt_pkg' -H 'x-hookdeck-attempt-count: 1' --data "$BODY")
+[ "$CODE" = "200" ] || fail "a signed delivery to the packaged plugin answered $CODE"
+
+echo
+echo "PASS: the packaged plugin loads, declares 8 tools, and verifies a signed delivery."
+echo "      Size: $(du -h "$ROOT/$TARBALL" | cut -f1)"

src/hookdeck/client.ts

@@ -216,6 +216,27 @@ export interface HookdeckClient {
    * endpoint takes `{cause, webhook_id, transformation_id}` with no date
    * filter, and there is no project-wide listing of ignored events.
    */
+  /**
+   * Recent inbound requests, for the one question the API cannot answer
+   * directly: whether a source is really verifying its provider's signatures.
+   *
+   * A source with a provider secret set is byte-identical to one without it
+   * over the API — the secret is never returned — so `verified` on the requests
+   * that actually arrived is the only signal there is.
+   */
+  listRequests(params?: {
+    limit?: number;
+    sourceId?: string;
+  }): Promise<
+    ApiResult<
+      {
+        id: string;
+        verified?: boolean | null;
+        rejection_cause?: string | null;
+      }[]
+    >
+  >;
+
   bulkReplayRequests(params: {
     query: Record<string, unknown>;
     target: { webhook_ids?: string[]; source_id?: string };
@@ -460,6 +481,20 @@ export function createHookdeckClient(
       return result.ok ? { ok: true, data: result.data.count ?? 0 } : result;
     },
 
+    async listRequests(params = {}) {
+      const query = new URLSearchParams({ limit: String(params.limit ?? 10) });
+      if (params.sourceId !== undefined)
+        query.set("source_id", params.sourceId);
+      const result = await request<{
+        models?: {
+          id: string;
+          verified?: boolean | null;
+          rejection_cause?: string | null;
+        }[];
+      }>("GET", `/requests?${query}`);
+      return result.ok ? { ok: true, data: result.data.models ?? [] } : result;
+    },
+
     async bulkReplayRequests(params) {
       // `target` goes INSIDE `query`, and is required there. Sending it at the
       // top level is answered `422 query.target is required` — the whole call

src/tools/doctor.ts

@@ -65,6 +65,30 @@ export async function doctorHandler(deps: ToolDeps) {
     });
   }
 
+  // Whether provider verification is actually in force. Setting a source's
+  // TYPE to STRIPE or GITHUB does not enable it — the provider's secret has to
+  // be set as well — and a source with one is indistinguishable from a source
+  // without it over the API, because the secret is never returned. So the only
+  // evidence is whether the requests that arrived were verified.
+  if (deps.client !== undefined) {
+    const requests = await deps.client.listRequests({ limit: 20 });
+    if (requests.ok && requests.data.length > 0) {
+      const unverified = requests.data.filter(
+        (r) => r.verified === false,
+      ).length;
+      checks.push({
+        name: "provider verification",
+        ok: unverified === 0,
+        detail:
+          unverified === 0
+            ? `the last ${requests.data.length} inbound request(s) were verified by Hookdeck`
+            : `${unverified} of the last ${requests.data.length} inbound request(s) arrived UNVERIFIED. ` +
+              `A source's type does not enable verification on its own — the provider's signing secret ` +
+              `has to be set on the source too, or anyone who learns the URL can post to it.`,
+      });
+    }
+  }
+
   // In `cli` transport, "which project" has two independent answers:
   // provisioning acts on the API key's project, while `hookdeck listen` looks
   // for that connection in whichever project the CLI's session points at. When

test/handler.test.ts

@@ -1187,3 +1187,116 @@ describe("form-encoded providers", () => {
     expect(dispatch).toHaveBeenCalledOnce();
   });
 });
+
+describe("a deliberate operator retry is not swallowed as a duplicate", () => {
+  // Admission is attempt-count based and does not special-case the trigger:
+  // the trigger arrives in an unsigned header, so honouring it would let
+  // anyone who can replay a body bypass deduplication. Hookdeck increments the
+  // attempt count on a manual retry, which is what admits it.
+  it("admits a MANUAL retry that advances the attempt count", async () => {
+    const { deps, dispatch } = harness();
+    await handleDelivery(deps, request({ attemptCount: "1" }));
+
+    const manual = await handleDelivery(
+      deps,
+      request({
+        attemptCount: "2",
+        extraHeaders: { "x-hookdeck-attempt-trigger": "MANUAL" },
+      }),
+    );
+
+    expect(manual.plan.status).toBe(200);
+    expect(manual.plan.code).toBe("dispatched");
+    expect(dispatch).toHaveBeenCalledTimes(2);
+  });
+
+  it("admits a manual retry of an event that already succeeded", async () => {
+    // Hookdeck allows retrying a successful event, and an operator doing so
+    // means it. This is also what makes Hookdeck usable as the work queue for
+    // interrupted runs.
+    const { deps, dispatch, ledger } = harness();
+    await handleDelivery(deps, request({ attemptCount: "1" }));
+    expect(ledger.get("evt_1")?.status).toBe("succeeded");
+
+    await handleDelivery(
+      deps,
+      request({
+        attemptCount: "2",
+        extraHeaders: { "x-hookdeck-attempt-trigger": "MANUAL" },
+      }),
+    );
+    expect(dispatch).toHaveBeenCalledTimes(2);
+  });
+
+  it("still suppresses a replayed attempt that does not advance", async () => {
+    // The trigger header is unsigned, so it cannot be the thing that decides.
+    const { deps, dispatch } = harness();
+    await handleDelivery(deps, request({ attemptCount: "2" }));
+
+    const replayed = await handleDelivery(
+      deps,
+      request({
+        attemptCount: "2",
+        extraHeaders: { "x-hookdeck-attempt-trigger": "MANUAL" },
+      }),
+    );
+    expect(replayed.plan.code).toBe("duplicate");
+    expect(dispatch).toHaveBeenCalledOnce();
+  });
+});
+
+describe("a burst past the concurrency cap", () => {
+  // Admission defers rather than queues, and a deferred event is held nowhere:
+  // it only returns when Hookdeck retries it. So the burst that survives is
+  // maxConcurrent x the connection's retry count, and anything beyond that is
+  // lost. This pins the local half of that arithmetic.
+  it("admits exactly maxConcurrent and defers the rest, recording nothing for them", async () => {
+    let release!: () => void;
+    const blocked = new Promise<void>((r) => {
+      release = r;
+    });
+
+    const { deps, ledger } = harness({
+      config: buildConfig({ maxConcurrent: 2 }),
+      dispatch: async () => {
+        await blocked;
+        return { settle: "succeeded" as const, plan: okPlan("dispatched") };
+      },
+    });
+
+    const inFlight = Array.from({ length: 6 }, (_, i) =>
+      handleDelivery(deps, request({ eventId: `evt_${i}` })),
+    );
+    // Let the first two claim their slots before the rest are judged.
+    await new Promise((r) => setTimeout(r, 10));
+
+    release();
+    const results = await Promise.all(inFlight);
+
+    const deferred = results.filter((r) => r.plan.status === 503);
+    expect(deferred).toHaveLength(4);
+    expect(results.filter((r) => r.plan.status === 200)).toHaveLength(2);
+
+    // Nothing is recorded for a deferred event: a ledger row would make
+    // Hookdeck's redelivery look like a duplicate, and the event would vanish.
+    for (let i = 0; i < 6; i += 1) {
+      const row = ledger.get(`evt_${i}`);
+      if (row !== undefined) expect(row.status).toBe("succeeded");
+    }
+    expect(ledger.stats().entries).toBe(2);
+  });
+
+  it("tells a deferred sender when to come back", async () => {
+    const { deps } = harness({
+      config: buildConfig({ maxConcurrent: 1, busyRetryAfterSeconds: 7 }),
+      dispatch: () => new Promise(() => ({})) as never,
+    });
+
+    void handleDelivery(deps, request({ eventId: "evt_a" }));
+    await new Promise((r) => setTimeout(r, 10));
+    const second = await handleDelivery(deps, request({ eventId: "evt_b" }));
+
+    expect(second.plan.status).toBe(503);
+    expect(second.plan.retry).toEqual({ kind: "after", seconds: 7 });
+  });
+});

test/manager.test.ts

@@ -60,6 +60,10 @@ function fakeClient(overrides: Partial<HookdeckClient> = {}): HookdeckClient {
       ok: true as const,
       data: [{ id: "web_1", team_id: "tm_a" }],
     })),
+    listRequests: vi.fn(async () => ({
+      ok: true as const,
+      data: [{ id: "req_1", verified: true }],
+    })),
     getIssue: vi.fn(async (id: string) => ({
       ok: true as const,
       data: { id },

test/tools.test.ts

@@ -117,6 +117,10 @@ function fakeClient(overrides: Partial<HookdeckClient> = {}): HookdeckClient {
       ok: true as const,
       data: [{ id: "web_1", team_id: "tm_a" }],
     })),
+    listRequests: vi.fn(async () => ({
+      ok: true as const,
+      data: [{ id: "req_1", verified: true }],
+    })),
     ...overrides,
   };
 }
@@ -1664,3 +1668,48 @@ describe("bulk replay is always scoped to a configured route", () => {
     expect(d.client!.bulkReplayRequests).not.toHaveBeenCalled();
   });
 });
+
+describe("doctor checks that provider verification is actually in force", () => {
+  // A source's TYPE does not enable verification — the provider's secret must
+  // be set too — and a source with one is byte-identical to one without over
+  // the API. Whether arriving requests were verified is the only evidence.
+  it("fails when requests are arriving unverified", async () => {
+    const d = await deps({
+      client: fakeClient({
+        listRequests: vi.fn(async () => ({
+          ok: true as const,
+          data: [
+            { id: "req_1", verified: false },
+            { id: "req_2", verified: true },
+            { id: "req_3", verified: false },
+          ],
+        })),
+      }),
+    });
+
+    const check = (await doctorHandler(d)).checks.find(
+      (c) => c.name === "provider verification",
+    );
+    expect(check?.ok).toBe(false);
+    expect(check?.detail).toMatch(/2 of the last 3/);
+    expect(check?.detail).toMatch(/anyone who learns the URL can post to it/);
+  });
+
+  it("passes when they are all verified", async () => {
+    const check = (await doctorHandler(await deps())).checks.find(
+      (c) => c.name === "provider verification",
+    );
+    expect(check?.ok).toBe(true);
+  });
+
+  it("says nothing rather than guessing when no request has arrived yet", async () => {
+    const d = await deps({
+      client: fakeClient({
+        listRequests: vi.fn(async () => ({ ok: true as const, data: [] })),
+      }),
+    });
+    expect(
+      (await doctorHandler(d)).checks.find((c) => c.name === "provider verification"),
+    ).toBeUndefined();
+  });
+});