Skip to content

fix(ci): resolve autofix threads with the maintainer token - #997

Merged
giswqs merged 2 commits into
mainfrom
ci/claude-autofix-resolve
Oct 27, 2026
Merged

giswqs merged 2 commits into
mainfrom
ci/claude-autofix-resolve

Conversation

@giswqs

@giswqs giswqs commented Oct 27, 2026

Copy link
Copy Markdown
Member

Fixes two problems found by running the claude-autofix workflow on #972.

Fixed threads weren't resolved. Every resolveReviewThread call in the push job failed with gh: Resource not accessible by integration, in both runs 40438967670 and 40439326237. GITHUB_TOKEN can post a reply to a thread but can't resolve it, and the step only logged a warning. Resolving now uses AUTOMERGE_TOKEN, the maintainer token the job already pushes with. A failed resolve now fails the step instead of passing quietly.

Threads were answered twice. Because the fixed threads stayed open, the second round staged them again. Claude correctly replied "declined: already addressed" under its own "fixed" reply. Fixing the resolve stops most of this. The thread staging now also skips any thread whose last comment is an autofix reply, so a declined or unclear thread only comes back once a reviewer answers it.

Tested: I ran the new filter against #972's threads. It stages 1 of the 8 open threads (the one nobody has answered yet), where the old filter staged all 8. actionlint and pre-commit pass.

GITHUB_TOKEN can reply to a review thread but not resolve it ("Resource
not accessible by integration"), so every fixed thread stayed open,
and the next round staged it again and answered it a second time as
"already addressed". Resolve with AUTOMERGE_TOKEN and fail the step
when a resolve fails, instead of warning.

Also skip threads whose last comment is an autofix reply: a declined
or unclear thread comes back only once a reviewer answers it.
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread .github/workflows/claude-autofix.yml Outdated
Comment thread .github/workflows/claude-autofix.yml
Comment thread .github/workflows/claude-autofix.yml Outdated
Comment thread .github/workflows/claude-autofix.yml Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Code review

Scope: one file, .github/workflows/claude-autofix.yml (+11/−3). I read the surrounding workflow, the staging step (lines 122–173), the push and resolve steps (lines 400–475) and the Summarise step. I could not run actionlint or the PR's test claims. The CI behaviour described in the PR body (the 8→1 thread count, the resolve permission failure) is taken from the PR text and not checked.

Bugs

  • Skip check ignores the author of the last comment (claude-autofix.yml:166, medium confidence). A non-trusted commenter can post a comment starting with **Claude autofix ( as the last comment of a maintainer's thread, which silently removes that thread from autofix. The check should require the bot's own reply.
  • Summary is skipped when resolve fails (claude-autofix.yml:477, medium confidence). The resolve step can now fail the job, and Summarise has no if:, so it doesn't run. The maintainer loses the summary in the case they most need it. if: always() would fix it.

Security

  • No new issues of note. RESOLVE_TOKEN is passed to the step's environment, which is wider than the one gh call that needs it (claude-autofix.yml:443, low confidence, low impact). The job already passes AUTOMERGE_TOKEN to checkout.

Performance

  • Nothing found.

Quality

  • The failure-marker file needs a comment explaining why it isn't a variable (claude-autofix.yml:474, nit).
  • comments(first: 50) (line 149) means .comments.nodes[-1] is the last fetched comment, not the last comment. This only matters for threads with more than 50 comments and is pre-existing, so it is low priority.

AGENTS.md

  • No violations. The change is CI-only and doesn't touch Rust code, .github/ file-change rules or the attribution rules.

Change size

Compared: the whole PR against main (merge base 659873b to PR head a89de00).

Measure Value
Files changed 1 (0 added, 1 modified, 0 removed, 0 renamed)
Size before 27.6 KB (28,270 B)
Size after 28.2 KB (28,927 B)
Total size change +657 B
Path Before After Change
.github/workflows/claude-autofix.yml 27.6 KB 28.2 KB +657 B

- Skip a thread only when its last comment is the bot's own autofix
  reply, so a passer-by can't silence a thread by imitating one.
- Post the summary whenever the push went through, even if a reply or
  resolve failed after it.
- Resolve in a step of its own, so only those calls see the maintainer
  token.
- Note why the threads to resolve go through a file.
@giswqs
giswqs merged commit 6abc4b9 into main Oct 27, 2026
3 of 4 checks passed
@giswqs
giswqs deleted the ci/claude-autofix-resolve branch October 27, 2026 02:47
Comment on lines +166 to +167
| select(.comments.nodes[-1]
| (.author.__typename == "Bot" and (.body | startswith("**Claude autofix ("))) | not)

Copy link
Copy Markdown
Contributor

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 comments array 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: last on an empty array gives null, but comments.nodes[0] is already required to be trusted, so the array is never empty at this point.

Suggested change
| select(.comments.nodes[-1]
| (.author.__typename == "Bot" and (.body | startswith("**Claude autofix ("))) | not)
| select(.comments.nodes | map(select(trusted)) | last
| (.author.__typename == "Bot" and (.body | startswith("**Claude autofix ("))) | not)

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 reading totalCount, would close that gap.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 env, but it does not keep the token away from the rest of the job. The checkout uses token: AUTOMERGE_TOKEN with credentials persisted by default, so the token already sits in .git/config for every later step, including Reply and Summarise. The push step needs that persisted credential, so this is not a bug in the PR. The comment "so only these calls see the maintainer's token" is still stronger than the reality, and "only in this step's env" would be accurate.

@github-actions

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • Medium confidence: the new staging skip (claude-autofix.yml ~L166) checks the thread's last comment from anyone, not the last trusted one. A passer-by's reply after an autofix reply re-stages the thread, and Claude answers it again. This is the double-answer the PR is meant to stop. Suggestion is inline.
  • Low confidence: comments(first: 50) (L149) is not paginated. In a thread with more than 50 comments, nodes[-1] is not the real last comment, so the skip can miss it.

Security

  • No issues found. The thread IDs passed to resolveReviewThread are filtered against the staged set, and -f thread= passes them as GraphQL variables, so there is no query injection. The maintainer token is limited to the resolve step's env.

Performance

  • Nothing worth raising.

Quality

  • Medium confidence (nit): the comment on the new resolve step says "only these calls see the maintainer's token". The checkout persists AUTOMERGE_TOKEN in .git/config for the whole job, so later steps can read it too. The push step needs that credential, so this is not a defect in the PR. The comment should say "in this step's env". Inline comment posted.

AGENTS.md

  • Nothing violated. The workflow header and docs/contributing.md still describe the behaviour correctly.
  • Not checked: whether the PR's "stages 1 of 8" claim holds. I can't run the workflow here.

Change size

Compared: the whole PR against main (merge base 659873b to PR head ec3ffb1).

Measure Value
Files changed 1 (added 0, modified 1, removed 0, renamed 0)
Size before 27.6 KB
Size after 28.8 KB
Total size change +1.2 KB
Path Before After Change
.github/workflows/claude-autofix.yml 27.6 KB 28.8 KB +1.2 KB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant