fix: reply to and resolve rejected review threads (and prevent reply loop) - #163
Open
cwbcheng wants to merge 2 commits into
Open
fix: reply to and resolve rejected review threads (and prevent reply loop)#163cwbcheng wants to merge 2 commits into
cwbcheng wants to merge 2 commits into
Conversation
Rejected review-thread feedback previously fell through the GitHub follow-up path silently: needsGitHubFollowUp only considered accepted items, so a rejected review comment was never replied to and its conversation was never resolved, contradicting the agent prompt that says to reply with the rejection reason and resolve the conversation. Now rejected review threads (excluding PatchDeck's own status/audit-trail replies) are collected as follow-up tasks, get a 'Rejected - no code change made' reply with the reason, and the conversation is resolved. Audit-trail verification skips the code-change audit for these items and only requires thread resolution, and the final status update keeps the rejection decision while advancing the thread-resolved flag.
The internal-reply exclusion in needsGitHubFollowUp only matched 'PatchDeck status comment' and 'Automation audit trail follow-up', but classifyNonActionableAppFeedback also produces 'PatchDeck audit trail comment' and 'PatchDeck agent command comment'. A rejected item carrying one of those reasons was treated as real reviewer feedback, which made PatchDeck reply to its own follow-up comment, producing a new audit trail, which was then replied to again - an infinite loop. Widen the exclusion to all PatchDeck-authored marker reasons and add a regression test that a rejected audit-trail comment never triggers a follow-up reply.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two related bugs in the review-thread lifecycle:
Rejected review threads are silently dropped.
needsGitHubFollowUp()returnsfalsefor any item whosedecision !== "accept", so a review comment that the agent evaluates as reject is never replied to and its conversation is never resolved on GitHub. This contradicts the code-owner agent prompt, which explicitly says: "For rejected feedback: Reply directly to the GitHub comment/thread with what was done, or why it was rejected. Resolve the GitHub conversation after replying." The thread stays open forever with no feedback.PatchDeck replies to its own audit-trail comments, looping forever. Once rejected threads started getting follow-up replies, the exclusion for PatchDeck's own comments only matched
"PatchDeck status comment"and"Automation audit trail follow-up", missing"PatchDeck audit trail comment"and"PatchDeck agent command comment". A rejected reply that PatchDeck itself posted was treated as real reviewer feedback, producing another follow-up reply, which produced another audit trail, which was replied to again — an infinite loop (we observed ~5 duplicate replies stacking on a single thread).Changes
server/babysitter.tsneedsGitHubFollowUp()now includes rejected review-thread items (excluding PatchDeck's own status/audit-trail/agent-command replies), so they get a closing reply + conversation resolution.collectAuditTrailErrors()skips the code-change audit requirement for rejected review threads (there is no code change to audit); only thread resolution is verified.Rejected — no code change made. **Reason:** …), and the item keeps itsrejectedstatus while the thread-resolved flag advances.server/github.ts— no change (exclusion lives in babysitter).Tests
babysitPR replies to and resolves rejected review threads— a rejected review thread triggers a follow-up withresolve: trueand the thread is resolved.babysitPR does not re-reply to its own rejected audit-trail comments— a rejected item whosestatusReasonisPatchDeck audit trail commentnever triggers a follow-up (regression guard for the loop).Environment