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
130 changes: 129 additions & 1 deletion examples/pr-review-command-smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,134 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
"current_head_review_missing_or_invalid" in blocked["blocking_reasons"]
), blocked
assert "status_checks_failed" in blocked["blocking_reasons"], blocked

# Merge readiness follows the typed approval verdict, not GitHub's review
# state. The platform blocks self-approval, so an author-owned approval is
# stored as COMMENTED; a state-based rule counted these still-open heads as
# concluded and hid approved heads that had gone behind, conflicted, lost
# checks, or become blocked.
approval_head = "f" * 40

def approved_open_head(
number: int,
*,
merge_state: str,
review_state: str = "COMMENTED",
verdict: str = "APPROVE",
) -> dict[str, object]:
title = (
"Approval conclusion (author-owned PR; GitHub blocks formal self-approval)"
if verdict == "APPROVE"
else "Request changes conclusion (author-owned PR; GitHub blocks formal self-review)"
)
return {
"number": number,
"title": f"Approved open head {number}",
"url": f"https://github.com/owner/repo/pull/{number}",
"state": "OPEN",
"author": {"login": "maintainer"},
"headRefOid": approval_head,
"baseRefName": "main",
"isDraft": False,
"reviewDecision": "REVIEW_REQUIRED",
"mergeStateStatus": merge_state,
"files": [{"path": "src/runtime.py", "additions": 1, "deletions": 1}],
"reviews": [
{
"state": review_state,
"body": (
f"{title}\n\n"
"## 动机\n动机。\n\n## 改动思路\n思路。\n\n"
"## 具体改动\n改动。\n\n## 对主干的风险\n风险。\n\n"
"## 我的整体评价\n通过。\n\n"
f"English verdict: {verdict} at exact head {approval_head}."
),
"author": {"login": "maintainer"},
"commit": {"oid": approval_head},
"submittedAt": "2026-09-09T11:14:01Z",
}
],
"statusCheckRollup": [
{"name": "Sign-off", "status": "COMPLETED", "conclusion": "SUCCESS"},
{
"name": "merge-gate",
"status": "COMPLETED",
"conclusion": "SUCCESS" if merge_state == "CLEAN" else "FAILURE",
},
],
"review_thread_summary": {
"schema_version": "github_review_thread_summary_v0",
"complete": True,
"total_count": 0,
"unresolved_count": 0,
},
}

with tempfile.TemporaryDirectory() as temp_dir:
approval_fixture_path = Path(temp_dir) / "approved-open-heads.json"
approval_fixture = {
"repository": "owner/repo",
"reviewer_login": "maintainer",
"pull_requests": [
approved_open_head(4111, merge_state="BEHIND"),
approved_open_head(4112, merge_state="CLEAN"),
approved_open_head(
4113, merge_state="CLEAN", verdict="REQUEST_CHANGES"
),
],
}
approval_fixture_path.write_text(
json.dumps(approval_fixture), encoding="utf-8"
)
approval_packet = json.loads(
run_cli(
"--format",
"json",
"pr-review",
"--fixture",
str(approval_fixture_path),
"--state",
"open",
).stdout
)
approvals = {
item["number"]: item for item in approval_packet["pull_requests"]
}
for number in (4111, 4112):
assert approvals[number]["review_conclusion"]["verdict"] == "APPROVE", approvals
assert (
approvals[number]["review_action_kind"]
== "qualify_pull_request_merge_readiness"
), approvals[number]
assert approvals[4111]["merge_state"] == "BEHIND", approvals[4111]
assert approvals[4112]["merge_state"] == "CLEAN", approvals[4112]
assert approvals[4113]["review_conclusion"]["verdict"] == "REQUEST_CHANGES", approvals
assert approvals[4113]["review_action_kind"] is None, approvals[4113]
assert approval_packet["summary"]["review_attention_count"] == 2, (
approval_packet["summary"]
)
assert sorted(
item["number"] for item in approval_packet["review_sequence"]
) == [4111, 4112], approval_packet["review_sequence"]
for number, expected_blockers in (
(4111, ("merge_state_requires_update", "status_checks_failed")),
(4112, ()),
):
readiness = json.loads(
run_cli(
"--format",
"json",
"pr-review",
"--fixture",
str(approval_fixture_path),
"--check-merge-readiness",
f"{number}@{approval_head}",
check=not expected_blockers,
).stdout
)
for blocker in expected_blockers:
assert blocker in readiness["blocking_reasons"], readiness
assert readiness["ready"] is (not expected_blockers), readiness
assert sequence[0]["risk_hint_level"] == "medium", sequence[0]
assert sequence[0]["main_risk_level"] == "medium", sequence[0]
merged_sequence = next(item for item in sequence if item["number"] == 770)
Expand Down Expand Up @@ -767,7 +895,7 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
)
assert incomplete_observation["candidate"] is None, incomplete_observation

