Skip to content

Fido posts duplicate replies to review thread comments (closes #566)#662

Merged
FidoCanCode merged 2 commits into
mainfrom
duplicate-review-replies
Apr 17, 2026
Merged

Fido posts duplicate replies to review thread comments (closes #566)#662
FidoCanCode merged 2 commits into
mainfrom
duplicate-review-replies

Conversation

@FidoCanCode
Copy link
Copy Markdown
Owner

@FidoCanCode FidoCanCode commented Apr 17, 2026

Fixes #566.

Closes the TOCTOU race on _replied_comments by claiming comment IDs atomically before posting, and wires the claimed set into handle_threads filtering so the worker's comments sub-agent won't re-reply to threads the webhook handler already handled.


Work queue

Completed (2)
  • Atomic claim-before-reply to close TOCTOU on _replied_comments
  • Filter handle_threads by webhook-claimed comment IDs

…#566)

Replace the module-level set[int] with RepliedComments — a thread-safe
class with an atomic claim() that checks-and-adds under a lock in one
operation.  The claim now happens *before* calling reply_to_comment() /
reply_to_issue_comment(), not after: concurrent webhook deliveries for
the same comment can no longer both pass the membership check before
either adds the ID.

On failure, release() clears the claim so a GitHub redelivery can retry.
The already_replied parameter on reply_to_review() is removed since that
function is a stub that never used it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@FidoCanCode FidoCanCode force-pushed the duplicate-review-replies branch from 86870e3 to 6ff6942 Compare April 17, 2026 12:57
Move RepliedComments to kennel/claimed.py as a shared module so both the
webhook handler (server.py) and the worker (worker.py) can consult the
same process-wide claimed set without a circular import.

_filter_threads now skips threads whose first_db_id appears in the
webhook-claimed set.  The last_author == gh_user filter already handles
threads where the reply landed before handle_threads fetches the thread
list, but it has an inherent race with GitHub API propagation.  The
in-process claimed-ID check is deterministic and fires immediately when
the webhook handler claims a comment — even when the reply is still in
flight.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@FidoCanCode FidoCanCode marked this pull request as ready for review April 17, 2026 13:06
@FidoCanCode FidoCanCode requested a review from rhencke April 17, 2026 13:06
@FidoCanCode FidoCanCode merged commit 5ceac18 into main Apr 17, 2026
3 checks passed
@FidoCanCode FidoCanCode deleted the duplicate-review-replies branch April 17, 2026 15:11
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.

Fido posts duplicate replies to review thread comments

2 participants