Skip to content

NO-ISSUE: fix(skills): assess Copilot review state before requesting#7

Merged
hhk7734 merged 6 commits into
mainfrom
fix/copilot-review-loop-assess-before-request
Jun 2, 2026
Merged

NO-ISSUE: fix(skills): assess Copilot review state before requesting#7
hhk7734 merged 6 commits into
mainfrom
fix/copilot-review-loop-assess-before-request

Conversation

@nulledge

@nulledge nulledge commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • copilot-review-loop blindly requested a Copilot review and then processed only the latest review's comments. On repos that auto-request Copilot when a PR opens, this produced a duplicate review and left the earlier (first-finished) review's threads unresolved.
  • Each round now assesses the PR's Copilot state before requesting: wait for an in-flight review, process pre-existing unresolved threads, and --add-reviewer @copilot only on a clean slate (nothing in flight, nothing open).
  • The per-round work unit is now every unresolved Copilot thread (GraphQL reviewThreads) instead of a single review's snapshot, so an auto-triggered review's comments get addressed instead of skipped.

Follow-up commits (ad8a7f3, e5c70ad, 1433e8a, 2db986a) address Copilot's own review on this PR.

Test Plan

  • scripts/check-skills-sync.sh — both skills/ and plugins/runes/skills/ copies byte-identical
  • bash -n on every embedded bash block — valid
  • All embedded gh api / GraphQL / jq queries execute against a live PR under set -e (exit 0)
  • No remnants of the removed LATEST_ID / pull_request_review_id mechanism
  • Live check: an in-flight Copilot review does appear in requested_reviewers as {"login":"Copilot","type":"Bot"} (pending=1, clears to 0 on submit)
  • Dogfooded the fixed skill end-to-end on this PR — 3 Copilot reviews, each requested exactly once (no duplicates), all 12 threads resolved, loop terminated on "generated no new comments"

🤖 Generated with Claude Code

copilot-review-loop blindly ran `gh pr edit --add-reviewer @copilot`
and then processed only the latest review's comments. On repos that
auto-request Copilot when a PR opens, this caused two failures:

- duplicate review (a second review on top of the auto-triggered one)
- the earlier review's threads were never addressed or resolved

Each round now assesses the PR's Copilot state first: wait for an
in-flight review, process pre-existing unresolved threads, and request
a fresh review only on a clean slate. The per-round work unit is every
unresolved Copilot thread (via GraphQL reviewThreads) instead of a
single review's snapshot, so an auto-triggered review's comments are
addressed instead of skipped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the copilot-review-loop skill to avoid duplicate Copilot review requests on repos that auto-request Copilot, by first assessing whether a Copilot review is pending and by processing unresolved Copilot review threads as the per-round unit of work.

Changes:

  • Add an “Assess before requesting” step that checks for pending Copilot review requests and existing unresolved Copilot threads.
  • Switch fetching/processing from “latest review snapshot” to “all unresolved Copilot review threads” via GraphQL reviewThreads.
  • Refine loop termination guidance to require both a “generated no new comments” review body and an empty unresolved-thread fetch.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.

File Description
skills/copilot-review-loop/SKILL.md Documents the new assess→(wait/process/request) flow and updates the GraphQL/gh snippets accordingly.
plugins/runes/skills/copilot-review-loop/SKILL.md Mirrors the same skill doc updates in the plugin copy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/copilot-review-loop/SKILL.md Outdated
Comment thread skills/copilot-review-loop/SKILL.md
Comment thread skills/copilot-review-loop/SKILL.md Outdated
Comment thread skills/copilot-review-loop/SKILL.md Outdated
Comment thread skills/copilot-review-loop/SKILL.md Outdated
Comment thread plugins/runes/skills/copilot-review-loop/SKILL.md Outdated
Comment thread plugins/runes/skills/copilot-review-loop/SKILL.md
Comment thread plugins/runes/skills/copilot-review-loop/SKILL.md Outdated
Comment thread plugins/runes/skills/copilot-review-loop/SKILL.md Outdated
Comment thread plugins/runes/skills/copilot-review-loop/SKILL.md Outdated
nulledge and others added 3 commits June 2, 2026 09:30
…-review-loop

The skill mixed `<PR>` and `{PR}` while the sibling skill
receiving-code-review (and the rest of the suite) uses lowercase
`{pr}`. Standardize on `{pr}` to prevent copy-paste/substitution
mistakes. Per Copilot review on PR #7.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prose said to halt and report when the 20-minute cap elapses with
no Copilot review, but the wait loop fell through silently — the agent
would then fetch on stale state. Add an explicit post-loop check that
exits non-zero (the base auto skill's "unrecognized error" path) when
no new review landed. Per Copilot review on PR #7.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…licit

The fetch comment said "ALL" unresolved threads, but the GraphQL query
caps at reviewThreads(first:100)/comments(first:100). Reword to "up to
the first 100" and note the cap is not a guarantee of all — paginate
for larger PRs. Per Copilot review on PR #7.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread skills/copilot-review-loop/SKILL.md Outdated
Comment thread plugins/runes/skills/copilot-review-loop/SKILL.md Outdated
…opilot-review-loop

The Assess table referred to a `requested_reviewer` (singular), but
the REST payload and the snippet below use `requested_reviewers`
(plural). Match the exact field name so the prose maps cleanly to the
API output. Per Copilot review on PR #7.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@nulledge nulledge requested a review from hhk7734 June 2, 2026 10:24
@hhk7734

hhk7734 commented Jun 2, 2026

Copy link
Copy Markdown
Member

버전 업데이트해주세요

Patch release covering the copilot-review-loop assess-before-request
fix and accompanying doc refinements.
@hhk7734 hhk7734 merged commit bc971f7 into main Jun 2, 2026
1 check passed
@hhk7734 hhk7734 deleted the fix/copilot-review-loop-assess-before-request branch June 2, 2026 11:23
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.

3 participants