Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Versions follow [SemVer](https://semver.org).

## [Unreleased]

- The advisory review covers exactly the head its triggering event named (`REVIEW_HEAD_SHA`, #462). If the PR has moved off that head, nothing is reviewed or posted. Otherwise the diff is read between fixed commits, every envelope records the reviewed `head_sha`, and the review is attached to that commit, with no fallback comment once the head has moved. The summarizer skips clean when its lenses read different heads. For a fork PR (`REVIEW_CHECKOUT_BASE=1`) the brief says the tree is the base branch and the change is only in the diff. Upgrading: envelopes without `head_sha` (from an older reviewer ref) still post; callers that do not pass REVIEW_HEAD_SHA behave as before.
- The advisory review covers exactly the head its triggering event named (`REVIEW_HEAD_SHA`, #462). If the PR has moved off that head, nothing is reviewed or posted. Otherwise the diff is read between fixed commits, every envelope records the reviewed `head_sha`, and the review is attached to that commit. When GitHub refuses its inline comments, the fallback is a body-only review on the same commit, never an unpinned comment (#569). The summarizer skips clean when its lenses read different heads. For a fork PR (`REVIEW_CHECKOUT_BASE=1`) the brief says the tree is the base branch and the change is only in the diff. Upgrading: envelopes without `head_sha` (from an older reviewer ref) still post; callers that do not pass REVIEW_HEAD_SHA behave as before.
- When check runs cannot be read on a token deployment, the warning now says the token cannot read them (fine-grained tokens have no checks permission) instead of asking for an App permission.

## [0.5.0] - 2026-10-23
Expand Down
31 changes: 15 additions & 16 deletions src/outerloop/posting.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,11 @@ def post_round_review(
fallback_body — the FULL single-comment rendering, because the review
body alone may say no more than "findings are attached" while the
findings live in the rejected inline payload. `commit_id` pins the
review to the head that was reviewed, so a push meanwhile cannot move it."""
review to the head that was reviewed, so a push meanwhile cannot move it;
its fallback is a body-only review pinned to the same head, never an
unpinned comment."""
stamp, round_label = _round_stamp(client, repo, number, marker, pr_data, reviewed_by)
full = fallback_body.replace(marker, f"{marker}\n{stamp}", 1)
try:
client.create_pr_review(
repo,
Expand All @@ -179,21 +182,17 @@ def post_round_review(
commit_id=commit_id,
)
except EXPECTED_FAILURES as exc:
if commit_id:
# a push can be why the pinned review was refused; an unpinned
# comment would land the old findings on the new head
head = client.get_pull_request(repo, number).get("head")
current = str(head.get("sha", "")) if isinstance(head, dict) else ""
if current != commit_id:
log.warning(
"pinned review failed (%s) and the head moved (%s -> %s); nothing posted",
exc,
commit_id[:8],
current[:8] or "unknown",
)
return ""
log.warning("inline review failed (%s); falling back to a comment", exc)
client.comment(repo, number, fallback_body.replace(marker, f"{marker}\n{stamp}", 1))
if not commit_id:
log.warning("inline review failed (%s); falling back to a comment", exc)
client.comment(repo, number, full)
return round_label
log.warning("pinned inline review failed (%s); retrying without inline comments", exc)
try:
client.create_pr_review(repo, number, full, [], commit_id=commit_id)
except EXPECTED_FAILURES as exc2:
# the head left the PR (force-push); nothing is posted
log.warning("pinned review to %s refused (%s); nothing posted", commit_id[:8], exc2)
return ""
return round_label


Expand Down
35 changes: 29 additions & 6 deletions tests/test_posting.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,18 +188,41 @@ def create_pr_review(
assert posted["kind"] == "comment" and "FULL findings in the fallback" in posted["body"]


def test_pinned_review_has_no_fallback_once_the_head_moved() -> None:
# a push can be why GitHub refused the pinned review; a plain comment
# would put the old findings on the new head
def test_pinned_review_falls_back_to_a_body_only_review_on_the_same_head() -> None:
class _NoInline(_FakeClient):
def create_pr_review(
self, repo: str, number: int, body: str, comments: Any = None, commit_id: str = ""
) -> None:
if comments:
raise GitHubError(422, "/repos/org/repo", "unprocessable")
self.posted.append({"body": body, "kind": "review", "commit_id": commit_id})

client = _NoInline()
label = posting.post_round_review(
client, # type: ignore[arg-type]
"org/repo",
1,
_MARKER,
f"{_MARKER}\nsummary only",
[{"path": "x.py", "line": 1}],
{"head": {"sha": "oldhead"}},
fallback_body=f"{_MARKER}\nFULL findings",
commit_id="oldhead",
)
assert label == "**Round 1**"
(posted,) = client.posted
assert posted["kind"] == "review" and posted["commit_id"] == "oldhead"
assert "FULL findings" in posted["body"]


def test_pinned_review_posts_nothing_once_the_head_left_the_pr() -> None:
# a plain comment would put the old findings on the new head
class _Moved(_FakeClient):
def create_pr_review(
self, repo: str, number: int, body: str, comments: Any = None, commit_id: str = ""
) -> None:
raise GitHubError(422, "/repos/org/repo", "commit_id is not part of the pull request")

def get_pull_request(self, repo: str, number: int) -> dict:
return {"head": {"sha": "newhead"}}

client = _Moved()
posting.post_round_review(
client, # type: ignore[arg-type]
Expand Down
Loading