Skip to content

fix(pr-review): read explicit exact targets directly #4876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
huangruiteng merged 1 commit into from
Sep 21, 2026
Merged

fix(pr-review): read explicit exact targets directly #4876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 14 additions & 1 deletion examples/pr-review-command-smoke.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def main() -> int:
"Never send the projection ACK before the Todo exists",
"Generic `re-review`, `重新review`, and `复审` wording selects the named PR; it is not a force-refresh token.",
"the row stays in `pull_requests` inventory but must not appear in `review_sequence`",
"--target-exact-head NUMBER@HEAD_OID",
):
assert phrase in skill_text, phrase
assert len(skill_source.splitlines()) <= 180, len(skill_source.splitlines())
Expand Down Expand Up @@ -229,7 +230,7 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
assert request["command"] == "/loopx-pr-review", request
assert (
request["cli_command"]
== "loopx pr-review [--repo owner/repo] [--state open|merged|all] [--review-priority other-developers-first|owner-first] [--since ISO]"
== "loopx pr-review [--repo owner/repo] [--target-exact-head NUMBER@HEAD_OID] [--state open|merged|all] [--review-priority other-developers-first|owner-first] [--since ISO]"
), request
assert request["privacy_mode"] == "public_safe_github_metadata", request
assert request["dry_run"] is True, request
Expand All @@ -253,6 +254,18 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
assert payload["summary"]["total_pr_count"] == 4, payload["summary"]
assert payload["summary"]["open_pr_count"] == 3, payload["summary"]
assert payload["summary"]["merged_pr_count"] == 1, payload["summary"]
target = payload["pull_requests"][0]
exact_target = f"{target['number']}@{target['head_oid']}"
targeted = json.loads(
run_cli(
"--format", "json", "pr-review", "--fixture", str(FIXTURE),
"--target-exact-head", exact_target,
).stdout
)
assert targeted["request"]["target_exact_heads"] == [exact_target], targeted
assert targeted["result_completeness"]["complete"] is True, targeted
assert targeted["result_completeness"]["limit_scope"] == "exact_targets", targeted
assert [item["number"] for item in targeted["pull_requests"]] == [target["number"]]
assert payload["summary"]["post_merge_review_count"] == 1, payload["summary"]
assert payload["summary"]["review_attention_count"] == 3, payload["summary"]
assert payload["summary"]["draft_count"] == 1, payload["summary"]
Expand Down
16 changes: 15 additions & 1 deletion loopx/capabilities/pr_review_queue/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ workflow or the merge-focused `loopx-pr-merge` skill.

| Command | CLI reference | Intent |
| --- | --- | --- |
| `/loopx-pr-review` | `loopx pr-review [--repo owner/repo] [--state open\|merged\|all] [--review-priority other-developers-first\|owner-first] [--since ISO] [--fresh-audit-exact-head NUMBER@HEAD_OID]` | List open and merged PRs for the current project or explicit repository, provide concrete main-regression analysis for each actionable PR, and include a blank five-block template that agentloop fills after reading the selected PR body/diff. The default prioritizes non-owner developer PRs; `owner-first` opts into owner priority. A typed exact-head option is required to re-audit an unchanged concluded head. |
| `/loopx-pr-review` | `loopx pr-review [--repo owner/repo] [--target-exact-head NUMBER@HEAD_OID] [--state open\|merged\|all] [--review-priority other-developers-first\|owner-first] [--since ISO] [--fresh-audit-exact-head NUMBER@HEAD_OID]` | Review a small explicit batch with repeatable `--target-exact-head`, or list a lifecycle queue when no target is supplied. Both paths provide concrete main-regression analysis and the five-block review contract. The default queue prioritizes non-owner developer PRs; `owner-first` opts into owner priority. `--fresh-audit-exact-head` separately forces new evidence for an unchanged concluded head. |
| pre-merge readback | `loopx pr-review --repo owner/repo --check-merge-readiness NUMBER@HEAD_OID` | Immediately before merge, fail closed unless the remote PR is still open at the reviewed head, its standalone conclusion approves that head, all checks are successful or skipped, review-thread pagination is complete with no unresolved thread, and merge state is compatible. This read grants no merge authority. |

