From e634127fb6efc702507e5c192c307951052b3a36 Mon Sep 17 00:00:00 2001 From: Damien Daspit Date: Tue, 29 Sep 2026 18:05:20 -0400 Subject: [PATCH] Let Claude follow up on reviews and open PRs from issues The review workflow runs on every push and posted the same findings again each time. The review now replies to its earlier findings, resolves the ones that were addressed or withdrawn, and posts only new findings. Runs for the same PR no longer overlap. The @claude workflow could not read the source of a porting issue or open a pull request. It can now read issues and PRs, run the local checks, and open a PR that closes the issue. It only runs when the issue or PR and the comment come from a repo member. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/pr-review/SKILL.md | 59 ++++++++++++++++++++++-- .github/workflows/claude-code-review.yml | 6 +++ .github/workflows/claude.yml | 51 ++++++++++++++++---- 3 files changed, 103 insertions(+), 13 deletions(-) diff --git a/.claude/skills/pr-review/SKILL.md b/.claude/skills/pr-review/SKILL.md index b7041d3f..dca0f646 100644 --- a/.claude/skills/pr-review/SKILL.md +++ b/.claude/skills/pr-review/SKILL.md @@ -8,13 +8,58 @@ user-invocable: true # Writing a machine.py review Post one short comment per finding, anchored on the line it is about, then one -summary comment. A review is read-only: do not edit, commit, push, or resolve -threads. +summary comment. A review leaves the code alone: do not edit, commit, or push. +The only thread it resolves is its own finding, once addressed or withdrawn. Unless verified findings are already in hand, get them first with `/code-review high `, without `--comment`: it finds and verifies, and this skill decides what gets posted. +## Start from the last round + +A PR is reviewed again on every push. Each round is a follow-up: settle the +open findings first, then add only what is new. + +1. Load the review threads, which carry the resolved state REST lacks: + + ``` + gh api graphql -F owner={owner} -F repo={repo} -F n= -f query=' + query($owner: String!, $repo: String!, $n: Int!) { + repository(owner: $owner, name: $repo) { pullRequest(number: $n) { + reviewThreads(first: 100) { nodes { id isResolved path line + comments(first: 50) { nodes { databaseId author { login } body } } } } } } }' + ``` + + Load summaries from + `gh api --paginate repos/{owner}/{repo}/issues//comments`. An earlier + finding is a thread whose first comment's author is `claude`, as GraphQL + spells `claude[bot]`, numbered `F` or not. It is open while unresolved, + even when `line` is null because the diff moved past it. The latest summary + names the commit it reviewed. +2. Check every open finding against the head commit and reply in its thread, + using its first comment's `databaseId`, with + `gh api repos/{owner}/{repo}/pulls//comments//replies -f body=...`: + - Fixed: `F3 addressed in :` and what fixed it, then resolve the + thread by its `id`: + `gh api graphql -F id= -f query='mutation($id: ID!) { resolveReviewThread(input: {threadId: $id}) { thread { isResolved } } }'` + - Author gave a reason: weigh it. If it holds, `F3 withdrawn:` and why, then + resolve the thread. If not, answer once with evidence; a point already + answered stays answered. + - Author chose to keep it, e.g. deferred to an issue: record it as accepted. + - Still applies and its code changed: `F3 still applies at :` and why. + - Still applies and its code is untouched: stay silent; the summary counts + it. +3. Treat each finding `/code-review` returns as a duplicate when any thread, + open or resolved, from anyone, already raises the same defect, even if the + line has moved. Drop duplicates: a resolved thread is a settled one. +4. Post a new finding when it is on code changed since the reviewed commit, or + when it is Critical. Diff with `git diff ` if + `gh api repos/{owner}/{repo}/compare/... --jq .status` + prints `ahead`; otherwise history was rewritten, so treat the whole PR as + changed. Number new findings on from the highest `F` in the thread. + +Every earlier finding has a status when this is done. + ## 1. One finding, one comment Anchor it on the line. Two problems on one line are two comments. A reviewer @@ -87,8 +132,14 @@ confirm `Unverified`; an unverified concern is never Critical. Say which public API, optional dependency, published-wheel surface, or parity contract with `sillsdev/machine` changed, or `None verified`. -Then mark each finding, by number, **changed**, **accepted**, or -**unverified**. Leave nothing implicit: a thread with no follow-up leaves nobody +Then mark every finding in the thread, earlier rounds included, by number: +**new**, **open**, **addressed**, **accepted** (the author keeps it knowingly), +or **withdrawn**, adding **unverified** where +it applies. Leave nothing implicit: a thread with no follow-up leaves nobody able to tell which findings mattered. +End with `Reviewed at `, the PR head from +`gh pr view --json headRefOid`, not the merge commit checked out, so the +next round knows where this one stopped. + For an adversarial second pass, apply `docs/review/devils-advocate.md`. diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 92a9884f..b0cd2712 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -10,6 +10,12 @@ on: # - "src/**/*.js" # - "src/**/*.jsx" +# Parallel reviews of one PR cannot see each other's comments and post duplicates. +# A cancelled run may leave a partial round; the next one picks up its threads. +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + env: PYTHON_VERSION: "3.12" diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 580e80a9..3fe1ebf6 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -10,13 +10,23 @@ on: pull_request_review: types: [submitted] +env: + PYTHON_VERSION: "3.12" + jobs: claude: + # Skips outsiders before the install; the action still checks write access. The issue or PR + # body reaches the prompt whoever wrote it, and an outsider's fork PR would run its code. if: | - (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || + contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), + github.event.comment.author_association || github.event.review.author_association || + github.event.issue.author_association) && + contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), + github.event.issue.author_association || github.event.pull_request.author_association) && + ((github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || - (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) + (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')))) runs-on: ubuntu-latest permissions: contents: read @@ -28,7 +38,31 @@ jobs: - name: Checkout repository uses: actions/checkout@v7 with: - fetch-depth: 1 + # Full history so git log and blame can find the commits AGENTS.md cites. + fetch-depth: 0 + + - name: Set up Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v6 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Install Poetry + uses: snok/install-poetry@v1 + with: + version: 2.4.1 + virtualenvs-create: true + virtualenvs-in-project: true + installer-parallel: true + + - name: Restore virtualenv + uses: actions/cache@v4 + with: + path: .venv + key: claude-venv-${{ runner.os }}-py${{ env.PYTHON_VERSION }}-${{ hashFiles('poetry.lock') }} + + # A change made from an issue is validated before its pull request is opened. + - name: Install dependencies + run: poetry install --no-interaction --all-extras - name: Run Claude Code id: claude @@ -40,11 +74,10 @@ jobs: additional_permissions: | actions: read - # Optional: Give a custom prompt to Claude. If this is not specified, Claude will perform the instructions specified in the comment that tagged it. - # prompt: 'Update the pull request description to include a summary of changes.' - - # Optional: Add claude_args to customize behavior and configuration + # The action's own prompt ends an issue run with a "Create a PR" link; the + # appended prompt has Claude open the pull request itself. + claude_args: >- + --allowedTools "Bash(gh pr create:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh issue view:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*),Bash(git status:*),Bash(poetry run:*),Bash(./local_check.sh:*)" + --append-system-prompt "When invoked on an issue and you change code, open the pull request yourself once your commits are pushed: gh pr create --base main --head . Post its link in your comment in place of a Create a PR link. Write the title and body with the pr-authoring skill, and put Closes # in the body. Before opening it, run ./local_check.sh --agent-strict and report its result in the body. Dependencies are already installed, and you work on the branch this action created: where a skill such as port-pr says to install, create or switch branches, or push, commit on the current branch and push it the way these instructions say." # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # or https://code.claude.com/docs/en/cli-reference for available options - # claude_args: '--allowed-tools Bash(gh pr *)' -