-
Notifications
You must be signed in to change notification settings - Fork 1
autofix: opt-in, one-shot fixing of the PR reviewer's feedback #298
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
Merged
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
c4d24d4
[jwies/review-dispatch-gate] review: block review submission when the…
jwbron 7cc1fc2
[jwies/review-dispatch-gate] review: harden the gate's fail-open orde…
jwbron 19f87ec
[jwies/review-dispatch-gate] review: protect the sentinel/pre-gate wr…
jwbron 588356b
[jwies/review-dispatch-gate] review: pin keep-list survivors, Step 6 …
jwbron f95d3f2
[jwies/review-pre-agent-staging] review: move the staging layer into …
jwbron c0573f0
[jwies/review-pre-agent-staging] review: fix the added-lines hash for…
jwbron 6bdfb95
[jwies/review-pre-agent-staging] review: tighten ghGet retry semantic…
jwbron 63930fd
[jwies/review-stamp-carrier] review: re-review fingerprint falls back…
jwbron 8942b86
[jwies/review-dispatch-gate-local] Merge branch 'jwies/review-stamp-c…
jwbron c6e43a8
[jwies/review-dispatch-gate-local] review: shared lenient sub-agent J…
jwbron f737f84
[jwies/review-pre-agent-staging-local] Merge branch 'jwies/review-dis…
jwbron f5aae5c
[jwies/review-pre-agent-staging-local] review: the staged plan artifa…
jwbron 5735a0e
[jwies/review-pre-agent-staging-local] review: retry the secondary-ra…
jwbron bdbc612
[jwies/autofix-v1] autofix: opt-in, one-shot fixing of the PR reviewe…
jwbron 11c4a11
[jwies/autofix-v1] autofix: add /autofix as a peer arming surface alo…
jwbron 0182485
[jwies/autofix-v1] autofix: fix the two faults the first live trial r…
jwbron 887fcc8
[jwies/autofix-v1] autofix: work around gh-aw's unbounded PR-branch f…
jwbron a4e778e
[jwies/autofix-v1] autofix: move to Opus 5; settle the tagOpt questio…
jwbron 73a35fa
[jwies/autofix-v1] autofix: self-contained commit messages, exception…
jwbron 77b7de7
[jwies/autofix-v1] autofix: stage deterministically in a pre-agent step
jwbron ce00455
[jwies/autofix-v1] autofix: fix the currency guard failing open; one …
jwbron d7f684b
[jwies/autofix-v1] autofix: reuse the stack's diff builder; close the…
jwbron 33ec833
[jwies/autofix-v1] autofix: price Opus 5 for the proxy; match bot log…
jwbron dd06f8c
[jwies/autofix-v1] autofix: pin the firewall to the version that hono…
jwbron 1a61004
[jwies/autofix-v1] autofix: hold at Opus 4.8; the Opus 5 pricing fall…
jwbron 5890044
[jwies/autofix-v1] autofix: correct the Opus 5 note; the cause is une…
jwbron 95ca721
[jwies/review-pre-agent-staging] review: don't tell the orchestrator …
jwbron ca109de
[jwies/autofix-v1] Merge branch 'jwies/review-pre-agent-staging' into…
jwbron a8f80b4
[jwies/review-dispatch-gate] Merge remote-tracking branch 'origin/mai…
jwbron e3a22e5
[jwies/review-pre-agent-staging] Merge branch 'jwies/review-dispatch-…
jwbron 68c90bb
[jwies/autofix-v1] Merge branch 'jwies/review-pre-agent-staging' into…
jwbron 9037a9b
[jwies/autofix-v1] autofix: actually implement the guards the command…
jwbron 2369cf6
[jwies/autofix-v1] autofix: lead the description with the command, no…
jwbron 57618a5
Merge branch 'main' into jwies/autofix-v1
jwbron a0c17ac
[jwies/autofix-v1] autofix: `skip-ai-review` no longer disarms autofix
jwbron 7816541
autofix: fail closed when the thread fetch fails
jwbron 7aed595
[jwies/autofix-v1] autofix: the re-review is best-effort verification…
jwbron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- | ||
| "autofix": minor | ||
| --- | ||
|
|
||
| Add the `autofix` workflow: opt-in, one-shot fixing of the PR reviewer's own feedback. | ||
|
|
||
| Arm a PR with an `autofix: blocking` / `autofix: nits` label or an `/autofix [scope]` comment; the run fixes the reviewer's open threads in that scope, pushes one commit, replies in each thread, and removes the label. Both arming surfaces are peers resolving through one shared token vocabulary, and the trigger decides which is read, so a stale label cannot widen an explicit command. | ||
|
|
||
| Everything except the code edit is deterministic. `lib/stage.ts` runs as a pre-agent step and fetches the inputs before the agent starts; `lib/plan.ts` then resolves scope, checks review currency, builds the work list, and renders the commit trailer. The plan is final: the prompt may execute it or stop, never widen it. | ||
|
|
||
| Guards fail closed. Currency is checked per file so one unrelated push doesn't refuse the whole run; unparseable labels, outdated anchors, threads a human opened, an unreadable diff, and a head that moves mid-run are all excluded. Refusal is reserved for a PR with no review at all, and for a thread fetch that fails: GitHub reports GraphQL rate limits and node-access failures as HTTP 200 with an `errors` array, so staging treats any `errors` entry or an unparseable body as fatal rather than as "this PR has no threads", which would clear the arming label while the findings it was armed for stayed open. | ||
|
|
||
| The reviewer's `skip-ai-review` label does not disarm autofix. It stops the reviewer's next run without withdrawing a review already posted, so a labelled PR can still carry current findings, and an explicit `autofix:` label or `/autofix` from someone with write access is the authorisation to act on them. A PR with no review is still refused, by the guard that checks for one. | ||
|
|
||
| The push uses `KHAN_ACTIONS_BOT_TOKEN`, because GitHub creates no workflow runs for `GITHUB_TOKEN`-triggered events, and the re-review of the autofix commit is the intended verification a fix gets. That verification is best-effort rather than guaranteed: the chain from the push to a posted review has several links, whether a break is visible depends on how the consumer triggers its reviewer, and the human re-arming loop is the accepted backstop for v1. The run's summary comment says so on every push. Ships with a documented workaround for gh-aw's unbounded PR-branch fetch, which is otherwise fatal on large monorepos. | ||
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
note (non-blocking): this changeset now documents the fail-closed thread-fetch refusal, but
workflows/autofix/README.md(~lines 108–112) wasn't synced to match.The README's refusal contract still says every refusal "clears any label that armed it" and frames a PR with no review as the only currency state that refuses. This commit's new refusal deliberately does the opposite — staging throws before the plan runs, so the arming label is intentionally retained for retry — and the README's "What it refuses to do" list omits the path entirely. Worth a one-paragraph README update so the prose doesn't contradict the shipped behavior.