diff --git a/.claude/skills/address-pr-comments/post-reply.py b/.claude/skills/address-pr-comments/post-reply.py index 4500e3c2..588981f9 100755 --- a/.claude/skills/address-pr-comments/post-reply.py +++ b/.claude/skills/address-pr-comments/post-reply.py @@ -15,14 +15,12 @@ """ import argparse -import json -import subprocess import sys from pathlib import Path sys.path.insert(0, str(Path(__file__).parent)) from utils import owner_repo_from_remote -from utils import run_cmd +from utils import post_via_gh_api def main() -> None: @@ -38,11 +36,6 @@ def main() -> None: args = parser.parse_args() owner, repo = owner_repo_from_remote() - try: - body = args.body_file.read_text(encoding="utf-8") - except OSError as e: - _ = sys.stderr.write(f"Unable to read body file {args.body_file}: {e}\n") - sys.exit(1) if args.comment_type == "pulls/comments": # Reply directly into the inline review comment thread. @@ -51,31 +44,7 @@ def main() -> None: # GitHub has no "reply" concept for issue comments — post a new top-level comment. endpoint = f"repos/{owner}/{repo}/issues/{args.pr}/comments" - try: - result = run_cmd( - [ - "gh", - "api", - endpoint, - "--method", - "POST", - "--raw-field", - f"body={body}", - ], - timeout=30, - timeout_msg="gh api timed out while posting reply.", - ) - except subprocess.CalledProcessError as e: - _ = sys.stderr.write(f"gh api error: {e.stderr}\n") - sys.exit(1) - - response = json.loads(result.stdout) - _ = sys.stdout.write(f"Posted: {response.get('html_url', '(no URL in response)')}\n") - - try: - args.body_file.unlink() - except OSError as e: - _ = sys.stderr.write(f"Warning: post succeeded but could not delete body file {args.body_file}: {e}\n") + post_via_gh_api(endpoint=endpoint, body_file=args.body_file, timeout_msg="gh api timed out while posting reply.") if __name__ == "__main__": diff --git a/.claude/skills/address-pr-comments/post-review-comment.py b/.claude/skills/address-pr-comments/post-review-comment.py new file mode 100755 index 00000000..638dd2d6 --- /dev/null +++ b/.claude/skills/address-pr-comments/post-review-comment.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +"""Post a new inline review comment on a PR diff via the GitHub API. + +Usage: + # Comment on one line of the new file: + post-review-comment.py --pr --path --line --body-file + + # Comment spanning a range of lines (start and end must be in the same diff hunk): + post-review-comment.py --pr --path --start-line --line --body-file + + # Comment on a removed line (the old file's numbering): + post-review-comment.py --pr --path --line --side LEFT --body-file + +Line numbers are those of the file at --commit-id (default: the PR's current head) for --side RIGHT, or of the base +file for --side LEFT. Owner and repo are derived from the git remote automatically. + +This script exists so that gh api calls are covered by the skills allow-list +rather than requiring manual approval each time. +""" + +import argparse +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent)) +from utils import owner_repo_from_remote +from utils import post_via_gh_api +from utils import run_cmd + + +def pr_head_sha(*, owner: str, repo: str, pr: int) -> str: + try: + result = run_cmd( + ["gh", "api", f"repos/{owner}/{repo}/pulls/{pr}", "--jq", ".head.sha"], + timeout=30, + timeout_msg="gh api timed out while reading the PR head commit.", + ) + except subprocess.CalledProcessError as e: + _ = sys.stderr.write(f"gh api error reading PR #{pr}: {e.stderr}\n") + sys.exit(1) + return result.stdout.strip() + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + _ = parser.add_argument("--pr", type=int, required=True, help="PR number") + _ = parser.add_argument("--path", required=True, help="File path relative to the repo root") + _ = parser.add_argument("--line", type=int, required=True, help="Line the comment ends on") + _ = parser.add_argument("--start-line", type=int, help="First line of a multi-line comment") + _ = parser.add_argument("--side", choices=["RIGHT", "LEFT"], default="RIGHT", help="RIGHT = new file, LEFT = old") + _ = parser.add_argument("--start-side", choices=["RIGHT", "LEFT"], help="Defaults to --side") + _ = parser.add_argument("--commit-id", help="Commit the line numbers refer to (default: the PR's head)") + _ = parser.add_argument("--body-file", required=True, type=Path) + args = parser.parse_args() + + if args.start_line is not None and args.start_line >= args.line: + _ = sys.stderr.write(f"--start-line ({args.start_line}) must be less than --line ({args.line})\n") + sys.exit(1) + + owner, repo = owner_repo_from_remote() + + commit_id = args.commit_id + if commit_id is None: + commit_id = pr_head_sha(owner=owner, repo=repo, pr=args.pr) + + string_fields = {"commit_id": commit_id, "path": args.path, "side": args.side} + int_fields = {"line": args.line} + if args.start_line is not None: + string_fields["start_side"] = args.side if args.start_side is None else args.start_side + int_fields["start_line"] = args.start_line + + post_via_gh_api( + endpoint=f"repos/{owner}/{repo}/pulls/{args.pr}/comments", + body_file=args.body_file, + string_fields=string_fields, + int_fields=int_fields, + timeout_msg="gh api timed out while posting the review comment.", + ) + + +if __name__ == "__main__": + main() diff --git a/.claude/skills/address-pr-comments/utils.py b/.claude/skills/address-pr-comments/utils.py index d68f10fc..c169fe39 100644 --- a/.claude/skills/address-pr-comments/utils.py +++ b/.claude/skills/address-pr-comments/utils.py @@ -1,6 +1,9 @@ +import json import re import subprocess import sys +from collections.abc import Mapping +from pathlib import Path def run_cmd( @@ -41,3 +44,55 @@ def owner_repo_from_remote() -> tuple[str, str]: _ = sys.stderr.write(f"Cannot parse GitHub owner/repo from remote: {url}\n") sys.exit(1) return match.group(1), match.group(2) + + +def _read_body_file(body_file: Path) -> str: + try: + return body_file.read_text(encoding="utf-8") + except OSError as e: + _ = sys.stderr.write(f"Unable to read body file {body_file}: {e}\n") + sys.exit(1) + + +def post_via_gh_api( + *, + endpoint: str, + body_file: Path, + string_fields: Mapping[str, str] | None = None, + int_fields: Mapping[str, int] | None = None, + timeout_msg: str, +) -> None: + """POST a comment to a GitHub API endpoint with gh api, print its URL, then delete the body file it came from. + + The comment's `body` field is read from body_file. string_fields are sent as strings (--raw-field) and int_fields as + JSON numbers (--field), which is what GitHub expects for values like line numbers. + + Exits with status 1 if gh api fails. Once gh api succeeds the comment exists, so an unparsable response still counts + as posted and the body file is still deleted (otherwise a retry would post a duplicate). A failure to delete the body + file only warns. + """ + body = _read_body_file(body_file) + cmd = ["gh", "api", endpoint, "--method", "POST", "--raw-field", f"body={body}"] + if string_fields is not None: + for key, value in string_fields.items(): + cmd += ["--raw-field", f"{key}={value}"] + if int_fields is not None: + for key, value in int_fields.items(): + cmd += ["--field", f"{key}={value}"] + + try: + result = run_cmd(cmd, timeout=30, timeout_msg=timeout_msg) + except subprocess.CalledProcessError as e: + _ = sys.stderr.write(f"gh api error: {e.stderr}\n") + sys.exit(1) + + try: + url = json.loads(result.stdout).get("html_url", "(no URL in response)") + except json.JSONDecodeError: + url = "(unparsable response)" + _ = sys.stdout.write(f"Posted: {url}\n") + + try: + body_file.unlink() + except OSError as e: + _ = sys.stderr.write(f"Warning: post succeeded but could not delete body file {body_file}: {e}\n") diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index b90b48a7..5ab1e5d9 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -14,13 +14,13 @@ -## Test plan +## How do we know this works? - + ## Other diff --git a/copier.yaml b/copier.yaml index 2eddb8c3..6e552cc3 100644 --- a/copier.yaml +++ b/copier.yaml @@ -106,7 +106,7 @@ _tasks: fi fi - command: | - for script in .claude/skills/address-pr-comments/check-footer.py .claude/skills/address-pr-comments/fetch-pr-comments.py .claude/skills/address-pr-comments/post-reply.py; do + for script in .claude/skills/address-pr-comments/check-footer.py .claude/skills/address-pr-comments/commit-link.py .claude/skills/address-pr-comments/fetch-pr-comments.py .claude/skills/address-pr-comments/post-reply.py .claude/skills/address-pr-comments/post-review-comment.py .claude/skills/address-pr-comments/verify-env.py; do if [ -f "$script" ]; then chmod +x "$script" if git ls-files --error-unmatch "$script" > /dev/null 2>&1; then diff --git a/template/copier.yml.jinja-base b/template/copier.yml.jinja-base index e953102e..d24ca076 100644 --- a/template/copier.yml.jinja-base +++ b/template/copier.yml.jinja-base @@ -199,7 +199,7 @@ pnpm_minimum_release_age_exclude: fi fi - command: | - for script in .claude/skills/address-pr-comments/check-footer.py .claude/skills/address-pr-comments/commit-link.py .claude/skills/address-pr-comments/fetch-pr-comments.py .claude/skills/address-pr-comments/post-reply.py .claude/skills/address-pr-comments/verify-env.py; do + for script in .claude/skills/address-pr-comments/check-footer.py .claude/skills/address-pr-comments/commit-link.py .claude/skills/address-pr-comments/fetch-pr-comments.py .claude/skills/address-pr-comments/post-reply.py .claude/skills/address-pr-comments/post-review-comment.py .claude/skills/address-pr-comments/verify-env.py; do if [ -f "$script" ]; then chmod +x "$script" if git ls-files --error-unmatch "$script" > /dev/null 2>&1; then