fix(pr-review): preserve effective exact-head review history

loopx/capabilities/pr_review_queue/README.md

@@ -799,7 +799,21 @@ state transition, author-owned conclusions use `COMMENTED` plus one exact title:
 `Approval conclusion (author-owned PR; GitHub blocks formal self-approval)` or
 `Request changes conclusion (author-owned PR; GitHub blocks formal self-review)`.
 The compact result is versioned as `pull_request_review_conclusion_v0` and
-reports a typed verdict and invalid-reason codes.
+reports a typed verdict and invalid-reason codes. Queue selection and approval
+closeout share the typed owner for each reviewer's latest submitted opinion.
+A brief approval by another reviewer cannot hide a complete exact-head approval.
+A reviewer's newer formal opinion retires their own previous opinion, including
+when it is dismissed or lacks the required body. Current-head requests for
+changes remain visible even without that format, and ordinary comments or
+pending reviews cannot restore a retired approval. A changed head still requires
+fresh evidence. This corrects default queue/readiness classification; it neither
+relaxes body qualification nor grants dismissal or merge authority.
+
+队列与 closeout 复用同一 TypeScript 历史选择规则：另一评审者的简短批准不会
+遮住完整的精确 head 批准；同一评审者的新正式意见会替换自己的旧意见，撤回或
+格式不完整也不会复活旧批准。当前 head 的阻塞意见保留，普通评论及待提交评审
+不改变已有正式意见，head 变化仍需新证据。本修复改变默认队列和 readiness
+分类，不放宽正文校验，也不授予撤回评审或合并权限。
 
 After a published exact-head `APPROVE` is read back, the capability-owned
 `review_execution_contract.approval_closeout` requires effective-review

loopx/capabilities/pr_review_queue/approval_closeout.py

@@ -41,6 +41,18 @@ def plan_approval_closeout(request: dict[str, Any]) -> dict[str, Any]:
     return result
 
 
