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
42 changes: 33 additions & 9 deletions .github/workflows/advisory-review-agent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
# another caller job with a different backend/model/opinion label.
#
# Topology (every backend, claude included):
# session job — READ-ONLY permissions. Checks out the PR head (never
# executed), runs the review session, emits raw findings (or a skip-stub
# session job — READ-ONLY permissions. Checks out the PR head (the base
# commit for a fork; no workflow step runs PR code, though the session
# holds a shell), runs the review session, emits raw findings (or a skip-stub
# marker) as an artifact. The only secrets present are the session's own
# model key and read-scoped tokens (job token + the reviewer repo's
# read-only deploy key while private) — nothing worth stealing, so even
Expand All @@ -18,7 +19,9 @@
# the round stamp.
#
# Constraints: never reviews bot-authored PRs, never approves, never fails
# the caller's CI, same-repo PRs only. By design: the "Round N" counter is
# the caller's CI, same-repo PRs only (plus fork PRs a maintainer labels, with
# allow_labeled_forks on; the review then covers the labeled head or nothing).
# By design: the "Round N" counter is
# shared across all opinions on a PR (the number is cosmetic; the stamp names
# the reviewer), and a missing/expired backend key posts a could-not-run stub
# naming its opinion label on each triggering PR until fixed — the noise IS the
Expand All @@ -28,6 +31,17 @@ name: advisory-review-agent
on:
workflow_call:
inputs:
allow_labeled_forks:
description: >-
Review a fork PR when the triggering event is a label being applied.
Applying a label needs triage access, so the label is the maintainer's
approval; filter the label name in the caller's `if:`. The fork's head
is never checked out (the session reads the base tree plus the diff),
but its diff reaches the model, and its key, through the prompt. Needs
a reviewer_ref that pins the labeled head (0.5.1 or later).
type: boolean
required: false
default: false
bot_login:
description: Login of your bot account — its PRs are never reviewed. Required.
type: string
Expand Down Expand Up @@ -153,16 +167,23 @@ jobs:
continue-on-error: true
# Backstop a hung session so it can't burn the 360-min default runner budget.
timeout-minutes: 70
# Fork PRs must not reach any secret. `pull_request_target` runs in the
# base repo's context, so this gate is what closes the pwn-request hole.
if: github.event.pull_request.head.repo.full_name == github.repository
# Fork PRs must not reach any secret unasked. `pull_request_target` runs in
# the base repo's context, so this gate is what closes the pwn-request hole;
# a maintainer's label opens it per PR when the caller opts in. A fork's
# code is never checked out (the session gets the base tree and the diff).
if: >-
github.event.pull_request.head.repo.full_name == github.repository ||
(inputs.allow_labeled_forks && github.event.action == 'labeled')
# READ-ONLY: the session runs here, so the job token must not be worth
# stealing. pull-requests:read lets the CLI fetch the PR metadata/diff.
permissions:
contents: read
pull-requests: read
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
# the head the triggering event saw; the review covers it or nothing
REVIEW_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
REVIEW_CHECKOUT_BASE: ${{ github.event.pull_request.head.repo.full_name != github.repository && '1' || '' }}
BACKEND: ${{ inputs.backend }}
steps:
# Checks out the reviewer, never the pull request's code for execution.
Expand All @@ -176,10 +197,10 @@ jobs:
ssh-key: ${{ secrets.checkout_ssh_key }}
path: .autoresearch
persist-credentials: false
- name: Check out the PR head (read-only; never executed)
- name: Check out the PR head, or the base for a fork (read-only)
uses: actions/checkout@v7
with:
ref: refs/pull/${{ env.PR_NUMBER }}/head
ref: ${{ env.REVIEW_CHECKOUT_BASE == '1' && github.event.pull_request.base.sha || env.REVIEW_HEAD_SHA }}
path: pr-head
persist-credentials: false
- uses: astral-sh/setup-uv@v10.3.0
Expand Down Expand Up @@ -291,7 +312,9 @@ jobs:
needs: session
# inputs.post=false -> emit-only: a lens opinion whose artifact the
# summarizer merges; nothing is posted from this call.
if: ${{ inputs.post && !cancelled() && github.event.pull_request.head.repo.full_name == github.repository }}
if: >-
${{ inputs.post && !cancelled() && (github.event.pull_request.head.repo.full_name == github.repository ||
(inputs.allow_labeled_forks && github.event.action == 'labeled')) }}
continue-on-error: true
timeout-minutes: 15
# The write half: no model session runs in this job.
Expand All @@ -300,6 +323,7 @@ jobs:
pull-requests: write
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEW_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
steps:
- name: Check out the reviewer
uses: actions/checkout@v7
Expand Down
17 changes: 15 additions & 2 deletions .github/workflows/advisory-review-summarize.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ name: advisory-review-summarize
on:
workflow_call:
inputs:
allow_labeled_forks:
description: >-
Summarize a fork PR when the triggering event is a label being applied
(set it the same as on the lens calls).
type: boolean
required: false
default: false
bot_login:
description: Login of your bot account — its PRs are never reviewed. Required.
type: string
Expand Down Expand Up @@ -81,14 +88,17 @@ jobs:
runs-on: ubuntu-latest
continue-on-error: true
timeout-minutes: 30
if: github.event.pull_request.head.repo.full_name == github.repository
if: >-
github.event.pull_request.head.repo.full_name == github.repository ||
(inputs.allow_labeled_forks && github.event.action == 'labeled')
# READ-ONLY, like the reviewer session job: the merge session must not
# run next to a write token.
permissions:
contents: read
pull-requests: read
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEW_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
BACKEND: ${{ inputs.backend }}
steps:
- name: Check out the reviewer
Expand Down Expand Up @@ -194,14 +204,17 @@ jobs:
post:
runs-on: ubuntu-latest
needs: summarize
if: ${{ !cancelled() && github.event.pull_request.head.repo.full_name == github.repository }}
if: >-
${{ !cancelled() && (github.event.pull_request.head.repo.full_name == github.repository ||
(inputs.allow_labeled_forks && github.event.action == 'labeled')) }}
continue-on-error: true
timeout-minutes: 15
permissions:
contents: read
pull-requests: write
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEW_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
steps:
- name: Check out the reviewer
uses: actions/checkout@v7
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +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.
- 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
23 changes: 22 additions & 1 deletion src/outerloop/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,23 @@ def get_pull_request_diff(self, repo: str, number: int) -> str:
)
return self.raw_transport(request)

