fix(release): count Copilot inline comments by their own login#180
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the release workflow’s PR snapshot so Copilot inline comments are correctly counted (and therefore correctly gated) even though Copilot uses a different login for inline comments than for reviews.
Changes:
- Teach
toplevel_comments_byto accept multiple logins and updatemain()to count Copilot inline comments using both known Copilot author identities. - Add targeted regression tests covering either-login matching, reply/foreign-login exclusion, and an end-to-end
main()snapshot. - Document the behavior/fix in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| skills/release/poll-pr-reviews.sh | Counts inline comments by a set of logins per reviewer and introduces explicit bot-login constants. |
| skills/release/tests/test_poll_pr_reviews.sh | Adds regression coverage to ensure Copilot inline comments are actually counted and don’t bleed into gh-aw counts. |
| CHANGELOG.md | Adds an entry describing the bug and the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Skills | ||
|
|
||
| - **`release`: `poll-pr-reviews.sh` counted zero Copilot inline comments on every PR** — Copilot authors its review as `copilot-pull-request-reviewer[bot]` but its inline comments as `Copilot`; `toplevel_comments_by` matched comments against the review login, so `inline_comments.copilot` was a constant `0`. Step 7's merge gate reads that count for its "every inline comment has a `Fixed in <sha>` or `Declining — <reason>` reply" condition, which a zero count satisfies vacuously — every Copilot inline finding since the counter was written was invisible to the release workflow and mergeable unanswered. Comment counting now matches a set of logins per reviewer (`GH_AW_COMMENT_LOGINS`, `COPILOT_COMMENT_LOGINS` at the top of the script) rather than the single review login; a comment carries one author, so listing both cannot double-count. gh-aw was unaffected (`github-actions[bot]` authors both its reviews and comments), as was `dismiss-stale-reviews.sh` (it matches reviews, where the review login is correct). Found while shipping PR #178: Copilot's body said "generated 1 comment", the watcher reported 0, and the real comment turned out to be a correct finding about the PR's own diff. Regression covered end-to-end through `main()` — the path the gate reads — plus unit coverage for either-login matching and reply/foreign-login exclusion. |
There was a problem hiding this comment.
Fixed in e6ac9ff — tightened to three sentences, keeping the two-login fact and the vacuous-gate consequence and pointing at this PR body for the rest.
Worth recording that this comment is itself the fix working: the watcher on this PR ran the patched script and reported inline_comments.copilot: 1. On the unpatched script it reads 0, the Step 7 reply gate is satisfied vacuously, and your finding merges unanswered — which is exactly what has been happening to every Copilot inline comment until now.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Policy loaded: 23 rule files from rules/ (PR head).
All rules pass — no violations found.
Checked the changed release polling script, its regression tests, and the changelog entry against the loaded policy set. The Copilot comment-login fix is covered through both toplevel_comments_by and the main() snapshot path, and the changed paths stay within the release skill’s scripting/testing expectations.
Generated by PR Policy Review (OpenAI) for #180 · 37.2 AIC · ⌖ 5.18 AIC · ⊞ 14.6K
Per rules/context-artifacts.md CHANGELOG Hygiene — an entry can be one or two sentences when the broader context lives in the PR body. Keep the two-login fact and the vacuous-gate consequence; point at #180 for the rest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgdWmv7KBj91nxbVjMsj7X
There was a problem hiding this comment.
Policy loaded: 23 rule files from rules/ (PR head).
All rules pass — no violations found.
Reviewed the changed release polling script, its regression tests, and the changelog entry against the loaded rules and the governing release skill. No inline findings.
Generated by PR Policy Review (OpenAI) for #180 · 38.2 AIC · ⌖ 3.4 AIC · ⊞ 15.5K
Copilot authors its review as `copilot-pull-request-reviewer[bot]` but its inline comments as `Copilot`. toplevel_comments_by matched comments against the review login, so inline_comments.copilot was a constant 0 on every PR. Step 7's merge gate reads that count to require a reply on every inline comment. A count that is always zero satisfies it vacuously, so every Copilot inline finding was invisible to the release workflow and mergeable unanswered. Match a set of logins per reviewer instead of the single review login. A comment carries one author, so listing both cannot double-count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgdWmv7KBj91nxbVjMsj7X
Per rules/context-artifacts.md CHANGELOG Hygiene — an entry can be one or two sentences when the broader context lives in the PR body. Keep the two-login fact and the vacuous-gate consequence; point at #180 for the rest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgdWmv7KBj91nxbVjMsj7X
e6ac9ff to
b7ad201
Compare
There was a problem hiding this comment.
Policy loaded: 23 rule files from rules/ (PR head).
All rules pass — no violations found.
Reviewed the changed allowlist: CHANGELOG.md, skills/release/poll-pr-reviews.sh, and skills/release/tests/test_poll_pr_reviews.sh. The release skill's governing SKILL.md was also loaded for the changed script path. No inline findings.
Generated by PR Policy Review (OpenAI) for #180 · 31.6 AIC · ⌖ 3.94 AIC · ⊞ 14.6K
Author-Model: claude-opus-4-8
Summary
poll-pr-reviews.shhas been reporting zero Copilot inline comments on every PR, always. Not a race, not intermittent — structural.Copilot uses two different logins on the same PR:
copilot-pull-request-reviewer[bot]Copilottoplevel_comments_bymatched comments against the review login, which never matches a comment, soinline_comments.copilotwas a constant0.Why that matters:
SKILL.mdStep 7 gates the merge on "every inline comment from Step 5'sinline_commentscount has aFixed in <sha>orDeclining — <reason>reply." A count that is always zero satisfies that condition vacuously. Every Copilot inline finding since this counter was written has been invisible to the release workflow and mergeable unanswered.Scope: gh-aw is unaffected —
github-actions[bot]authors both its reviews and its comments.dismiss-stale-reviews.shis unaffected — it matches on reviews, where the review login is correct.Found while shipping #178. Copilot's review body said "generated 1 comment", the watcher reported
inline_comments.copilot: 0, and the comment turned out to be a correct finding about that PR's own diff. Worth noting this fix is a precondition for #178 specifically: that PR adds an always-on rule telling agents to merge on a green gate without asking, which removes the accidental human backstop — someone eyeballing the PR page — that has been quietly compensating for this bug. Autonomy is only as good as the gate's fidelity, so the gate gets fixed first.Fix
toplevel_comments_byis variadic on login and matches a set per reviewer. A comment carries exactly one author, so listing both logins cannot double-count.GH_AW_REVIEW_LOGIN,COPILOT_REVIEW_LOGIN,GH_AW_COMMENT_LOGINS,COPILOT_COMMENT_LOGINS) with the surface/login table documented at the top of the script, perrules/script-as-black-box.md.Test plan
t_main_counts_copilot_comments_in_snapshot— end-to-end throughmain(), the path the gate reads. Fails against the pre-fix script withexpected '2|1', got '2|0'; that0is the bug.t_toplevel_comments_by_matches_either_copilot_login— guards against a fix that swaps the login instead of matching both.t_toplevel_comments_by_excludes_replies_and_other_logins— replies and human comments stay excluded.bash -nclean,shellcheckclean.inline_comments.copilotnow1, was0.🤖 Generated with Claude Code
https://claude.ai/code/session_01JgdWmv7KBj91nxbVjMsj7X