Skip to content

fix: idempotency guard on pull_request.opened (Bug #13)#52

Open
avrabe wants to merge 1 commit intomainfrom
fix/pr-opened-idempotency
Open

fix: idempotency guard on pull_request.opened (Bug #13)#52
avrabe wants to merge 1 commit intomainfrom
fix/pr-opened-idempotency

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented May 2, 2026

Summary

Why

Without this, Probot's webhook retry (~5 attempts on 5xx/timeout) replays `enablePullRequestAutoMerge` (spammy), the AI review (only catches retries within the 5-min `_reviewTimestamps` window), and the dependabot enqueue (creates duplicate task-store entries with the same key).

Test plan

  • 2 new tests covering the dup-skip and mark-on-success paths
  • `npm test` → 836 pass (was 834)
  • `npm run lint` → clean

Follow-up

`pull_request.synchronize` and `.reopened` (added in PR #50) need the same guard. Folding that in once #50 lands rather than producing a merge conflict here.

🤖 Generated with Claude Code

Why: QA bug-hunter flagged that pull_request.opened had no
isProcessed/markProcessed gate. Probot retries the webhook ~5 times on
5xx/timeout, replaying enablePullRequestAutoMerge (idempotent on
GitHub's side, but spammy in logs), the AI review (only protected by
the 5-min _reviewTimestamps throttle inside reviewPullRequest), and
the dependabot generation enqueue (creates duplicate task-store
entries with the same key — wasteful and confusing).

What:
  - Add `if (deliveryId && isProcessed(deliveryId)) return;` at handler
    entry, mirroring the pattern already used by repository.created,
    issues.opened, issue_comment.created, and pull_request.closed.
  - Add `if (deliveryId) markProcessed(deliveryId);` at handler exit so
    subsequent retries hit the dedup gate.

Test plan:
  - 2 new tests:
    - duplicate delivery short-circuits before reviewPullRequest
    - successful delivery records markProcessed
  - npm test → 836 pass (was 834)
  - npm run lint → clean

Risk: low — same pattern as four sibling handlers.

Note: pull_request.synchronize and pull_request.reopened (added in
PR #50) will need the same guard. Will fold in once #50 lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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