fix(pull-request skill): reply and resolve all threads including non-actionable#3573
Open
PierreBrisorgueil wants to merge 2 commits intomasterfrom
Open
fix(pull-request skill): reply and resolve all threads including non-actionable#3573PierreBrisorgueil wants to merge 2 commits intomasterfrom
PierreBrisorgueil wants to merge 2 commits intomasterfrom
Conversation
…actionable Informational/false-positive threads must be explicitly replied to and resolved before the stop condition is met — not silently skipped. - Loop procedure: add step 6 for non-actionable open threads (reply, resolve) - Stop condition: zero open threads (not just zero actionable)
📝 WalkthroughWalkthroughThe pull-request skill docs were updated to require replying to and resolving non-actionable/informational review threads, added an explicit loop step to handle those replies/resolutions, and changed the stop condition to wait for zero unresolved threads before checking branch protection and stopping. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Monitor as PR Monitor
participant GH as GitHub API (GraphQL/Threads)
participant CI as Branch Protection / CI
Monitor->>GH: Fetch review threads
GH-->>Monitor: Return threads (actionable + non-actionable)
alt non-actionable/unresolved exist
Monitor->>GH: Post brief reply to each non-actionable thread
Monitor->>GH: Resolve threads via GraphQL
Monitor-->>Monitor: GOTO fetch threads (loop)
else zero unresolved threads
Monitor->>CI: Check branch protection / CI status
CI-->>Monitor: Return status
Monitor->>Monitor: Stop if checks pass
end
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/skills/pull-request/SKILL.md (2)
239-242:⚠️ Potential issue | 🟡 MinorStop condition text contradicts the loop update.
Section 6f still says “zero unresolved actionable threads,” but the loop now stops on “zero open threads.” Please align this stop condition with the new policy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/pull-request/SKILL.md around lines 239 - 242, Update the stop-condition wording in the "6f. Stop condition" section: replace the phrase “zero unresolved actionable threads” with “zero open threads” (or otherwise mirror the loop's stop condition) so the text matches the new policy; ensure the paragraph under the "6f. Stop condition" heading and any nearby references use the exact same term (“zero open threads”) as the loop update.
3-3:⚠️ Potential issue | 🟡 MinorAlign description with new “zero open threads” stop condition.
Line 3 still says “zero actionable threads,” which conflicts with the updated loop logic. Update the description to “zero open threads” to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/pull-request/SKILL.md at line 3, Replace the phrase "zero actionable threads" with "zero open threads" in the PR lifecycle description so it matches the updated loop logic; edit the description line in SKILL.md (the "Full PR lifecycle — branch, commit, issue, draft PR, CI, ready, autonomous monitor loop (fix comments, resolve threads, iterate until CI green and zero actionable threads)." string) to read "...iterate until CI green and zero open threads."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.claude/skills/pull-request/SKILL.md:
- Around line 239-242: Update the stop-condition wording in the "6f. Stop
condition" section: replace the phrase “zero unresolved actionable threads” with
“zero open threads” (or otherwise mirror the loop's stop condition) so the text
matches the new policy; ensure the paragraph under the "6f. Stop condition"
heading and any nearby references use the exact same term (“zero open threads”)
as the loop update.
- Line 3: Replace the phrase "zero actionable threads" with "zero open threads"
in the PR lifecycle description so it matches the updated loop logic; edit the
description line in SKILL.md (the "Full PR lifecycle — branch, commit, issue,
draft PR, CI, ready, autonomous monitor loop (fix comments, resolve threads,
iterate until CI green and zero actionable threads)." string) to read
"...iterate until CI green and zero open threads."
There was a problem hiding this comment.
Pull request overview
This PR fixes a workflow issue in the pull-request skill where non-actionable review threads (e.g., informational comments, approvals, false positives) were being skipped instead of replied to and resolved, leaving open threads on merged PRs. The solution updates the autonomous monitor loop to require handling ALL thread types and changes the stop condition from "zero actionable comments" to "zero open threads."
Changes:
- Added new step 6 in the monitor loop to handle non-actionable threads by replying and resolving them
- Renumbered the stop condition from step 6 to step 7 and updated it to check for "zero open threads" instead of "zero actionable comments"
- Updated the "Informational" category definition to clarify that these threads must be replied to and resolved (not skipped)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3573 +/- ##
=======================================
Coverage 95.93% 95.93%
=======================================
Files 20 20
Lines 516 516
Branches 140 140
=======================================
Hits 495 495
Misses 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tency - Terminology: 'open threads' → 'unresolved threads' throughout loop procedure - Step 6: add 'all' to match step 5 explicitness (reply all, resolve all) - Informational section: reformat as bullet list, clarify review threads vs PR-level comments
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/skills/pull-request/SKILL.md (2)
3-3:⚠️ Potential issue | 🟠 MajorAlign summary description with new stop condition (“zero unresolved threads”).
The description still says “zero actionable threads,” which conflicts with the updated loop. Update to “zero unresolved threads” to reflect the new behavior.
Proposed fix
-description: Full PR lifecycle — branch, commit, issue, draft PR, CI, ready, autonomous monitor loop (fix comments, resolve threads, iterate until CI green and zero actionable threads). +description: Full PR lifecycle — branch, commit, issue, draft PR, CI, ready, autonomous monitor loop (fix comments, resolve threads, iterate until CI green and zero unresolved threads).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/pull-request/SKILL.md at line 3, Update the description line in SKILL.md to match the new stop condition by replacing the phrase "zero actionable threads" with "zero unresolved threads" (edit the value for the description key shown in the diff); ensure the single-line summary now reads "...CI, ready, autonomous monitor loop (fix comments, resolve threads, iterate until CI green and zero unresolved threads)."
244-245:⚠️ Potential issue | 🟠 MajorStop condition still references “unresolved actionable” instead of “unresolved.”
Section 6f contradicts the new loop logic by limiting to “unresolved actionable threads.” This can cause the loop to stop while non-actionable unresolved threads remain. Update to “zero unresolved threads.”
Proposed fix
-All CI checks pass **and** a complete polling pass (after the grace period) produces **zero unresolved actionable threads**. +All CI checks pass **and** a complete polling pass (after the grace period) produces **zero unresolved threads**.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/pull-request/SKILL.md around lines 244 - 245, Update the stop condition text in Section 6f to match the loop logic by replacing the phrase "zero unresolved actionable threads" with "zero unresolved threads" so the stop condition no longer excludes non-actionable unresolved threads; locate the Section 6f wording (search for "unresolved actionable threads" or the stop condition sentence) and change it to explicitly state "zero unresolved threads."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.claude/skills/pull-request/SKILL.md:
- Line 3: Update the description line in SKILL.md to match the new stop
condition by replacing the phrase "zero actionable threads" with "zero
unresolved threads" (edit the value for the description key shown in the diff);
ensure the single-line summary now reads "...CI, ready, autonomous monitor loop
(fix comments, resolve threads, iterate until CI green and zero unresolved
threads)."
- Around line 244-245: Update the stop condition text in Section 6f to match the
loop logic by replacing the phrase "zero unresolved actionable threads" with
"zero unresolved threads" so the stop condition no longer excludes
non-actionable unresolved threads; locate the Section 6f wording (search for
"unresolved actionable threads" or the stop condition sentence) and change it to
explicitly state "zero unresolved threads."
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.
Summary
.claude/skills/pull-request/SKILL.md— loop step 6 now requires replying to and resolving all open threads (including non-actionable/informational ones); stop condition updated to zero open threads.Scope
.claude/skills/pull-request/SKILL.md(skill definition only)nonelowValidation
npm run lintnpm run test:unitnpm run buildGuardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers
Summary by CodeRabbit