Deduplicate chat mentions handled by webhook and poller #50

New issue

Closed

opened 2026-05-03 08:35:52 -07:00 by jwilger · 0 comments

jwilger commented

2026-05-03 08:35:52 -07:00

Owner

Problem

A single top-level PR comment containing @auto_review re-review can queue two reviews.

Observed on PR #48: commenting @auto_review re-review caused both the issue_comment webhook path and the background chat poller to process the same top-level PR comment. Because re-review queues with force=true, orchestrator same-SHA review-history dedup did not suppress the duplicate jobs.

Likely cause

The poller was intended as a fallback for inline review-thread mentions because Forgejo does not reliably fire pull_request_review_comment webhooks. However, it currently lists PR conversation comments via repos/{owner}/{repo}/issues/{n}/comments, which overlaps with the issue_comment webhook path for top-level PR comments.

Current overlap:

ar-gateway::webhook::handle_issue_comment handles freshly-created top-level PR comments.
ar-gateway::poller::ChatPoller also scans known PR comments for new @<bot_name> mentions.
The two paths do not share a processed-comment cursor or dedup key.

Desired behavior

Each user-authored chat mention should be processed at most once, regardless of whether it is seen through a webhook, the poller, or both.

Acceptance criteria

Add regression coverage for a top-level PR comment with @auto_review re-review that is seen by both paths but dispatches only one review job.
Preserve the poller fallback for comments that are not reliably delivered by webhook, especially inline review-thread replies.
Bot-authored comments still never trigger chat handling.
Dedup should be keyed by stable Forgejo comment identity, e.g. (owner, repo, pr_number, comment_id), or otherwise advance/share the poller cursor when webhook comments are handled.
force=true re-review jobs must not bypass chat-command dedup for the same comment.
Add or update metrics/docs if the fix changes poller behavior or operator expectations.

Notes

Disabling the poller via AR_POLL_INTERVAL_SECS=0 avoids duplicate top-level comment handling but loses inline-thread mention fallback, so it is only a workaround.

## Problem A single top-level PR comment containing `@auto_review re-review` can queue two reviews. Observed on PR #48: commenting `@auto_review re-review` caused both the `issue_comment` webhook path and the background chat poller to process the same top-level PR comment. Because `re-review` queues with `force=true`, orchestrator same-SHA review-history dedup did not suppress the duplicate jobs. ## Likely cause The poller was intended as a fallback for inline review-thread mentions because Forgejo does not reliably fire `pull_request_review_comment` webhooks. However, it currently lists PR conversation comments via `repos/{owner}/{repo}/issues/{n}/comments`, which overlaps with the `issue_comment` webhook path for top-level PR comments. Current overlap: - `ar-gateway::webhook::handle_issue_comment` handles freshly-created top-level PR comments. - `ar-gateway::poller::ChatPoller` also scans known PR comments for new `@<bot_name>` mentions. - The two paths do not share a processed-comment cursor or dedup key. ## Desired behavior Each user-authored chat mention should be processed at most once, regardless of whether it is seen through a webhook, the poller, or both. ## Acceptance criteria - Add regression coverage for a top-level PR comment with `@auto_review re-review` that is seen by both paths but dispatches only one review job. - Preserve the poller fallback for comments that are not reliably delivered by webhook, especially inline review-thread replies. - Bot-authored comments still never trigger chat handling. - Dedup should be keyed by stable Forgejo comment identity, e.g. `(owner, repo, pr_number, comment_id)`, or otherwise advance/share the poller cursor when webhook comments are handled. - `force=true` re-review jobs must not bypass chat-command dedup for the same comment. - Add or update metrics/docs if the fix changes poller behavior or operator expectations. ## Notes Disabling the poller via `AR_POLL_INTERVAL_SECS=0` avoids duplicate top-level comment handling but loses inline-thread mention fallback, so it is only a workaround.

jwilger referenced this issue from a pull request that will close it,

2026-05-03 15:51:19 -07:00

fix(gateway): deduplicate chat mentions #62

jwilger closed this issue

2026-05-03 17:41:46 -07:00

jwilger referenced this issue from a commit

2026-05-03 17:41:47 -07:00

fix(gateway): deduplicate chat mentions (#62)