Repository navigation
Advisory review for labeled fork PRs, pinned to the labeled head - #566
Conversation
…e head_sha, post refusal and commit_id; base-tree brief note for forks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Round 1 — reviewed head 57a2da1b — reviewer hermes/gpt-5.6-terra.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
A rejected pinned review can fall back to an unpinned stale comment.
There was a problem hiding this comment.
Round 2 (re-run on the same head) — reviewed head 57a2da1b — reviewer summarizer:hermes/gpt-5.6-terra over coverage+credentials+deployment+general+lifecycle+prose.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: 2 blocking, 2 advisory.
3 findings attached to the lines below.
A push during post can format comments from the new head. [general] pull_request reads PR metadata, then fetches the live PR diff before the code compares envelope_head with the earlier metadata; a push between those calls lets format_review build B's line anchors and create_pr_review attach them to A. (src/outerloop/review_post_cli.py:64; high confidence)
Verdict: block. The pinned-review fallback can publish stale findings after a moved head, and formatting can race a head change before the comparison. Mixed legacy/pinned envelopes can also be labeled with the wrong head. Rejected as duplicates: [credentials] review_post_cli.py:132 identifies the same unguarded fallback recorded at posting.py:181; [lifecycle] posting.py:181 is the same fallback defect; [coverage], [deployment], and [prose] posting.py:181 agree with the merged fallback finding rather than adding distinct defects.
… findings cannot be merged under a pinned head; shorter changelog entry
renmengye
added
outerloop:review
There was a problem hiding this comment.
Round 3 — reviewed head 23587e30 — reviewer hermes/gpt-5.6-terra.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: nothing blocking — 1 advisory note.
1 finding attached to the lines below.
The pinned-review fallback can still post stale findings after a push.
… base commit; REVIEW_HEAD_SHA to both jobs
renmengye
added
outerloop:review
There was a problem hiding this comment.
Round 4 — reviewed head 3f09e11b — reviewer hermes/gpt-5.6-terra.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
The fallback comment still has a race after the pinned review check.
Closes #462: fork PRs get the advisory review once a maintainer applies the review label, and the review covers exactly the head that label event named.
Not ready to merge until the workflow commit lands: the
.github/workflowshalf is a patch the maintainer applies (the agent's tooling cannot write.github/); it will arrive as a second commit on this branch.Python side (this commit)
REVIEW_HEAD_SHA(the event'spull_request.head.sha): the session skips clean, running no model, if the PR has moved off it; otherwise it reads the diff between the base and that head as fixed commits (compare API), so a push after the metadata read cannot swap in another head's diff.head_sha. The post job posts nothing when the envelope's or the event's head differs from the PR's current head, and attaches the review to the reviewed commit (commit_id), so a push during posting cannot move it.head_shaand skips clean if they disagree.REVIEW_CHECKOUT_BASE=1adds a brief note that the tree is the base branch and the change is only in the diff.Workflow side (patch, to be applied)
allow_labeled_forksinput (default false) on the reviewer and summarizer workflows; with it, the session/summarize and post jobs also run for a fork PR when the event islabeled. The label-name filter stays in the caller'sif:, as today.allow-unsafe-pr-checkout). Same-repo PRs check out the event's head SHA instead ofrefs/pull/N/head.REVIEW_HEAD_SHAon every job; the input's description says it needs a reviewer ref with this commit (0.5.1 or later).Compatibility
Envelopes cross a job boundary. Envelopes without
head_sha(an older reviewer ref) still post; callers that pass noREVIEW_HEAD_SHAbehave as before;allow_labeled_forksdefaults off. Same-repo behavior change: a push during a review session now means that round posts nothing (before, its findings were stamped onto the new head); re-applying the label reviews the new head.Deferred
reviewer_refwithallow_labeled_forkson does not pin the head; documented on the input.Testing
Full gate: ruff, format, mypy, pytest (3176 passed, 7 skipped). Focused tests: moved head skips before the session; unset variable unchanged; pinned diff read between fixed commits; post refuses a stale envelope head and a moved event head; post pins
commit_id; summarizer disagreement. The workflow patch was checked for YAML validity and applies cleanly to main; no live label run yet.Built by DeepSeek-V4.1-Flash (head pinning) and Claude Opus (codex's fixes, workflow patch). Reviewed by codex (three rounds) and Claude.