Skip to content

fix(ci): resolve threads autofix finds already addressed #999

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.

Sign up for GitHub

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

Merged
giswqs merged 2 commits into from
Oct 27, 2026
Merged

fix(ci): resolve threads autofix finds already addressed #999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2652a0a
fix(ci): resolve threads autofix finds already addressed
giswqs Oct 27, 2026
875738e
Address Claude review feedback
giswqs Oct 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .github/workflows/claude-autofix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,14 +311,15 @@ jobs:

For each thread, read the code it points at and decide:
- `fixed` — the comment is right: make the smallest change that addresses it, in the style of the surrounding code. A bug fix gets a regression test where the crate has tests.
- `declined` — the comment is wrong, already addressed, or not worth the change: say why in one or two sentences.
- `addressed` — the code already does exactly what the comment asks, usually because an earlier commit fixed it: cite the `path:line` that does it, in one sentence. The thread is resolved, so use this only when you can point at that line.
- `declined` — the comment is wrong (including a misreading of the code) or not worth the change: say why in one or two sentences. The thread stays open for a maintainer.
- `unclear` — you can't tell what is wanted, or the fix needs a design decision a maintainer should make.
Don't change anything no thread asks for, don't touch `.github/`, and don't add dependencies.

${{ github.event.pull_request.head.repo.full_name != github.repository && 'You have no shell here: the change is checked by the pull request''s CI after it is pushed. Keep it to edits you are sure compile, formatted the way rustfmt would format them, and mark a thread `unclear` rather than guess.' || 'Then check your changes: `cargo fmt --all`, `cargo clippy --workspace --all-targets --locked -- -D warnings`, and `cargo test --locked -p <crate>` for every crate you changed. Fix what they report in code you changed. If a check still fails, mark the threads behind it `unclear` and undo their changes, so what is left passes.' }}

Don't commit or push; the workflow does. Finish by writing two files:
- `.autofix/report.json` — a JSON array with one object per thread: `{"id": "<thread id>", "status": "fixed" | "declined" | "unclear", "reply": "<what you changed, or why not>"}`. Replies are posted on the threads as written: plain, short, no greetings.
- `.autofix/report.json` — a JSON array with one object per thread: `{"id": "<thread id>", "status": "fixed" | "addressed" | "declined" | "unclear", "reply": "<what you changed, or why not>"}`. Replies are posted on the threads as written: plain, short, no greetings.
- `.autofix/commit-message.txt` — a commit message for the change: a subject line under 72 characters in the repository's style (for example `fix(labels): …`), a blank line, then a short body wrapped at 72 columns.
claude_args: |
--model claude-haiku-5-5
Expand Down Expand Up @@ -434,7 +435,8 @@ jobs:
echo "pushed=true" >> "$GITHUB_OUTPUT"

# 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.
# Claude calls fixed is resolved only when a fix commit was pushed; one it calls addressed
# needs no commit. Declined and unclear threads stay open for a maintainer.
- name: Reply to the threads
if: needs.fix.outputs.threads != '0'
env:
Expand All @@ -446,7 +448,7 @@ jobs:
: > "$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"))' \
| select(.status | IN("fixed", "addressed", "declined", "unclear"))' \
"$DIR/report.json" |
while read -r entry; do
id=$(jq -r .id <<< "$entry")
Expand All @@ -464,7 +466,7 @@ jobs:
}
}' --silent || echo "::warning::Couldn't reply to $id."
# 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
case "$status" in fixed|addressed) echo "$id" >> "$RUNNER_TEMP/to-resolve" ;; esac
Comment thread
giswqs marked this conversation as resolved.

@github-actions github-actions Bot Oct 27, 2026

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): an addressed verdict resolves a thread on the model's word alone.

The prompt asks Claude to cite path:line for addressed, but nothing here checks that the citation exists. A wrong addressed closes a valid review thread, and a maintainer may not notice. Before this change a wrong verdict left the thread open. A guard that keeps an uncited addressed open:

Suggested change
case "$status" in fixed|addressed) echo "$id" >> "$RUNNER_TEMP/to-resolve" ;; esac
if [ "$status" = addressed ] && ! grep -Eq '\.[A-Za-z0-9]+:[0-9]+' <<< "$reply"; then status=declined; fi
case "$status" in fixed|addressed) echo "$id" >> "$RUNNER_TEMP/to-resolve" ;; esac

The reply has already been posted under the addressed label at this point, so a downgraded thread shows addressed but stays open. Moving the downgrade above the reply (before line 461) would keep the label accurate. The check only tests the shape of the citation, not that the line exists, so it is a floor rather than a verification.

done

# Its own step so only these calls see the maintainer's token, which resolving takes:
Expand Down
10 changes: 5 additions & 5 deletions docs/contributing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ A maintainer's `automerge` label approves and squash-merges a pull request after
checks pass. Pushing again removes the label and disables auto-merge; removing the label also
disables it.

A maintainer's `claude-autofix` label has Claude bring a pull request up to date: it merges the
base branch and resolves any conflicts, then fixes the open review comments it agrees with,
replies to each and resolves the fixed ones, and pushes the commits to the branch (a fork's too,
when edits by maintainers are allowed). The label comes off when it's done; add it again for
another round.
A maintainer's `claude-autofix` label has Claude bring a pull request up to date: it merges the base
branch and resolves any conflicts, then fixes the open review comments it agrees with, replies to
each and resolves the ones it fixed or found already fixed, and pushes the commits to the branch (a
fork's too, when edits by maintainers are allowed). The label comes off when it's done; add it again

@github-actions github-actions Bot Oct 27, 2026

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: this sentence says which threads get resolved but not which stay open. The workflow and the PR description both say declined and unclear threads stay open for a maintainer, and a reader would look for that here. Consider adding "; declined and unclear ones stay open for you" after "found already fixed".

for another round.

## Developer checks

Expand Down
Loading