Conversation
…ts. Fixes #135 The check accepted only a marker for the current head commit, so every push after a review turned it red and forced another review to clear the status. One review per pull request is enough: any marker from an OWNER, MEMBER or COLLABORATOR review now counts, and the status names the reviewed commit and how many commits came after it when that is not the head. The marker for the head commit still wins when there is one. Without a trusted marker the check is red as before. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…review. Associates #135 Review findings on the first commit of this pull request: - The commit a review covers now comes from the review's server-set commit_id, not from marker text, so free-form text never reaches the compare API path and the "commits since" note cannot be steered by the reviewer. - A reviewed commit that is no longer an ancestor of the head (force-push, rebase) is reported as "commits since unknown" instead of a misleading count. - Dismissed reviews do not count; a later push no longer retires a review, so dismissal is the way to take one back. The readme says how a caller makes dismissal re-evaluate at once. - Comments that still described the head binding are reworded; the readme file table no longer calls the check "head-commit". Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
plamber
reviewed
Sep 30, 2026
plamber
left a comment
plamber
left a comment
Member
There was a problem hiding this comment.
No blocking findings; the gate change is sound once the marker text stops reaching an API path.
Reviewed at 3c35bc9 (code-reviewer and security-reviewer). Everything below was fixed in 0410b76 after this review, so the check says "1 commit since".
| Finding | Where | |
|---|---|---|
| 🟠 | Marker sha text went unvalidated into the compare API path (path traversal to other GETs, and any string turned the check green). Fixed: the review's server-set commit_id is used instead. |
scripts/pr-agent-review.mjs:311 |
| 🟠 | Dismissed reviews still counted, and a push no longer retires a review. Fixed: dismissed reviews are skipped; the readme says how a caller re-evaluates on dismissal. | scripts/pr-agent-review.mjs:272 |
| 🟠 | readme.md still called the check "head-commit" and said a review exists "for this commit". Fixed. |
readme.md:240 |
| ⚪ | A diverged compare (force-push) understated the change. Fixed: reported as "commits since unknown". | scripts/pr-agent-review.mjs:311 |
| ⚪ | Comments still described the head binding and one was badly wrapped. Fixed. | scripts/pr-agent-review.mjs:56,123,217 |
No token, permission or trigger change; merge-queue behaviour is unchanged. The accepted weak spot (small first commit reviewed, large change pushed later) is not a finding.
Good: the head marker still wins over a newer old one, and a failed compare degrades to "unknown" instead of failing the gate.
Generated by Claude Code
plamber
self-requested a review
September 30, 2026 10:35
plamber
approved these changes
Sep 30, 2026
3 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue Reference
Fixes #135
Summary
agent-reviewcompared each review marker'sshawith the pull request's head commit, so every push after a review turned the check red and forced another/el-reviewjust to clear it. Decision: one review per pull request is enough (the weak spot, a small first commit reviewed and a large change pushed afterwards, is accepted).Reviewed by @user at fd0066f, 2 commits since (model ..., effort ..., n finding(s)). If the comparison cannot be made (commit gone after a force-push) it sayscommits since unknown; that never fails the check.Companion change in the plugin (rules and skills): EasyLife365/.github-private#220.
SDLC Stage
Build
Testing Evidence
Ran the real script against a local fake GitHub API, one process per scenario: marker on head; older marker with 2 commits since; with 1 commit since (singular); older marker with the compare call failing; head marker preferred over a newer old marker; no review; reviewer requested before any review; untrusted marker only; approval without a marker; marker without a sha; draft; exempt author. All 12 give the expected state and description, exit code 0 and exactly one status post. The workflow files have no tests of their own;
node --checkpasses.Not verified: a real run on a pull request. That happens after merge, when callers pick up the new script from
main; the first proof is a push after a review on any wired repository, which should now keepagent-reviewgreen with the "commits since" text.Changelog
Changed:
agent-reviewstays green after later pushes once a trusted review exists, and shows how many commits came after the reviewed one.Documentation Impact
Readme and workflow comments updated in this PR.
Checklist
🤖 Generated with Claude Code
Review status
/el-review(code-reviewer and security-reviewer) ran once, on3c35bc9; its marker stands on that commit. Its findings were fixed afterwards.Changed after review
0410b76answers all five findings: the commit a review covers now comes from the review's server-setcommit_id(no marker text in the compare API path), a diverged compare is reported as unknown, dismissed reviews are ignored (the readme says how a caller re-evaluates on dismissal), and the head-binding comments and the readme file table are reworded. The harness now has 16 scenarios (adds diverged compare, traversal text in the marker, marker text claiming the head while the review is on an older commit, dismissed review); all pass.Not run: a re-review of
0410b76. The status will say1 commit since.Rollout note: callers use
@main, so the looser gate reaches every wired repository when this merges.agent-reviewis not a required status anywhere today (see docs/agents/pr-review.md), so nothing can be blocked or newly let through by it yet.