def get_compare_diff(self, repo: str, base: str, head: str) -> str:
"""The unified diff `base...head` (from their merge base, as a PR diff
is) between two fixed commits, so a later push cannot change it."""
path = (
f"/repos/{urllib.parse.quote(repo)}/compare/"
f"{urllib.parse.quote(base, safe='')}...{urllib.parse.quote(head, safe='')}"
)
request = urllib.request.Request(
f"{API}{path}",
method="GET",
headers={
"Authorization": f"Bearer {self.auth.token()}",
"Accept": "application/vnd.github.v3.diff",
},
)
return self.raw_transport(request)

def list_directory(self, repo: str, path: str, ref: str) -> list[dict[str, Any]]:
"""Entries of a directory at a ref ([] when absent or not a dir).

Expand Down Expand Up @@ -841,8 +858,10 @@ def create_pr_review(
number: int,
body: str,
comments: list[dict[str, Any]] | None = None,
commit_id: str = "",
) -> None:
"""Post a PR review with optional inline comments.
"""Post a PR review with optional inline comments, attached to
`commit_id` when given (else GitHub picks the PR's current head).

The event is hard-coded to COMMENT: this client must never be able
to approve or request changes — a Write-role bot review that blocks
Expand All @@ -861,6 +880,8 @@ def create_pr_review(
payload: dict[str, Any] = {"body": body, "event": "COMMENT"}
if comments:
payload["comments"] = comments
if commit_id:
payload["commit_id"] = commit_id
self._request(
"POST",
f"/repos/{urllib.parse.quote(repo)}/pulls/{number}/reviews",
Expand Down
25 changes: 23 additions & 2 deletions src/outerloop/posting.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,38 @@ def post_round_review(
pr_data: dict,
fallback_body: str,
reviewed_by: str = "",
commit_id: str = "",
) -> str:
"""The Reviews-API sibling of post_round: body summary plus anchored
inline comments, event COMMENT always (the client hard-codes it). A
posting failure falls back to a plain issue comment carrying
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."""
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."""
stamp, round_label = _round_stamp(client, repo, number, marker, pr_data, reviewed_by)
try:
client.create_pr_review(repo, number, body.replace(marker, f"{marker}\n{stamp}", 1), inline)
client.create_pr_review(
repo,
number,
body.replace(marker, f"{marker}\n{stamp}", 1),
inline,
commit_id=commit_id,
)
except EXPECTED_FAILURES as exc:
Comment thread
renmengye marked this conversation as resolved.
Resolved
Comment thread
renmengye marked this conversation as resolved.
Resolved
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)
Comment thread
renmengye marked this conversation as resolved.
Resolved
client.comment(repo, number, fallback_body.replace(marker, f"{marker}\n{stamp}", 1))
Comment thread
renmengye marked this conversation as resolved.
Resolved
return round_label
Expand Down
11 changes: 10 additions & 1 deletion src/outerloop/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ def build_agent_brief(
lens: str = "",
guidance: str = "",
panel: bool = False,
base_tree: bool = False,
) -> str:
"""The reviewer brief for an agent session: the shared rubric, the
investigation instruction, and the PR itself, built on the shared
Expand All @@ -376,14 +377,22 @@ def build_agent_brief(
raise ValueError(f"unknown review lens {lens!r} (have: {sorted(REVIEW_LENSES)})")
focus = f"\n\n{REVIEW_LENSES[lens]}" if lens and lens != "general" else ""
rules = f"\n\n{PANEL_BLOCKING}" if panel else ""
# a fork PR's head is never checked out: its code stays off the runner
tree = f"\n\n{BASE_TREE_NOTE}" if base_tree else ""
# the guidance rides the trusted prompt BEFORE the untrusted PR text, so a
# report cannot forge or swallow it
return (
f"{SYSTEM_PROMPT}{focus}{rules}{guidance_section(guidance)}\n\n"
f"{_agent_investigation(syscall_cmd)}\n\n{build_prompt(pr, today)}"
f"{_agent_investigation(syscall_cmd)}{tree}\n\n{build_prompt(pr, today)}"
)


BASE_TREE_NOTE = (
"The working tree is the pull request's base branch, not its head: the change exists "
"only in the diff below. Read the surrounding code in the tree and the change in the diff."
)


MEASUREMENT_PROVENANCE_CAUTION = (
"Numbers measured on different commits are not directly comparable; check the commits "
"in the checkout history before attributing a gap between them."
Expand Down
Loading
Loading