Skip to content

feat(webhooks): accepted PR events ride the durable signal queue - #614

Merged
alari76 merged 1 commit into
mainfrom
feat/webhook-signals
Aug 30, 2026
Merged

feat(webhooks): accepted PR events ride the durable signal queue#614
alari76 merged 1 commit into
mainfrom
feat/webhook-signals

Conversation

@alari76

@alari76 alari76 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

The last follow-up from the durable-signals work (#605): accepted PR webhook events now flow through the at-least-once queue, so a crash between the 202 response and the review session spawning delays the review instead of losing it.

Design — surgical, not a rewrite

  • The filter chain stays inline (signature, action, draft, actor allowlist, dedup, session cap): GitHub still gets its synchronous filter/duplicate/429 responses, and none of the existing response shapes change.
  • On acceptance, the handler enqueues a pr-review signal — deduped on the existing PR idempotency key, 1 h TTL — carrying the payload, event record, and the pre-allocated session id.
  • processQueuedPrReview consumes it with three explicit outcomes: existing session → no-op (redelivery after a mid-spawn crash), malformed payload → dropped with a log (retry can't fix it), transient failure (gh hiccup before any session exists) → rethrown so the queue retries (3 attempts, then ledgered as failed).
  • Graceful fallback: without a wired publisher (tests, engine unavailable, enqueue failure) the legacy fire-and-forget path runs unchanged — the migration is strictly additive.

Testing

4 new tests: enqueue-on-accept (with dedupe key + no inline spawn), publisher-failure fallback, redelivery no-op, malformed-signal drop. Full suite: 3070 pass; build + lint clean.

🤖 Generated with Claude Code

Closes the last lossy event path: a crash between the webhook 202 and the
review session spawning no longer loses the PR event.

- The filter chain (signature, action, draft, allowlist, dedup, cap) stays
  inline — response semantics unchanged. On acceptance the handler
  enqueues a pr-review signal (deduped on the existing idempotency key,
  1h TTL) instead of fire-and-forgetting processPrReviewAsync
- processQueuedPrReview consumes from the queue: redelivery-safe via the
  pre-allocated session id (existing session = spawn already happened),
  malformed signals dropped with a log, transient failures rethrown so
  the queue retries them (3 attempts)
- Publisher injected by ws-server; without one (tests, engine down or
  enqueue failure) the legacy inline path runs unchanged

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alari76
alari76 merged commit 974140d into main Aug 30, 2026
2 checks passed
@alari76
alari76 deleted the feat/webhook-signals branch August 30, 2026 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant