diff --git a/.agents/skills/iterate-pr/SKILL.md b/.agents/skills/iterate-pr/SKILL.md index a2abc362..6d8cadd4 100644 --- a/.agents/skills/iterate-pr/SKILL.md +++ b/.agents/skills/iterate-pr/SKILL.md @@ -57,7 +57,7 @@ Returns JSON with feedback categorized as: - `medium` - Should address (`m:`, standard feedback) - `low` - Optional (`l:`, nit, style, suggestion) - `bot` - Informational automated comments (Codecov, Dependabot, etc.) -- `resolved` - Already resolved threads +- `resolved` - Already resolved threads, and top-level comments carrying our own 🎉 acknowledgement Review bot feedback (from Sentry, Warden, Copilot, Cursor, Bugbot, CodeQL, etc.) appears in `high`/`medium`/`low` with `review_bot: true` — it is NOT placed in the `bot` bucket. @@ -182,7 +182,25 @@ To close out several threads in one pass, use `scripts/resolve_pr_threads.py THR **Top-level comments** (items WITHOUT a `thread_id` — `review_summary` items and top-level PR/issue comments, e.g. a review bot like Claude that posts its findings as one top-level comment): -There is no thread to reply into, so post a **new top-level comment** with `gh pr comment --body "..."`. A PR can carry several independent top-level comments, so a bare reply is ambiguous — **open every top-level reply with a reference marker** identifying the comment you are addressing. Cite the author and the opening of the original, and link it when the item includes a `url`: +There is no thread to reply into, so post a **new top-level comment** — and when the item carries a `comment_id`, also **add a 🎉 reaction to the original**. The reaction is the machine-readable record that the item is handled: `fetch_pr_feedback.py` checks whether _we_ reacted and buckets the comment as `resolved`, so a re-run stops reporting it. Without it, every later pass re-surfaces the same comment and you have to reason about whether you already dealt with it. + +```bash +gh pr comment --body "..." +gh api -X POST "repos/{owner}/{repo}/issues/comments//reactions" -f content=hooray +``` + +Take the id from the item's `comment_id` rather than searching by body text. If you do need to look it up, use the **PR-scoped** endpoint _with_ `--paginate` — the repo-wide `repos/{owner}/{repo}/issues/comments` returns every comment in the repository, and the PR-scoped one defaults to 30 per page, so an unpaginated search silently misses comments on later pages: + +```bash +gh api "repos/{owner}/{repo}/issues//comments" --paginate \ + --jq '.[] | select(.id == ) | .id' +``` + +**The reaction is scoped to us.** The script does not trust the raw reaction count — anyone can react 🎉 to a comment for unrelated reasons, and treating that as handled would silently drop real feedback. It confirms the reaction belongs to the authenticated user, and fails closed: if the viewer cannot be identified, the item resurfaces. Answering twice beats dropping something. + +**Items without a `comment_id` cannot be reacted to.** GitHub exposes no reactions endpoint for a pull-request review body, so `review_summary` items — the text of a submitted review, as opposed to an ordinary PR conversation comment — have no reaction target. For those, dedupe by scanning existing top-level comments for one whose **reference marker** already cites this author and snippet. In practice most review bots post their findings as an ordinary comment, which does carry a `comment_id`. + +A PR can carry several independent top-level comments, so a bare reply is ambiguous — **open every top-level reply with a reference marker** identifying the comment you are addressing. Cite the author and the opening of the original, and link it when the item includes a `url`: ``` > **Re:** @ — "…" @@ -197,7 +215,7 @@ There is no thread to reply into, so post a **new top-level comment** with `gh p - 1-2 sentences: what was changed, why it's not an issue, or acknowledgment of declined items. - End every reply with `\n\n*— AI Coding Agent*`. -- Before replying, dedupe against re-loops: for inline threads, check whether the thread already has a reply ending in `*- AI Coding Agent*` / `*— AI Coding Agent*`; for top-level comments, scan existing top-level comments for one whose **reference marker** already cites this author + snippet (the signature alone is not enough — distinct top-level items would otherwise collide). +- Before replying, dedupe against re-loops: for inline threads, check whether the thread already has a reply ending in `*- AI Coding Agent*` / `*— AI Coding Agent*`. For a top-level comment with a `comment_id`, the 🎉 reaction handles it — an item carrying `acknowledged: true` (bucketed as `resolved`) has already been answered, so skip it. For a `review_summary` with no `comment_id`, fall back to the reference-marker scan. - If the `gh`/GraphQL call fails, log and continue — do not block the workflow. ### 4. Check CI Status diff --git a/.agents/skills/iterate-pr/scripts/fetch_pr_feedback.py b/.agents/skills/iterate-pr/scripts/fetch_pr_feedback.py index 69aafb8e..004e63c2 100644 --- a/.agents/skills/iterate-pr/scripts/fetch_pr_feedback.py +++ b/.agents/skills/iterate-pr/scripts/fetch_pr_feedback.py @@ -147,6 +147,58 @@ def get_review_comments(owner: str, repo: str, pr_number: int) -> list[dict[str, return result if isinstance(result, list) else [] +_VIEWER_LOGIN: str | None = None +_VIEWER_LOOKUP_FAILED = False + + +def get_viewer_login() -> str | None: + """Login of the authenticated gh user, looked up once per run.""" + global _VIEWER_LOGIN, _VIEWER_LOOKUP_FAILED + if _VIEWER_LOGIN is None and not _VIEWER_LOOKUP_FAILED: + # No --jq here: run_gh json.loads() its stdout, and --jq emits a bare + # unquoted string that is not valid JSON. + result = run_gh(["api", "user"]) + login = result.get("login") if isinstance(result, dict) else None + if login: + _VIEWER_LOGIN = login + else: + _VIEWER_LOOKUP_FAILED = True + return _VIEWER_LOGIN + + +def has_our_acknowledgement( + owner: str, repo: str, comment_id: int, hooray_count: int +) -> bool: + """Whether *we* left the acknowledgement reaction on this comment. + + The count on the comment payload is every user's reaction, so a maintainer + celebrating a review would otherwise mark it handled and silently drop real + feedback. Only our own reaction counts. The per-comment lookup is skipped + entirely when the count is zero, so the common case costs nothing. + + Fails closed: if the viewer cannot be identified or the lookup errors, the + item is treated as unacknowledged and resurfaces, which is the safe + direction — answering twice beats dropping feedback. + """ + if hooray_count <= 0: + return False + viewer = get_viewer_login() + if not viewer: + return False + reactions = run_gh([ + "api", + f"repos/{owner}/{repo}/issues/comments/{comment_id}/reactions", + "--paginate", + ]) + if not isinstance(reactions, list): + return False + return any( + r.get("content") == "hooray" + and (r.get("user") or {}).get("login") == viewer + for r in reactions + ) + + def get_issue_comments(owner: str, repo: str, pr_number: int) -> list[dict[str, Any]]: """Get PR conversation comments (includes bot comments).""" result = run_gh([ @@ -301,6 +353,8 @@ def extract_feedback_item( review_bot: bool = False, self_review: bool = False, thread_id: str | None = None, + comment_id: int | None = None, + acknowledged: bool = False, ) -> dict[str, Any]: """Create a standardized feedback item.""" # Truncate long bodies for summary @@ -329,6 +383,10 @@ def extract_feedback_item( item["self_review"] = True if thread_id: item["thread_id"] = thread_id + if comment_id is not None: + item["comment_id"] = comment_id + if acknowledged: + item["acknowledged"] = True return item @@ -462,12 +520,30 @@ def main(): if not body or len(body.strip()) < 3: continue + # Our own 🎉 reaction is the machine-readable "this was handled" marker + # for top-level comments, which have no thread to resolve. Scoped to the + # authenticated user: an unrelated 🎉 from anyone else must not silence + # real feedback. Set it after replying; see the skill's "Replying to + # Comments" section. + acknowledged = has_our_acknowledgement( + owner, + repo, + comment.get("id", 0), + comment.get("reactions", {}).get("hooray", 0), + ) + item = extract_feedback_item( body=body, author=author, url=comment.get("html_url"), + comment_id=comment.get("id"), + acknowledged=acknowledged, ) + if acknowledged: + feedback["resolved"].append(item) + continue + if is_review_bot(author): category = categorize_comment(comment, body) item["review_bot"] = True