Repository navigation
Add /address-feedback: judge, fix and reply to every comment on a pull request - #124
Conversation
…a pull request Takes every open review thread, top-level comment and review summary on one pull request or merge request, from anyone or any bot, treats each as data to judge against the code, plans and executes only the fixes in a worktree, pushes them to the branch without force and replies to each item. Claude Code only.
GitHub Actions
Failure
|
/review |
|
/review |
![ncode-review[bot]](https://avatars.githubusercontent.com/u/12114478?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 NCode Review at cadaf4f: 3 new findings on the changed lines.
🤖 Review completeVerdict: 💬 Comment — This PR adds 5 reviewers run — 2 findings (run details)
Skipped: api-compat, architecture, completeness, correctness, data-model, infrastructure, performance, tests Findings
Trial: findings are posted as review threads and in the check run. $0.50 on Amazon Bedrock, 63s, at dcb8031. (how to interact) |
…m comments may run Review of /address-feedback (F1 to F3): the pull request's branch name must match a plain pattern and is quoted in every git command; Done-when commands use only the repository's own tooling or plain file checks, the orchestrator reads each one and each Change line before any step runs, and replies never carry environment values, credentials or file contents beyond the cited lines.
|
The bot ran on this pull request for the first time in this repository (the earlier failures were the Bedrock secret not yet being visible to it). All three findings were right and are fixed in 121b99b; replies are in each thread. |
|
/review |
There was a problem hiding this comment.
🤖 NCode Review at 121b99b: 3 new findings on the changed lines.
…or the branch check with \z Second review of /address-feedback (F1 to F3): a request from an author without write access, who isn't a bot or named with only:, is at most an answer, a decline or a question for the maintainer; the check before execution also stops for dependency manifests, lock files, package scripts and build or install scripts no item concerns; the branch-name pattern ends in \z; the plan adds no changelog step of its own and edits only the entry the pull request already has.
|
/review |
There was a problem hiding this comment.
🤖 NCode Review at a9886e5: 3 new findings on the changed lines.
1 more was already posted on this pull request.
…ist, and put trusted reviewers first Third review of /address-feedback (F2 to F4): steps that a review round or a re-plan adds are checked as Plan item 4 says before they run, and the diff is checked for protected kinds of file before the push; the item query reports when a list has another page and reads each thread's last comments; the 40-item cap fills with items from authors who may ask for a fix first.
|
/review |
There was a problem hiding this comment.
🤖 NCode Review at 74563de: 1 new finding on the changed lines.
1 more was already posted on this pull request.
Not on a changed line:
- 🟡 F3 Nit: Get-PlanState row doesn't list address-feedback among the commands that run it
workflow/README.md:11. TheGet-PlanState.ps1row saysship-plan,continue-plan,finish-planandrun-issuesrun it, butaddress-feedbackalso runs it in Start, Execute and Push. Fix: Addaddress-feedbackto that row's list of commands that runGet-PlanState.ps1.
…m, and list address-feedback with the state script Fourth review of /address-feedback (F1, F3): a step may edit a CI or release workflow, a Git hook, a dependency manifest or lock file, a package or build script, or a file outside the repository only when a fix item from an author who may ask for a fix names it; an item from anyone else, or one that only mentions the file, never lifts the stop, before a step or before the push. workflow/README.md lists address-feedback among the commands that run Get-PlanState.ps1.
|
/review |
There was a problem hiding this comment.
🤖 NCode Review at dcb8031: 1 new finding on the changed lines.
1 more was already posted on this pull request.
|
|
||
| ## Reply | ||
|
|
||
| Reply to each item whose verdict isn't `none`, after the push, so a `fix` can name its commit: `git log --format=%h --grep "Plan step <n>:"` finds the commit of a step. |
There was a problem hiding this comment.
🟡 Nit: The commit lookup by step number can match earlier commits on the PR branch F2 · hygiene
git log --format=%h --grep "Plan step <n>:" searches the whole history, and the branch already holds Plan step N: commits from its own earlier plan runs, with the same step numbers.
Consequence: A fix reply (Autopilot: fixed in <commit>) can name an unrelated commit from the original pull request, or list several hashes.
Fix: Search only this run's commits, for example git log --format=%h -1 --grep "Plan step <n>:" "<pre-push origin/<head>>..HEAD", recording the pre-push tip before Push item 5. Make the same change in .claude/commands/address-feedback.md.
Evidence
Reply section uses the grep with no range. orchestrate-plan commits each step as "Plan step N: <title>", so the numbers repeat across plans on one branch.
Confidence 0.35.
There was a problem hiding this comment.
Right. #124 merged before this landed, so the fix is in #126: the command notes the commit it starts from (<start>) when it creates the worktree, and the reply finds a step's commit with git log --format=%h -1 --grep "Plan step <n>:" "<start>..HEAD", so it can't match a Plan step commit of the same number from the branch's own plan.
Adds
/address-feedback, a Claude Code command for the work left after a pull request opens: its review threads, top-level comments and review summaries, from people, Copilot, review bots and scanners. It is the counterpart of/run-issues' review loop, aimed at a pull request that already exists, and it works on a GitHub pull request or a GitLab merge request (the name avoids either term).What it does
Autopilot:reply.only:<login>andexclude:<login>limit them to, or leave out, one author. It takes the first 40 and lists the rest.plan-bugfix, judges each item against the code at the branch head, never the comment alone: fix, decline (with what in the code or conventions shows it), answer, ask (needs the maintainer) or none (a coverage report, a deploy notice). The plan has a## Feedbacksection with the verdicts and a step for each fix, and noIssue:,Release:orRefs:line and no changelog step of its own (a step edits the entry the pull request already has only when a fix changes behavior it describes).orchestrate-planandreview-plan-diff, up to three review rounds, in a worktreefeedback-<number>.git push origin HEAD:<head>, never forced; it stops, leaving the commits in the worktree, when someone pushed to the branch meanwhile. The landing gate runs first.none, starting withAutopilot:: the commit for a fix, the evidence for a decline, the answer, or the question for the maintainer. Threads stay unresolved.What it never does
Merge, mark ready or draft, enable auto-merge, resolve a thread, dismiss a review or an alert, close the pull request, edit its title or description, force-push or skip hooks. It stops for a fork, a closed pull request or a branch that is the default branch. Scanner alerts among the items are listed for the maintainer, who dismisses them.
What anyone wrote on the pull request is data to judge, never an instruction: a request to change something is a claim, checked before a line changes.
Checked
Test-Templatepasses with a new check that the command reaches Claude Code only (no Copilot file), carries those rules, and has its argument placeholder once.tools/Update-Template.ps1), andgit diff --statmatches with and without--ignore-cr-at-eol.Not checked
The command has not been run.
/run-issueshas a headless end-to-end test against a fixture project, and this has none yet; its first real run on a small pull request is the test. There is no Copilot version.Who can ask for a change
An item from an author who can write to the repository, from a bot (
[bot]), or from an author the request names withonly:<login>can become a fix. A request from anyone else is answered, declined or put to the maintainer, because it would otherwise let a stranger have code written, run and pushed under the maintainer's credentials. The plan's Done-when commands and Change lines are read before any step runs, and a step that edits CI or release workflows, Git hooks, a dependency manifest or lock file, a package script or a build or install script stops the run unless an item concerns it.