repository, fixture_prs = load_pr_fixture(FIXTURE)
repository, fixture_prs, _fixture_reviewer = load_pr_fixture(FIXTURE)
merged_fixture = next(item for item in fixture_prs if item.get("state") == "MERGED")
busy_window = []
for offset in range(105):
Expand Down
16 changes: 11 additions & 5 deletions loopx/capabilities/pr_review_queue/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,17 @@ The command packet keeps inventory and execution queues distinct.
window for compact conclusion readback. Top-level and group `review_sequence`
contain only rows whose `review_action_kind` is non-null. A merged exact head
without a valid conclusion receives `audit_merged_pull_request_exact_head`; a
merged or open exact head with a valid non-action conclusion remains
inventory-only and cannot become the recommended first PR. The summary's
attention counts are derived from this same actionable set. Inventory-only rows
set `review_plan` and `review_template` to null and `evidence_commands` to an
empty list so hosts cannot mistake readback metadata for execution authority.
merged or open exact head whose valid conclusion is not an approval remains
inventory-only and cannot become the recommended first PR. An open exact head
with a valid approval keeps owing `qualify_pull_request_merge_readiness`,
because merge readiness is decided by the typed verdict rather than by GitHub's
review state: the platform blocks self-approval, so an author-owned approval is
recorded as `COMMENTED` and a state-based rule would count that still-unmerged
head as concluded, even after it goes behind, conflicts, loses its checks or is
blocked. The summary's attention counts are derived from this same actionable
set. Inventory-only rows set `review_plan` and `review_template` to null and
`evidence_commands` to an empty list so hosts cannot mistake readback metadata
for execution authority.