+def select_review_conclusion(request: dict[str, Any]) -> int:
+    """Transport qualified bodies to the shared typed history owner."""
+    try:
+        result = effect_runtime_result("capabilities.pr_review.conclusion.select", request)
+    except EffectRuntimeRejected as error:
+        raise ValueError(str(error)) from error
+    index = result.get("index") if isinstance(result, dict) else None
+    if type(index) is not int or not 0 <= index < len(request["reviews"]):
+        raise TypeError("typed review conclusion index mismatch")
+    return index
+
+
 def read_github_approval_closeout(*, repository: str, exact_head: str) -> dict[str, Any]:
     from .github_source import _fetch_complete_pr_files, run_gh_json
     from ...pr_review import (

loopx/control_plane/capabilities/pr_review_approval_closeout.ts

@@ -7,15 +7,60 @@ import {parseIsoTimestamp} from "../runtime_timestamp.ts";
 
 // GitHub owns these input states; closeout statuses are local to this read model.
 type ReviewState = "PENDING" | "COMMENTED" | "APPROVED" | "CHANGES_REQUESTED" | "DISMISSED";
-type Review = {id: number; login: string; state: ReviewState; head: string; time: number;
-  submittedAt: string | null; url: string};
+type Opinion = {id: number; login: string; state: ReviewState; head: string; time: number};
+type Review = Opinion & {submittedAt: string | null; url: string};
 const states = new Set<ReviewState>(["PENDING", "COMMENTED", "APPROVED", "CHANGES_REQUESTED", "DISMISSED"]);
 function fail(message: string): never { throw new EffectRuntimeRequestError(message); }
 function oid(value: unknown): string {
   const result = requireNonEmptyString(value, "review commit").toLowerCase();
   return /^(?:[a-f0-9]{40}|[a-f0-9]{64})$/.test(result) ? result : fail("review commit must be a full SHA");
 }
 
+/** A submitted formal opinion retires only that reviewer's prior opinion.
+ * Ordinary comments and pending reviews cannot retire an earned conclusion. */
+function effectiveOpinions<T extends Opinion>(reviews: readonly T[], author: string | null,
+  conclusions: ReadonlyMap<number, JsonObject>): T[] {
+  const latest = new Map<string, T>();
+  for (const row of [...reviews].sort((a, b) => a.time - b.time || a.id - b.id)) {
+    const conclusion = conclusions.get(row.id)!;
+    const authorFallback = row.state === "COMMENTED" && row.login.toLowerCase() === author
+      && conclusion.valid === true && ["APPROVE", "REQUEST_CHANGES"].includes(String(conclusion.verdict));
+    if (row.state !== "PENDING" && (row.state !== "COMMENTED" || authorFallback)) latest.set(row.login.toLowerCase(), row);
+  }
+  return [...latest.values()];
+}
+
+/** Bodies are qualified by the adapter; history precedence has one typed owner.
+ * Queue indices are local positions, never GitHub review identifiers. */
+export function selectPrReviewConclusion(value: unknown): JsonObject {
+  const request = requireJsonObject(value, "review conclusion selection");
+  if (!Array.isArray(request.reviews) || !request.reviews.length) return fail("review history must be nonempty");
+  const history = request.reviews;
+  const conclusions = new Map<number, JsonObject>();
+  const reviews: Opinion[] = history.map((value, index) => {
+    const row = requireJsonObject(value, "review opinion");
+    const state = row.state as ReviewState;
+    if (!states.has(state)) return fail("unknown GitHub review state");
+    if (typeof row.time !== "number" || !Number.isFinite(row.time)) return fail("review time must be finite");
+    if (typeof row.head !== "string" || typeof row.login !== "string") return fail("review identity must be strings");
+    // The adapter supplies newest-first rows; equal-time ties retain its order.
+    const id = history.length - index;
+    conclusions.set(id, requireJsonObject(row.conclusion, "qualified review conclusion"));
+    return {id, login: row.login.toLowerCase(), state, head: row.head.toLowerCase(), time: row.time};
+  });
+  const author = typeof request.author === "string" ? request.author.toLowerCase() : null;
+  const head = requireNonEmptyString(request.head, "current head").toLowerCase();
+  const effective = effectiveOpinions(reviews, author, conclusions)
+    .sort((a, b) => b.time - a.time || b.id - a.id);
+  // Unformatted dissent remains dissent. A different reviewer's later approval
+  // cannot erase it; complete closeout still reconciles blockers on older heads.
+  const selected = effective.find(row => row.head === head && (row.state === "CHANGES_REQUESTED"
+    || conclusions.get(row.id)!.valid === true && conclusions.get(row.id)!.verdict === "REQUEST_CHANGES"))
+    ?? effective.find(row => row.head === head && conclusions.get(row.id)!.valid === true)
+    ?? effective[0] ?? reviews[0];
+  return {index: history.length - selected.id};
+}
+
 export function planPrReviewApprovalCloseout(value: unknown): JsonObject {
   const request = requireJsonObject(value, "approval closeout");
   const exact = requireNonEmptyString(request.expected_exact_head, "expected_exact_head");
@@ -65,19 +110,13 @@ export function planPrReviewApprovalCloseout(value: unknown): JsonObject {
   if (conclusions.size !== reviews.length) return fail("review conclusions are incomplete");
   const author = typeof pr.author === "object" && pr.author !== null
     ? requireNonEmptyString(requireJsonObject(pr.author, "PR author").login, "PR author login").toLowerCase() : null;
-  const latest = new Map<string, Review>();
-  for (const row of reviews.sort((a, b) => a.time - b.time || a.id - b.id)) {
-    const conclusion = conclusions.get(row.id)!;
-    const authorFallback = row.state === "COMMENTED" && row.login.toLowerCase() === author
-      && conclusion.valid === true && ["APPROVE", "REQUEST_CHANGES"].includes(String(conclusion.verdict));
-    if (row.state !== "PENDING" && (row.state !== "COMMENTED" || authorFallback)) latest.set(row.login.toLowerCase(), row);
-  }
-  const approvalReview = [...latest.values()].filter(row => row.head === match[2]
+  const effective = effectiveOpinions(reviews, author, conclusions);
+  const approvalReview = effective.filter(row => row.head === match[2]
     && conclusions.get(row.id)!.valid === true && conclusions.get(row.id)!.verdict === "APPROVE"
     && (row.state === "APPROVED" || (row.state === "COMMENTED" && row.login.toLowerCase() === author)))
     .sort((a, b) => b.time - a.time || b.id - a.id)[0];
   if (!approvalReview) holds.push("exact_head_approval_missing");
-  const blockers = [...latest.values()].filter(row => row.state === "CHANGES_REQUESTED")
+  const blockers = effective.filter(row => row.state === "CHANGES_REQUESTED")
     .sort((a, b) => a.id - b.id).map(row => ({review_id: row.id, reviewer: row.login,
       review_head: row.head, review_url: row.url, on_approved_head: row.head === match[2]}));
   if (request.reviews_complete === true && !blockers.length && pr.reviewDecision === "CHANGES_REQUESTED") {

loopx/control_plane/effect_runtime_handlers.ts

@@ -462,6 +462,7 @@ export function createEffectRuntimeHandlers(
     ["capabilities.issue_fix.monitor_reconciliation.plan", lazyHandler(() => import("./capabilities/issue_fix_monitor_reconciliation.ts"), ({planIssueFixMonitorReconciliation}) => planIssueFixMonitorReconciliation)],
     ["capabilities.change_quality.validation_gate", lazyHandler(() => import("./capabilities/change_quality_validation.ts"), ({qualifyChangeQualityValidation}) => qualifyChangeQualityValidation)],
     ["capabilities.pr_review.approval_closeout.plan", lazyHandler(() => import("./capabilities/pr_review_approval_closeout.ts"), ({planPrReviewApprovalCloseout}) => planPrReviewApprovalCloseout)],
+    ["capabilities.pr_review.conclusion.select", lazyHandler(() => import("./capabilities/pr_review_approval_closeout.ts"), ({selectPrReviewConclusion}) => selectPrReviewConclusion)],
     ["capabilities.pr_review.configuration", lazyHandler(() => import("./capabilities/pr_review_order.ts"), ({prReviewConfiguration}) => prReviewConfiguration)],
     ["capabilities.pr_review.order", lazyHandler(() => import("./capabilities/pr_review_order.ts"), ({orderPrReviewQueue}) => orderPrReviewQueue)],
     ["coordination.local_authority_shadow.record", lazyHandler(() => import("./coordination/local_authority_shadow.ts"), ({recordLocalAuthorityShadow}) => recordLocalAuthorityShadow)],

loopx/pr_review.py

@@ -860,16 +860,18 @@ def evaluate(review: Mapping[str, Any]) -> dict[str, Any]:
             "invalid_reasons": reasons,
         }
 
-    latest_result = evaluate(reviews[0])
-    if (
-        str(reviews[0].get("state") or "").upper() == "COMMENTED"
-        and not latest_result["valid"]
-    ):
-        for earlier in reviews[1:]:
-            earlier_result = evaluate(earlier)
-            if earlier_result["valid"]:
-                return earlier_result
-    return latest_result
+    conclusions = [evaluate(review) for review in reviews]
+    if len(reviews) == 1:
+        return conclusions[0]
+    from .capabilities.pr_review_queue.approval_closeout import select_review_conclusion
+    index = select_review_conclusion({"head": head_oid, "author": pr_author,
+        "reviews": [{"state": conclusion["state"],
+            "login": str(_as_dict(review.get("author")).get("login") or ""),
+            "head": str(_as_dict(review.get("commit")).get("oid") or ""),
+            "time": _parse_updated_epoch(review.get("submittedAt")),
+            "conclusion": conclusion}
+            for review, conclusion in zip(reviews, conclusions)]})
+    return conclusions[index]
 
 
 def _review_sequence_entry(item: dict[str, Any], *, rank: int) -> dict[str, Any]:

tests/capabilities/test_pr_review_approval_closeout.py

@@ -63,6 +63,17 @@ def test_aggregate_history_conflict_is_not_a_clear_closeout():
     assert "aggregate_review_decision_conflict" in result["hold_reasons"]
 
 
+def test_brief_approval_by_another_reviewer_does_not_hide_qualified_approval():
+    data = request([])
+    brief = review(51, "APPROVED", login="reporter", head=HEAD)
+    data["reviews"].append(brief)
+    data["review_conclusions"].append(conclusion(brief) | {"valid": False})
+    data["pull_request"]["reviewDecision"] = "APPROVED"
+    result = plan_approval_closeout(data)
+    assert result["status"] == "clear"
+    assert result["approval_snapshot"]["review_id"] == 50
+
+
 def test_only_latest_blocker_per_reviewer_needs_reconciliation():
     result = plan_approval_closeout(request([
         review(1, "CHANGES_REQUESTED"), review(2, "CHANGES_REQUESTED", head=HEAD),

tests/test_pr_review_github_scan.py

@@ -1691,6 +1691,83 @@ def test_latest_review_and_author_owned_fallback_are_enforced(monkeypatch) -> No
     )
 
 
+def test_other_reviewers_brief_approval_preserves_full_exact_head_conclusion() -> None:
+    row = _queue_pr(32, author="community", ready_at="2026-08-18T07:00:00Z",
+                    updated_at="2026-08-18T11:00:00Z")
+    head = row["headRefOid"]
+    row["reviews"] = [
+        {"author": {"login": "maintainer"}, "body": _full_review_body(head),
+         "commit": {"oid": head}, "state": "APPROVED",
+         "submittedAt": "2026-08-18T09:00:00Z"},
+        {"author": {"login": "reporter"}, "body": "Native platform reproduction passes.",
+         "commit": {"oid": head}, "state": "APPROVED",
+         "submittedAt": "2026-08-18T10:00:00Z"},
+    ]
+    item = pr_review_module.build_pr_review_packet(pull_requests=[row],
+        repository="owner/repo", limit=10, source="fixture", reviewer_login="maintainer",
+        wait_for_ci=False)["pull_requests"][0]
+    assert item["review_conclusion"]["valid"] is True
+    assert item["review_conclusion"]["reviewer"] == "maintainer"
+    assert item["review_action_kind"] == "qualify_pull_request_merge_readiness"
+
+
+@pytest.mark.parametrize("superseding", ["DISMISSED", "CHANGES_REQUESTED", "APPROVED"])
+def test_same_reviewers_new_opinion_cannot_resurrect_approval(superseding) -> None:
+    head = "a" * 40
+    reviews = [
+        {"author": {"login": "maintainer"}, "body": _full_review_body(head),
+         "commit": {"oid": head}, "state": "APPROVED",
+         "submittedAt": "2026-08-18T09:00:00Z"},
+        {"author": {"login": "MAINTAINER"}, "body": "New formal opinion.",
+         "commit": {"oid": head}, "state": superseding,
+         "submittedAt": "2026-08-18T10:00:00Z"},
+        {"author": {"login": "observer"}, "body": "Supplemental note.",
+         "commit": {"oid": head}, "state": "COMMENTED",
+         "submittedAt": "2026-08-18T11:00:00Z"},
+    ]
+    result = pr_review_module._review_conclusion(
+        {"headRefOid": head, "author": {"login": "community"}, "reviews": reviews},
+        reviewer_login="maintainer")
+    assert result["valid"] is False
+    assert result["state"] == superseding
+
+
+def test_unformatted_current_head_blocker_survives_other_approval() -> None:
+    head = "a" * 40
+    reviews = [
+        {"author": {"login": "dissent"}, "body": "Please fix the reproduced failure.",
+         "commit": {"oid": head}, "state": "CHANGES_REQUESTED",
+         "submittedAt": "2026-08-18T09:00:00Z"},
+        {"author": {"login": "maintainer"}, "body": _full_review_body(head),
+         "commit": {"oid": head}, "state": "APPROVED",
+         "submittedAt": "2026-08-18T10:00:00Z"},
+    ]
+    result = pr_review_module._review_conclusion(
+        {"headRefOid": head, "author": {"login": "community"}, "reviews": reviews},
+        reviewer_login="maintainer")
+    assert result["valid"] is False
+    assert result["state"] == "CHANGES_REQUESTED"
+    assert result["reviewer"] == "dissent"
+
+
+@pytest.mark.parametrize("head", ["a" * 40, "b" * 40])
+def test_pending_review_cannot_retire_opinion_or_transfer_it_to_new_head(head) -> None:
+    approved_head = "a" * 40
+    result = pr_review_module._review_conclusion({"headRefOid": head,
+        "author": {"login": "community"}, "reviews": [
+            {"author": {"login": "maintainer"}, "body": _full_review_body(approved_head),
+             "commit": {"oid": approved_head}, "state": "APPROVED",
+             "submittedAt": "2026-08-18T09:00:00Z"},
+            {"author": {"login": "maintainer"}, "body": "Pending draft.",
+             "commit": {"oid": head}, "state": "PENDING",
+             "submittedAt": "2026-08-18T10:00:00Z"},
+        ]}, reviewer_login="maintainer")
+    assert result["valid"] is (head == approved_head)
+    assert result["state"] == "APPROVED"
+    if head != approved_head:
+        assert "review_not_bound_to_current_head" in result["invalid_reasons"]
+
+
 def test_actionable_sequence_excludes_valid_merged_exact_head(monkeypatch) -> None:
     monkeypatch.setattr(pr_review_module, "_now_iso", lambda: "2026-08-18T12:00:00Z")
     reviewed = _queue_pr(