qodo-gate: re-evaluate when Qodo posts its review comment - #2272
Conversation
Follow-up to #2269, from Qodo's own review of it. That PR taught the gate to accept an issue comment headed "Code Review by Qodo" as proof of review, which is the only proof a zero-finding PR ever produces. It did not give the workflow a reason to look again when that comment arrives. So the gap only moved. On a clean PR the run fired at `opened` fails — Qodo has not answered yet — the comment lands a minute or two later, and nothing re-triggers the check. It stays red until an unrelated push. That is exactly how #2269 itself went green: a follow-up commit, not the review. The PR that introduced the detection fix hid its own trigger gap. Adds the issue_comment trigger, plus the plumbing it needs: - a job guard, since issue_comment also fires for plain issues, which have no PR to gate. Every other trigger here is PR-only. - the PR number from github.event.issue.number, as issue_comment carries `issue` rather than `pull_request`; for a PR comment the issue number is the PR number. - a head sha lookup in the sweep step, which reads it from the pull_request payload that issue_comment does not have. Resolved from the PR instead, and still best-effort: a failed lookup skips the sweep rather than turning a PASS into a FAIL. The sweep matters most on precisely this trigger — it is the one that clears a clean PR's earlier red run off the commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR Summary by QodoRe-run qodo-gate when Qodo posts its PR review comment
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
Second finding from Qodo on this PR, and a fair one: the issue_comment
trigger as first written fired for ANY comment on ANY pull request. The
guard only established that the thing commented on was a PR.
That makes a required check into an API-heavy workflow, holding
`actions: write` for the rerun sweep, that any permitted commenter could
run as often as they liked — CI noise, and needless rate-limit exposure
on a check that blocks merges.
The guard now also requires the comment to be the bot's, and to carry the
"Code Review by Qodo" header. Both matter:
- author: only Qodo's own comment is evidence Qodo reviewed. A human
typing /review still works — Qodo answers, and its answer triggers
this.
- body: the bot also posts "Qodo is busy working", written before it
has read anything, and "PR Summary by Qodo", which is /describe
output. Neither means the diff was reviewed. Matching the review
header also keeps this to one run per review instead of three.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed in the latest push — a fair catch, and I had left the guard doing only half its job. The guard established that the commented-on thing was a PR, and stopped there. So It now also requires the comment to be the bot's and to carry the
On your severity framing: I agree it is worth fixing, though the practical exposure was bounded — |
Problem
Found by Qodo's own review of #2269 — a correct finding against the fix in that PR.
#2269 taught the gate to accept an issue comment headed
Code Review by Qodoas proof of review, since that is the only proof a zero-finding PR ever produces. It did not give the workflow a reason to look again when that comment arrives.So the gap only moved:
qodo-gateruns → Qodo has not answered yet → fail (correct)Code Review by Qodoa minute or two later — as an issue commentissue_commentwas not a trigger, so nothing re-evaluates the gateThe irony is that #2269 could not have caught this itself: it went green because I pushed a second commit, which re-triggered the gate at a point where the comment already existed. A
synchronizeevent masked a missingissue_commentevent.Fix
Adds the
issue_commenttrigger, plus the plumbing that payload requires:issue_commentalso fires for plain issues, which have no PR to gate. Every other trigger here is PR-only, sogithub.event_name != 'issue_comment' || github.event.issue.pull_requestis the whole condition.github.event.issue.number.issue_commentcarriesissue, notpull_request; for a comment on a PR the issue number is the PR number.github.event.pull_request.head.sha, whichissue_commentdoes not carry. Now looked up from the PR when empty, and still best-effort — a failed lookup skips the sweep rather than turning a PASS into a FAIL.The sweep matters most on exactly this trigger: it is the one that clears a clean PR's earlier red run off the head commit, which branch protection's rollup would otherwise keep counting.
Evidence
Review configuration only; nothing enters an image.
Root cause is reproducible by inspection:
on:had noissue_comment, while the detection logic added in qodo: unbreak the config loader, and stop the gate blocking clean PRs #2269 depends on a comment.Workflow parses; triggers are
pull_request,pull_request_review,pull_request_review_comment,issue_comment; both steps resolvePRfrom either payload shape.This PR cannot test the path it fixes, and its own gate will stay red. I initially expected it to self-heal; it will not. GitHub runs
issue_comment-triggered workflows from the default branch, not from the PR head — so the new trigger is inert until this merges. Observed directly here: Qodo postedPR Summary by Qodo(an issue comment) at16:01:26Zand noqodo-gaterun appeared for it; the only run is thepull_requestone that failed at open. The same is true of thepull_request_reviewtriggers, which is why those runs sat queued forever on qodo: review standards, compliance gates, and a PR template #2268 while onlypull_requestruns completed.So merging this needs the same manual nudge qodo: unbreak the config loader, and stop the gate blocking clean PRs #2269 needed — a push, or the
skip-qodo-gatelabel. That is the bootstrap problem in a sentence: the fix for "the gate cannot self-heal" cannot itself self-heal.Real verification is the first clean PR opened after this merges: its
qodo-gateshould go red at open and turn green on its own once Qodo comments, with no push. If it stays red, this fix is wrong.Also confirmed while verifying #2269's merge: the
.pr_agent.tomlloader fix works. A/reviewon master's config produced a normal review with nofailed to apply 'local' repo settingscomment, so the custom standards are live for the first time.Scope
general/overlay/; no package sources changed🤖 Generated with Claude Code