The slash command must run the CLI first. Agentloop must not reconstruct the
Expand All @@ -62,6 +62,20 @@ templates enter the model context:
loopx --format json pr-review --state all [--repo owner/repo] [--since ISO]
```

When the user explicitly names one or a few PRs, resolve each current head and
request only those exact heads. This direct path is complete for the named
targets and must not be expanded into a historical queue merely to satisfy
queue completeness:

```bash
loopx --format json pr-review --repo owner/repo \
--target-exact-head 4868@0123456789abcdef0123456789abcdef01234567
```

The option is repeatable. A remote head mismatch fails closed. Use
`--fresh-audit-exact-head` in addition only when an unchanged target already has
a valid conclusion and the user explicitly requests new evidence.

The live source scan keeps the list query lightweight and enriches each PR's
nested commits, reviews, and checks with a bounded pool of concurrent
`gh pr view` reads (at most eight at a time). Results are reassembled in list
Expand Down
96 changes: 96 additions & 0 deletions loopx/capabilities/pr_review_queue/github_source.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from pathlib import Path
from typing import Any, Callable

from .selection_execution import normalize_fresh_audit_exact_heads

GitHubJsonRunner = Callable[..., Any]

DETAIL_FIELDS = (
Expand All @@ -21,6 +23,26 @@
"reviews",
)

PR_LIST_FIELDS = (
"number",
"title",
"url",
"state",
"isDraft",
"headRefName",
"headRefOid",
"baseRefName",
"author",
"createdAt",
"updatedAt",
"closedAt",
"mergedAt",
"mergeCommit",
"changedFiles",
"additions",
"deletions",
)


def run_gh_json(args: list[str], *, cwd: Path | None = None) -> Any:
proc = subprocess.run(
Expand Down Expand Up @@ -140,6 +162,80 @@ def attach_pr_review_details(
return True


def scan_github_pull_request_targets(
*,
repository: str,
exact_heads: Sequence[str],
cwd: Path | None = None,
run_gh_json: GitHubJsonRunner = run_gh_json,
wait_for_ci: bool = True,
) -> dict[str, Any]:
"""Read only explicitly requested exact heads, without scanning a queue."""

targets = normalize_fresh_audit_exact_heads(exact_heads)
if not targets:
raise ValueError("at least one target exact head is required")

pull_requests: list[dict[str, Any]] = []
for target in sorted(
targets, key=lambda item: (int(item.split("@", 1)[0]), item)
):
number, expected_head = target.split("@", 1)
try:
row = run_gh_json(
[
"pr",
"view",
number,
"--json",
",".join(PR_LIST_FIELDS),
"--repo",
repository,
],
cwd=cwd,
)
except Exception as exc:
raise RuntimeError(f"target PR #{number} metadata read failed") from exc
if not isinstance(row, dict):
raise RuntimeError(f"target PR #{number} metadata read was not an object")
actual_head = str(row.get("headRefOid") or "").strip().lower()
if actual_head != expected_head:
raise ValueError(
f"target PR #{number} head changed: expected {expected_head}, "
f"remote is {actual_head or 'unavailable'}"
)
details_ok = attach_pr_review_details(
row,
repository=repository,
cwd=cwd,
**({"wait_for_ci": False} if not wait_for_ci else {}),
run_gh_json=run_gh_json,
)
if not details_ok:
raise RuntimeError(f"target PR #{number} detail read was incomplete")
pull_requests.append(row)

return {
"schema_version": "pr_review_source_scan_v0",
"complete": True,
"mode": "exact_targets",
"requested_exact_heads": sorted(targets),
"observed_exact_heads": sorted(targets),
"pull_requests": pull_requests,
"states": [
{
"state": "exact_targets",
"fetch_limit": len(targets),
"fetched_count": len(pull_requests),
"included_after_window": len(pull_requests),
"detail_read_failures": 0,
"source_saturated": False,
"source_read_valid": True,
}
],
}


PR_REVIEW_DETAIL_MAX_WORKERS = 8


Expand Down
66 changes: 58 additions & 8 deletions loopx/cli_commands/pr_review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
)
from ..capabilities.machine_configuration.store import read_machine_configuration
from ..capabilities.pr_review_queue.result_check import check_review_result
from ..capabilities.pr_review_queue.github_source import (
scan_github_pull_request_targets,
)
from ..file_lock import exclusive_file_lock
from ..pr_review import (
build_pr_review_packet,
Expand Down Expand Up @@ -148,6 +151,16 @@ def register_pr_review_command(
"--fixture",
help="Read public-safe PR metadata from a JSON fixture instead of live gh output.",
)
parser.add_argument(
"--target-exact-head",
action="append",
default=[],
metavar="NUMBER@HEAD_OID",
help=(
"Read only this exact PR head instead of scanning a lifecycle queue. "
"Repeatable for a small explicit review batch."
),
)
parser.add_argument(
"--fresh-audit-exact-head",
action="append",
Expand Down Expand Up @@ -208,6 +221,7 @@ def handle_pr_review_command(
return None
checkpoint_path: Path | None = None
resolved_review_priority = DEFAULT_REVIEW_PRIORITY
target_exact_heads = list(getattr(args, "target_exact_head", []) or [])
try:
machine_configuration = (read_machine_configuration(runtime_root, registry=build_builtin_machine_configuration_registry()) if runtime_root is not None else None)
goal = None
Expand All @@ -233,6 +247,7 @@ def handle_pr_review_command(
or args.repo
or args.since
or args.fresh_audit_exact_head
or target_exact_heads
or args.check_merge_readiness
):
raise ValueError(
Expand Down Expand Up @@ -263,6 +278,7 @@ def handle_pr_review_command(
or args.projected_exact_head
or args.since
or args.fresh_audit_exact_head
or target_exact_heads
):
raise ValueError(
"merge readiness cannot be combined with queue or observation options"
Expand Down Expand Up @@ -349,6 +365,15 @@ def handle_pr_review_command(
"--observation-state-file cannot be combined with "
"--previous-observation-json"
)
if target_exact_heads and args.autonomous_observation:
raise ValueError(
"--target-exact-head cannot be combined with --autonomous-observation"
)
if target_exact_heads and args.since:
raise ValueError(
"--target-exact-head already defines the review window and "
"cannot be combined with --since"
)
explicit_review_priority = getattr(args, "review_priority", None)
if explicit_review_priority is not None:
resolved_review_priority = normalize_review_priority(explicit_review_priority)
Expand All @@ -372,6 +397,19 @@ def handle_pr_review_command(
Path(args.fixture).expanduser()
)
repository = repository or repository_from_fixture
if target_exact_heads:
requested_targets = normalize_fresh_audit_exact_heads(
target_exact_heads
)
pull_requests = [
item
for item in pull_requests
if (
f"{item.get('number')}@"
f"{str(item.get('headRefOid') or '').lower()}"
in requested_targets
)
]
source = "fixture"
source_scan = None
else:
Expand All @@ -382,13 +420,23 @@ def handle_pr_review_command(
"authenticated GitHub reviewer identity is required for "
"autonomous author-owned scheduling"
)
source_scan = scan_github_pull_requests(
repo=repository,
limit=max(1, args.limit) + 1,
state_filter=normalize_pr_state_filter(args.state),
since=args.since,
**({"wait_for_ci": False} if not wait_for_ci else {}),
)
if target_exact_heads:
if not repository:
raise RuntimeError("GitHub repository could not be resolved")
source_scan = scan_github_pull_request_targets(
repository=repository,
exact_heads=target_exact_heads,
**({"wait_for_ci": False} if not wait_for_ci else {}),
)
source = "github_cli_exact_targets"
else:
source_scan = scan_github_pull_requests(
repo=repository,
limit=max(1, args.limit) + 1,
state_filter=normalize_pr_state_filter(args.state),
since=args.since,
**({"wait_for_ci": False} if not wait_for_ci else {}),
)
pull_requests = source_scan["pull_requests"]
if checkpoint_path is not None and previous_observation:
checkpoint_repository = str(
Expand All @@ -411,6 +459,7 @@ def handle_pr_review_command(
source_scan=source_scan,
reviewer_login=reviewer_login,
fresh_audit_exact_heads=args.fresh_audit_exact_head,
target_exact_heads=target_exact_heads,
review_priority=resolved_review_priority,
wait_for_ci=wait_for_ci,
)
Expand Down Expand Up @@ -465,13 +514,14 @@ def handle_pr_review_command(
"request": {
"schema_version": "loopx_pr_review_command_request_v0",
"command": "/loopx-pr-review",
"cli_command": "loopx pr-review [--repo owner/repo] [--state open|merged|all] [--review-priority other-developers-first|owner-first] [--since ISO]",
"cli_command": "loopx pr-review [--repo owner/repo] [--target-exact-head NUMBER@HEAD_OID] [--state open|merged|all] [--review-priority other-developers-first|owner-first] [--since ISO]",
"repository": args.repo,
"limit": max(1, args.limit),
"state_filter": normalize_pr_state_filter(args.state),
"since": args.since,
"review_priority": resolved_review_priority.value,
"fresh_audit_exact_heads": list(args.fresh_audit_exact_head),
"target_exact_heads": target_exact_heads,
"source": "fixture" if args.fixture else "github_cli",
"privacy_mode": "public_safe_github_metadata",
"dry_run": True,
Expand Down
Loading
Loading