Repository navigation
fix(ci): resolve autofix threads with the maintainer token #997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,9 +158,13 @@ jobs: | |
| jq -s ' | ||
| def trusted: (.author.__typename == "Bot") | ||
| or (.authorAssociation | IN("OWNER", "MEMBER", "COLLABORATOR")); | ||
| # A thread whose last comment is an autofix reply was answered in an earlier round; | ||
| # it comes back once a reviewer replies to it. | ||
| [ .[] | ||
| | select(.isResolved | not) | ||
| | select(.comments.nodes[0] | trusted) | ||
| | select(.comments.nodes[-1] | ||
| | (.author.__typename == "Bot" and (.body | startswith("**Claude autofix ("))) | not) | ||
| | { id, path, line: (.line // .originalLine), outdated: .isOutdated, | ||
| comments: [ .comments.nodes[] | select(trusted) | ||
| | { author: (.author.login // "ghost"), body } ] } ] | ||
|
|
@@ -431,14 +435,15 @@ jobs: | |
|
|
||
| # Replies go out only for threads that were staged, whatever ids the report holds. A thread | ||
| # Claude calls fixed is resolved only when a fix commit was pushed. | ||
| - name: Reply to and resolve the threads | ||
| - name: Reply to the threads | ||
| if: needs.fix.outputs.threads != '0' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| DIR: ${{ runner.temp }}/autofix | ||
| LANDED: ${{ needs.fix.outputs.fixed == 'true' && steps.push.outputs.pushed == 'true' }} | ||
| run: | | ||
| set -euo pipefail | ||
| : > "$RUNNER_TEMP/to-resolve" | ||
| jq -c --slurpfile t "$DIR/threads.json" \ | ||
| '($t[0] | map(.id)) as $ids | .[] | select(.id as $i | $ids | index($i)) | ||
| | select(.status | IN("fixed", "declined", "unclear"))' \ | ||
|
|
@@ -458,15 +463,31 @@ jobs: | |
| comment { id } | ||
| } | ||
| }' --silent || echo "::warning::Couldn't reply to $id." | ||
| if [ "$status" = fixed ]; then | ||
| gh api graphql -f thread="$id" -f query=' | ||
| mutation($thread: ID!) { | ||
| resolveReviewThread(input: {threadId: $thread}) { thread { id } } | ||
| }' --silent || echo "::warning::Couldn't resolve $id." | ||
| fi | ||
| # The loop runs in a pipe's subshell, so the threads to resolve go to a file. | ||
| if [ "$status" = fixed ]; then echo "$id" >> "$RUNNER_TEMP/to-resolve"; fi | ||
| done | ||
|
|
||
| # Its own step so only these calls see the maintainer's token, which resolving takes: | ||
| # GITHUB_TOKEN can reply to a thread but not resolve it ("Resource not accessible by | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit (medium confidence): the step split narrows which steps get the maintainer token in |
||
| # integration"). A fixed thread left open is staged again next round and answered twice, so | ||
| # a failed resolve fails the step. | ||
| - name: Resolve the fixed threads | ||
| if: needs.fix.outputs.threads != '0' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.AUTOMERGE_TOKEN }} | ||
| run: | | ||
| failed=0 | ||
| while read -r id; do | ||
| gh api graphql -f thread="$id" -f query=' | ||
| mutation($thread: ID!) { | ||
| resolveReviewThread(input: {threadId: $thread}) { thread { id } } | ||
| }' --silent || { echo "::error::Couldn't resolve $id."; failed=1; } | ||
| done < "$RUNNER_TEMP/to-resolve" | ||
| exit "$failed" | ||
|
|
||
| # Posted once the push went through, even when a reply or a resolve failed after it. | ||
| - name: Summarise | ||
|
giswqs marked this conversation as resolved.
|
||
| if: ${{ !cancelled() && steps.push.outcome == 'success' }} | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR: ${{ github.event.pull_request.html_url }} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug (medium confidence): the skip checks the thread's last comment from anyone, but the comment above says the thread "comes back once a reviewer replies". A passer-by's reply after an autofix reply therefore re-stages the thread. The staged
commentsarray keeps only trusted authors, so Claude sees just its own earlier "fixed" reply and answers again ("already addressed"), which is the double answer this PR is meant to stop. Checking the last trusted comment fixes that. The jq is untested here:laston an empty array givesnull, butcomments.nodes[0]is already required to be trusted, so the array is never empty at this point.Related, low confidence:
comments(first: 50)on line 149 has no pagination. In a thread with more than 50 comments,nodes[-1]is not the real last comment and the skip misses it. Fetching the comments with pagination, or readingtotalCount, would close that gap.