feat(dev): make the dev harness able to actually deliver a webhook - #183

src/dev/webhook-worker.test.ts (3)

107-133: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Assert the retry with a deterministic clock or a bounded poll.

The test sleeps 120 ms and then asserts on calls and errors. Passes 2 and onward each perform a real outbound delivery attempt, so pass timing depends on network behavior. Replace the fixed sleep with a poll that waits until calls > 1, so a loaded CI runner cannot fail the test.

♻️ Proposed change

-    await new Promise((resolve) => setTimeout(resolve, 120))
+    await vi.waitFor(() => {
+      expect(calls).toBeGreaterThan(1)
+    })

Add vi to the vitest import if you apply this.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/dev/webhook-worker.test.ts` around lines 107 - 133, Replace the fixed 120
ms sleep in the “keeps looping after a pass throws” test with a bounded poll
that waits until calls exceeds 1, using the existing Vitest utilities and adding
vi to the vitest import if needed. Keep the errors length assertion and enforce
a timeout so the test remains deterministic without waiting on outbound delivery
timing.

---

97-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This test does not exercise the in-flight wait.

stop() runs before the first timer fires, so no pass ever starts and inFlight is still the initial resolved promise. The test verifies idempotent stop() only, not the title's "leaving no pass in flight". Add a wait longer than intervalMs before stop(), or start a pass with runOnce() and stop concurrently, to cover the wait path.

💚 Proposed addition

     worker = startWebhookWorker(h, { intervalMs: 10 })
+    // Let at least one pass start, so `stop()` actually awaits in-flight work.
+    await new Promise((resolve) => setTimeout(resolve, 30))
     await worker.stop()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/dev/webhook-worker.test.ts` around lines 97 - 105, Update the “stops
cleanly, leaving no pass in flight” test to ensure a webhook pass is active
before calling stop, either by waiting longer than the configured intervalMs or
by starting runOnce() concurrently with worker.stop(). Preserve the existing
second-stop assertion so the test continues covering idempotent shutdown as well
as the in-flight wait path.

---

70-85: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Avoid real outbound requests in webhook worker tests.

This test uses module.example.com, so delivery still performs a real DNS lookup. The route rejects 127.0.0.1 for the SSRF guard, so use the existing HTTP transport seam: expose requestImpl in startWebhookWorker(), wire createWebhookDeliveryHandler({ ..., send: { requestImpl?: ... } }), and pass a stub from this factory.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/dev/webhook-worker.test.ts` around lines 70 - 85, Update
startWebhookWorker and its delivery-handler setup to accept an optional
requestImpl and pass it through createWebhookDeliveryHandler’s send
configuration. In the test’s harness/factory, provide a stub request
implementation so the queued webhook delivery avoids DNS and outbound network
access while preserving the existing dispatched and failed assertions.

src/dev/webhook-worker.ts (1)

58-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Type handlers to the queue drain contract and drop the cast.

PostgresQueue.drainOnce accepts handlers: Record<string, QueueMessageHandler<unknown>>, so the handler map can be typed by import as DrainDeps['handlers'] and passed directly. The broad Record<string, ...> cast only broadens the type instead of strengthening it.

♻️ Proposed refactor

-  const handlers = {
+  import type { DrainDeps } from '../providers/adapters/postgres-queue/index.js'
+  const handlers: DrainDeps['handlers'] = {
     [WEBHOOK_DELIVERY_TOPIC]: createWebhookDeliveryHandler({
       webhookEndpoints: deps.webhookEndpoints,
     }),
   }
@@
-    const delivery = await deps.queue.drainOnce({
-      handlers: handlers as Record<
-        string,
-        Parameters<PostgresQueue['drainOnce']>[0]['handlers'][string]
-      >,
-    })
+    const delivery = await deps.queue.drainOnce({ handlers })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/dev/webhook-worker.ts` around lines 58 - 79, Type the handlers map in the
worker using the queue drain contract, such as DrainDeps['handlers'], and remove
the broad Record<string, ...> cast from the deps.queue.drainOnce call. Import
the contract type from the existing queue module and keep the current webhook
delivery handler mapping unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes