diff --git a/.claude/skills/pr-flow/SKILL.md b/.claude/skills/pr-flow/SKILL.md index 1cc421f7..786b5c37 100644 --- a/.claude/skills/pr-flow/SKILL.md +++ b/.claude/skills/pr-flow/SKILL.md @@ -279,11 +279,34 @@ arrive late (see step 8). - It is **not** necessary to implement every suggestion. Implementing one a different way, or declining it with a reason, is fine. -- After making the changes, **respond to each comment** with what was done, or - why it was ignored. -- ⚠️ **Inline replies go hidden once the fix is pushed** (the threads become - outdated), so **mirror each round at PR level** as a summary comment, and always - read the "Suppressed comments" block. +- After making the changes, **reply to each review comment in its own thread** + with what was done, or why it was declined. That inline reply is the primary + response and it is not optional — each review comment is a discussion thread + with its own resolve state, and only a reply _in_ the thread can resolve it or + show a reviewer reading it that the finding was answered. + + ```sh + # Fetch the round's comments by REVIEW id — the unpaginated /reviews listing + # hides later rounds behind your own replies. + gh api repos/modelcontextprotocol/inspector/pulls//reviews//comments \ + --jq '.[]|"\(.id) \(.path):\(.line)\n\(.body)"' + + # Reply into one thread, keyed by the comment id from above. + gh api repos/modelcontextprotocol/inspector/pulls//comments//replies \ + -f body='Fixed in — …' + ``` + +- ⚠️ **Then mirror the round at PR level, in addition — never instead.** Inline + replies go hidden once the fix is pushed, because the threads become outdated, + so a summary comment is what keeps the round readable afterwards. It does + **not** discharge the per-comment replies: a rollup bullet cannot be connected + back to the thread it answers, so the thread stays open with a finding and + silence in it, and by round three matching bullets to comments is + reconstruction rather than reading. +- ⚠️ Always read the **"Suppressed comments"** block in the review body. Those + findings have no comment id, so they have no thread to reply into — the + PR-level mirror is the only place they can be answered, and it is the one case + where answering there is the whole response. - ⚠️ **Copilot's inline comments lag its review body.** The body's "generated N comments" count lands first; fetch by recency and reconcile. Repeated re-review silence means the session ended. diff --git a/AGENTS.md b/AGENTS.md index 2d8d3ffd..ca30281a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -372,6 +372,7 @@ When asked to respond to a code review of a PR: - it is not necessary to implement all suggestions - you are free to implement suggestions in a different way, or to ignore one if there is a good reason - after making the changes, respond to each review comment with what was done (or why it was ignored) +- **that response goes in the review comment's own thread — a rollup comment does not discharge it.** Each review comment is a discussion thread with its own resolve state, so a bullet posted elsewhere on the page cannot be connected back to the thread it answers: the thread stays open showing a finding and no reply, and the PR reads as though the review were ignored. Reply inline first, per comment; then post the PR-level summary **in addition**, because inline replies go hidden once the fix is pushed. A finding in the review's "Suppressed comments" block has no thread to reply into, so the summary is the only place it can be answered — that is the one exception. The `gh` calls are in the `pr-flow` skill, step 8. ## Always test new or modified code