It emits a
`pull_request_review_todo_preview_v0` bound to its exact head. The preview may
Expand Down
6 changes: 5 additions & 1 deletion loopx/capabilities/pr_review_queue/selection_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ def review_action_kind(item: Mapping[str, Any]) -> str | None:
conclusion = item.get("review_conclusion")
conclusion = conclusion if isinstance(conclusion, Mapping) else {}
if conclusion.get("valid") is True:
if state == "OPEN" and str(conclusion.get("state") or "").upper() == "APPROVED":
# Merge readiness is owed by the typed verdict, not by GitHub's review
# state: the platform blocks self-approval, so an author-owned approval
# is recorded as COMMENTED and would otherwise sit in the concluded
# lane forever, even after the head stops being mergeable.
if state == "OPEN" and str(conclusion.get("verdict") or "").upper() == "APPROVE":
return "qualify_pull_request_merge_readiness"
return None
if state == "MERGED":
Expand Down
4 changes: 2 additions & 2 deletions loopx/cli_commands/pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ def handle_pr_review_command(
repository = args.repo
reviewer_login = None
if args.fixture:
fixture_repository, pull_requests = load_pr_fixture(
fixture_repository, pull_requests, reviewer_login = load_pr_fixture(
Path(args.fixture).expanduser()
)
repository = repository or fixture_repository
Expand Down Expand Up @@ -368,7 +368,7 @@ def handle_pr_review_command(
source = "github_cli"
reviewer_login = None
if args.fixture:
repository_from_fixture, pull_requests = load_pr_fixture(
repository_from_fixture, pull_requests, reviewer_login = load_pr_fixture(
Path(args.fixture).expanduser()
)
repository = repository or repository_from_fixture
Expand Down
18 changes: 14 additions & 4 deletions loopx/pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,16 +338,26 @@ def append_state(state: str) -> None:
}


def load_pr_fixture(path: Path) -> tuple[str | None, list[dict[str, Any]]]:
def load_pr_fixture(
path: Path,
) -> tuple[str | None, list[dict[str, Any]], str | None]:
"""Load an offline PR window, including the reviewer identity it declares.

Author-owned conclusions depend on the reviewer identity: GitHub records an
author-owned approval as `COMMENTED`, so offline use must be able to name
the authenticated reviewer or it cannot represent that real case.
"""

payload = json.loads(path.read_text(encoding="utf-8"))
if isinstance(payload, list):
return None, [item for item in payload if isinstance(item, dict)]
return None, [item for item in payload if isinstance(item, dict)], None
if not isinstance(payload, dict):
return None, []
return None, [], None
items = payload.get("pull_requests") or payload.get("prs") or []
return (
str(payload.get("repository") or "") or None,
[item for item in _as_list(items) if isinstance(item, dict)],
str(payload.get("reviewer_login") or "") or None,
)


Expand Down Expand Up @@ -1302,7 +1312,7 @@ def _review_why_now(item: dict[str, Any]) -> str:
return "Draft PR; skim for early direction but do not treat as merge-ready."
conclusion = _as_dict(item.get("review_conclusion"))
if conclusion.get("valid") is True:
if str(conclusion.get("state") or "").upper() == "APPROVED":
if str(conclusion.get("verdict") or "").upper() == "APPROVE":
return "The current exact head has a complete approval; qualify merge readiness."
return "The current exact head already has a complete standalone conclusion."
if item.get("author_owned"):
Expand Down
71 changes: 71 additions & 0 deletions tests/capabilities/test_pr_review_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,37 @@
PullRequestReviewPriority,
build_pull_request_review_queue_observation,
build_scheduling_policy,
materialize_review_execution,
scheduling_tier,
)


def _concluded_item(
*,
number: int = 1,
state: str = "OPEN",
conclusion: dict[str, object] | None = None,
draft: bool = False,
decision: str = "REVIEW_REQUIRED",
) -> dict[str, object]:
return {
"number": number,
"state": state,
"head_oid": f"{number:040d}",
"is_draft": draft,
"review_decision": decision,
"review_conclusion": conclusion
if conclusion is not None
else {"valid": True, "state": "COMMENTED", "verdict": "APPROVE"},
}


def _action_kind(item: dict[str, object]) -> str | None:
return materialize_review_execution(
item, fresh_audit_exact_heads=set()
)["review_action_kind"]


def _pr(
number: int,
*,
Expand Down Expand Up @@ -418,6 +445,50 @@ def test_approved_transition_routes_to_merge_policy_without_granting_it() -> Non
assert approved["write_authority_granted"] is False


def test_open_head_merge_readiness_follows_typed_verdict() -> None:
"""An approval keeps owing the pre-merge gate while the PR is open.

GitHub blocks self-approval, so an author-owned approval is stored as a
COMMENTED review. Keying the queue on the formal review state counted such a
head as concluded and hid approved heads that had gone behind, conflicted,
lost checks, or become blocked.
"""

assert (
_action_kind(_concluded_item())
== "qualify_pull_request_merge_readiness"
)
assert (
_action_kind(
_concluded_item(
conclusion={"valid": True, "state": "APPROVED", "verdict": "APPROVE"}
)
)
== "qualify_pull_request_merge_readiness"
)

author_owned_request_changes = _concluded_item(
conclusion={
"valid": True,
"state": "COMMENTED",
"verdict": "REQUEST_CHANGES",
}
)
assert _action_kind(author_owned_request_changes) is None
assert _action_kind(_concluded_item(state="MERGED")) is None
assert _action_kind(_concluded_item(draft=True)) is None

invalid = _concluded_item(
conclusion={"valid": False, "state": None, "verdict": None}
)
assert _action_kind(invalid) == "review_pull_request_exact_head"
changes_requested = _concluded_item(
conclusion={"valid": False, "state": None, "verdict": None},
decision="CHANGES_REQUESTED",
)
assert _action_kind(changes_requested) == "rereview_pull_request_exact_head"


def test_review_backlog_keeps_active_cadence_until_all_handled() -> None:
first = _observe([_pr(1), _pr(2), _pr(3)])

Expand Down
16 changes: 14 additions & 2 deletions tests/test_pr_review_github_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,13 @@ def test_latest_review_and_author_owned_fallback_are_enforced(monkeypatch) -> No
reviewer_login="maintainer",
)["pull_requests"][0]
assert valid_fallback["review_conclusion"]["status"] == "valid"
assert valid_fallback["review_action_kind"] is None
# Merge readiness follows the typed verdict, not GitHub's review state: an
# author-owned approval is COMMENTED because the platform blocks
# self-approval, and it still owes the pre-merge gate while the PR is open.
assert (
valid_fallback["review_action_kind"]
== "qualify_pull_request_merge_readiness"
)

row["reviews"] = [
{
Expand Down Expand Up @@ -1281,7 +1287,13 @@ def test_latest_review_and_author_owned_fallback_are_enforced(monkeypatch) -> No
reviewer_login="maintainer",
)["pull_requests"][0]
assert ordinary_comment["review_conclusion"]["status"] == "valid"
assert ordinary_comment["review_action_kind"] is None
# The later COMMENTED note is not a conclusion, so the earlier valid
# author-owned approval still binds the current head and still owes the
# pre-merge gate.
assert (
ordinary_comment["review_action_kind"]
== "qualify_pull_request_merge_readiness"
)


def test_actionable_sequence_excludes_valid_merged_exact_head(monkeypatch) -> None:
Expand Down
Loading