From e2800b70c26ae367a506f1b6ea4c696e095e52ab Mon Sep 17 00:00:00 2001 From: ll7 <32880741+ll7@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:33:36 +0200 Subject: [PATCH] fix(dev): machine-recognize review-claim and author_decision states (#7508) --- CHANGELOG.md | 15 ++ scripts/dev/pr_loop_policy.py | 321 +++++++++++++++++++++++++-- tests/dev/test_pr_loop_policy.py | 359 +++++++++++++++++++++++++++++++ 3 files changed, 682 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0f71d7120..c10770169c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -422,6 +422,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 shared resource budget (API quota, runner starvation, tmpfs worktrees). Terminal states are now part of the output contract. Docs-only; no runtime behavior changed. +* **issue #7508 `pr_loop_policy` machine-recognizes the review-claim and + author-decision markers.** The policy classifier now parks a PR as + `active_writer` when a trusted (OWNER/MEMBER/COLLABORATOR) comment carries an + unexpired, unreleased `review-claim: @ until ` marker bound + to the live head, and as `author_decision` when the `decision-required` label + is paired with a live-head `### Decision packet` comment — both distinct from + the generic `blocked_preflight` state and both recommended `no_action` / + `stop`. `review-claim: released @ `, expiry (`now >= until`), and head + movement clear a claim. The marker helpers take an explicit `now` instant for + deterministic testing and the CLI defaults to the current UTC time. The + not-ready-sentinel half (merge-ready present while the body carries a + not-ready sentence) remains tracked in issue #7491 and is intentionally not + implemented here. Workflow/tooling only; no benchmark, planner, or evidence + claim. + * **Issue #7086 representative-run rule de-duplicated into one shared utility.** The rule that decides which seed stands in for a whole campaign cell — majority-verdict pool, weaker-label tie-break, median primary order parameter, lower seed on an exact tie — was implemented twice, diff --git a/scripts/dev/pr_loop_policy.py b/scripts/dev/pr_loop_policy.py index b4a0cc6d59..399fc26ad5 100644 --- a/scripts/dev/pr_loop_policy.py +++ b/scripts/dev/pr_loop_policy.py @@ -6,6 +6,20 @@ refresh_snapshot, mark_ready_candidate, await_gate_verdict, reconcile_pr_metadata, await_review_threads, or no_action. +Two parking states are machine-recognized from trusted PR comments (issue #7508, +the machine half of the goal-pr-review skill contract from PR #7500): + +- ``active_writer`` — a trusted comment carries an unexpired, unreleased + ``review-claim: @ until `` marker covering the live head, so + another review lane owns the write window and the loop must not race it. +- ``author_decision`` — the ``decision-required`` label is paired with a + ``### Decision packet`` comment at the live head, so the PR is parked on an + author-reserved ruling rather than an infra/preflight blocker. + +Both park the PR (``no_action`` / ``stop``). The not-ready-sentinel half +(merge-ready present while the body carries a not-ready sentence) is tracked +separately in issue #7491 and intentionally not implemented here. + Every PolicyDecision also emits a high-level ``flow_decision`` — one of exactly ``stop``, ``continue``, ``reroute``, or ``escalate`` — for machine consumption. @@ -32,6 +46,7 @@ import re import sys from dataclasses import asdict, dataclass +from datetime import UTC, datetime from pathlib import Path from typing import Any @@ -76,6 +91,8 @@ "unknown_review_threads", "pending_gate_verdict", "pending_pr_metadata", + "active_writer", + "author_decision", "ready_to_merge", "no_action", } @@ -105,6 +122,24 @@ ) EXACT_HEAD_RE = _EXACT_HEAD_RE +# Issue #7508 markers from the goal-pr-review skill (PR #7500). An advisory +# ``review-claim`` comment announces a lane's mutable-write window; the same +# comment thread is released with ``review-claim: released @ ``. The +# ``until`` timestamp is the ISO-8601 UTC expiry (default claim window 90 min). +_REVIEW_CLAIM_RE = re.compile( + r"review-claim\s*:\s*(?P[^\s@]+)\s*@\s*(?P[0-9a-fA-F]{7,40})\b" + r"\s+until\s+(?P\S+)", + re.IGNORECASE, +) +REVIEW_CLAIM_RE = _REVIEW_CLAIM_RE +_REVIEW_CLAIM_RELEASED_RE = re.compile( + r"review-claim\s*:\s*released\s*@\s*(?P[0-9a-fA-F]{7,40})\b", + re.IGNORECASE, +) +REVIEW_CLAIM_RELEASED_RE = _REVIEW_CLAIM_RELEASED_RE +_DECISION_PACKET_HEADING_RE = re.compile(r"^###\s+Decision\s+packet\b", re.IGNORECASE) +DECISION_PACKET_HEADING_RE = _DECISION_PACKET_HEADING_RE + @dataclass(frozen=True, slots=True) class ShaCarrier: @@ -120,6 +155,20 @@ class ShaCarrier: full: bool +@dataclass(frozen=True, slots=True) +class ReviewClaim: + """One parsed ``review-claim`` marker from a trusted comment (issue #7508). + + ``lane`` is the claiming lane id, ``sha`` the claimed head SHA as written + (lowercased), and ``expires_at`` the parsed ``until`` timestamp (UTC) or + ``None`` when the timestamp is missing or unparseable. + """ + + lane: str + sha: str + expires_at: datetime | None + + def extract_sha_carriers(text: str) -> list[ShaCarrier]: """Extract exact-head SHA carriers from a PR metadata text blob. @@ -156,6 +205,166 @@ def invalid_sha_carriers(carriers: list[ShaCarrier], live_head_sha: str) -> list return [carrier for carrier in carriers if not carrier.full or carrier.sha != live_head] +def _parse_review_claim_marker(text: str) -> ReviewClaim | None: + """Parse one ``review-claim: @ until `` marker, or None. + + Returns ``None`` for non-string/empty blobs and for blobs whose timestamp + cannot be parsed as an ISO-8601 UTC datetime (fail closed: an unparseable + claim is treated as expired rather than parking the PR forever). + """ + if not isinstance(text, str) or not text: + return None + match = _REVIEW_CLAIM_RE.search(text) + if not match: + return None + raw_until = match.group("until") + try: + expires_at = datetime.fromisoformat(raw_until.replace("Z", "+00:00")) + except ValueError: + return None + if expires_at.tzinfo is None: + expires_at = expires_at.replace(tzinfo=UTC) + return ReviewClaim( + lane=match.group("lane").lower(), + sha=match.group("sha").lower(), + expires_at=expires_at.astimezone(UTC), + ) + + +def _review_claim_released_shas(text: str) -> set[str]: + """Return lowercased SHAs released by ``review-claim: released @ `` markers.""" + if not isinstance(text, str) or not text: + return set() + return {match.group("sha").lower() for match in _REVIEW_CLAIM_RELEASED_RE.finditer(text)} + + +def _has_decision_packet_heading(text: str) -> bool: + """Return True when a comment body carries a ``### Decision packet`` heading.""" + if not isinstance(text, str) or not text: + return False + return any(_DECISION_PACKET_HEADING_RE.match(line) for line in text.splitlines()) + + +def _trusted_marker_comments(pr: dict[str, Any]) -> list[dict[str, Any]]: + """Return comment/review entries that may carry an issue #7508 marker. + + Only repository-trusted authors (OWNER/MEMBER/COLLABORATOR) may park a PR. + Mirrors ``_snapshot_body_texts`` and the snapshot producer's + ``_extract_trailers_from_bodies`` trust rule. + """ + entries: list[dict[str, Any]] = [] + for items in (pr.get("comments"), pr.get("reviews")): + if not isinstance(items, list): + continue + for entry in items: + if not isinstance(entry, dict): + continue + association = str( + entry.get("authorAssociation") or entry.get("author_association") or "" + ).upper() + if association not in _TRUSTED_GATE_VERDICT_ASSOCIATIONS: + continue + body = entry.get("body") + if isinstance(body, str) and body: + entries.append(entry) + return entries + + +def _normalize_now(now: datetime | None) -> datetime | None: + """Return a UTC-aware evaluation instant, or None for a non-datetime value.""" + if now is None: + return datetime.now(UTC) + if not isinstance(now, datetime): + return None + if now.tzinfo is None: + now = now.replace(tzinfo=UTC) + return now.astimezone(UTC) + + +def _claim_parks(claim: ReviewClaim, *, head_sha: str, released: set[str], now: datetime) -> bool: + """Return True when one parsed claim still parks the PR.""" + if not _sha_matches_head(claim.sha, head_sha): + return False + if claim.sha in released: + return False + if claim.expires_at is None or now >= claim.expires_at: + return False + return True + + +def active_review_claim( + pr: dict[str, Any], head_sha: str, now: datetime | None +) -> ReviewClaim | None: + """Return the review-claim parking the PR as an active writer, or None. + + A PR is ``active_writer`` when a trusted comment carries a ``review-claim: + @ until `` marker that is (issue #7508): + + - unexpired: ``now < until`` (unparseable timestamps fail closed as + expired; equality is expiry, matching the ``now >= until`` rule), + - unreleased: no trusted ``review-claim: released @ `` marker names + the same head SHA after the claim (releases clear regardless of head + movement), + - head-bound: the marker's SHA matches the live head SHA. + + Lane identity is not distinguishable in the compact snapshot, so the + documented simplification applies: any unexpired unreleased trusted marker + parks the PR (see the module docstring limitation note). + + ``now`` is an explicit evaluation instant for deterministic tests; ``None`` + falls back to ``datetime.now(UTC)`` for live queue evaluation. A naive + datetime is interpreted as UTC. + """ + if not isinstance(pr, dict) or not head_sha: + return None + now = _normalize_now(now) + if now is None: + return None + + released: set[str] = set() + claims: list[ReviewClaim] = [] + for entry in _trusted_marker_comments(pr): + body = str(entry.get("body", "")) + released.update(_review_claim_released_shas(body)) + claim = _parse_review_claim_marker(body) + if claim is not None: + claims.append(claim) + + for claim in claims: + if _claim_parks(claim, head_sha=head_sha, released=released, now=now): + return claim + return None + + +def has_author_decision_packet(pr: dict[str, Any], head_sha: str) -> bool: + """Return True when a trusted comment at the live head posts a Decision packet. + + ``author_decision`` requires the ``decision-required`` label AND a trusted + comment whose body contains a ``### Decision packet`` heading. The comment + must be bound to the live head: either it names the head SHA in a + ``review-claim``/``exact-head`` carrier, or the PR snapshot carries only + conversation comments (no historical review bodies), in which case the + comment set is assumed to live at the current head. + """ + if not isinstance(pr, dict) or not head_sha: + return False + carriers: list[ShaCarrier] = [] + comments_with_packet = 0 + has_historical_review_bodies = False + for entry in _trusted_marker_comments(pr): + body = str(entry.get("body", "")) + carriers.extend(extract_sha_carriers(body)) + if _has_decision_packet_heading(body): + comments_with_packet += 1 + if "reviews" in pr and entry in pr.get("reviews", []): + has_historical_review_bodies = True + if comments_with_packet == 0: + return False + if has_historical_review_bodies and any(carrier.full for carrier in carriers): + return any(_sha_matches_head(carrier.sha, head_sha) for carrier in carriers) + return True + + @dataclass(frozen=True, slots=True) class PolicyDecision: """A deterministic policy recommendation for one PR.""" @@ -592,14 +801,73 @@ def _merge_ready_state( return "ready_to_merge" +def _active_writer_or_author_decision( + pr: dict[str, Any], + *, + label_names: list[str], + head_sha: str, + now: datetime | None, +) -> str | None: + """Return the issue #7508 marker parking state for a PR, or None. + + Checked before the generic preflight pipeline so a PR parks on a live + review-claim or an author decision even when other preflight logic would + otherwise fire. + """ + if active_review_claim(pr, head_sha, now) is not None: + return "active_writer" + if "decision-required" in label_names and has_author_decision_packet(pr, head_sha): + return "author_decision" + return None + + +def _preflight_state_before_pending( + pr: dict[str, Any], + *, + overall: str, + head_sha: str, +) -> str | None: + """Return the fail-closed state that precedes a pending-CI wait, or None. + + Mirrors the original classifier ordering: base freshness, stale head, and + blocked preflight evidence all take precedence over the generic pending-CI + wait. + """ + base_state = _base_state_after_policy(pr, head_sha) + if base_state is not None: + return base_state + head_state = _head_preflight_state(pr) + if head_state is not None: + return head_state + preflight_state = _blocked_preflight_state(pr) + if preflight_state is not None: + return preflight_state + if overall == "pending": + return "pending_ci" + return None + + def classify_pr_state( pr: dict[str, Any], *, compact_artifacts: dict[str, Any] | None = None, + now: datetime | None = None, ) -> str: """Classify a single PR into a machine-checkable loop state. Pure function: no side effects, no GitHub calls. + + Marker precedence (issue #7508): ``active_writer`` and ``author_decision`` + are checked before the generic ``blocked_preflight`` classification, so a PR + can be parked for a live review-claim or an author decision even when other + preflight logic would otherwise fire. Draft/closed/error PRs still return + ``no_action`` first, exactly as before. The not-ready-sentinel half + (merge-ready present while the body carries a not-ready sentence) is tracked + separately in issue #7491 and intentionally not implemented here. + + ``now`` is an explicit UTC evaluation instant for review-claim expiry so the + classifier stays deterministic; a naive datetime is interpreted as UTC and + ``None`` falls back to ``datetime.now(UTC)``. """ if not isinstance(pr, dict): return "no_action" @@ -615,19 +883,19 @@ def classify_pr_state( artifacts = pr.get("artifacts") if is_draft: return "no_action" + marker_state = _active_writer_or_author_decision( + pr, + label_names=label_names, + head_sha=head_sha, + now=now, + ) + if marker_state is not None: + return marker_state if overall == "failure": return "failed_ci" - base_state = _base_state_after_policy(pr, head_sha) - if base_state is not None: - return base_state - head_state = _head_preflight_state(pr) - if head_state is not None: - return head_state - preflight_state = _blocked_preflight_state(pr) + preflight_state = _preflight_state_before_pending(pr, overall=overall, head_sha=head_sha) if preflight_state is not None: return preflight_state - if overall == "pending": - return "pending_ci" artifact_state = _artifact_state(artifacts, compact_artifacts=compact_artifacts) if artifact_state is not None: return artifact_state @@ -684,7 +952,7 @@ def _compute_flow_decision( - ready_to_merge -> continue - unknown_review_threads -> continue (wait for a thread-capable snapshot) - failed_ci, failed_validation, missing_artifacts, stale_worktree, stale_merge_base -> reroute - - blocked_preflight -> stop + - active_writer, author_decision, blocked_preflight -> stop (parked) - no_action -> stop """ if budget_exhausted: @@ -708,7 +976,7 @@ def _compute_flow_decision( return "stop" -def recommend_action( # noqa: C901 +def recommend_action( # noqa: C901, PLR0912 state: str, *, pr_number: int, @@ -863,6 +1131,30 @@ def recommend_action( # noqa: C901 reason="snapshot preflight is blocked; inspect the blocking reason before retry", actions_remaining=remaining, ) + case "active_writer": + return PolicyDecision( + pr=pr_number, + action="no_action", + state=state, + flow_decision=flow_decision, + reason=( + "another lane holds an unexpired review-claim on this head; " + "park until the claim is released or expires" + ), + actions_remaining=remaining, + ) + case "author_decision": + return PolicyDecision( + pr=pr_number, + action="no_action", + state=state, + flow_decision=flow_decision, + reason=( + "decision-required label with a live-head Decision packet; " + "park for the author's ruling" + ), + actions_remaining=remaining, + ) case _: return PolicyDecision( pr=pr_number, @@ -889,10 +1181,13 @@ def evaluate_queue( expected_head_shas: dict[int, str] | None = None, artifact_presence: dict[int, bool] | None = None, compact_artifacts: dict[int, dict[str, Any]] | None = None, + now: datetime | None = None, ) -> dict[str, Any]: """Evaluate a PR queue and emit per-PR decisions under a loop budget. - Pure function: reads snapshot dicts, never calls external APIs. + ``now`` is forwarded to ``classify_pr_state`` for deterministic review-claim + expiry evaluation; ``None`` falls back to ``datetime.now(UTC)``. Pure + function: reads snapshot dicts, never calls external APIs. """ decisions: list[dict[str, Any]] = [] actions_used = 0 @@ -909,7 +1204,7 @@ def evaluate_queue( compact = compact_by_pr.get(num) if compact is not None: enriched["compact_artifacts"] = compact - state = classify_pr_state(enriched, compact_artifacts=compact) + state = classify_pr_state(enriched, compact_artifacts=compact, now=now) review = _review_state(enriched) labels = enriched.get("labels") or [] label_names = [str(label) for label in labels] if isinstance(labels, list) else [] diff --git a/tests/dev/test_pr_loop_policy.py b/tests/dev/test_pr_loop_policy.py index 9b1a542aa2..d198e2a3bc 100644 --- a/tests/dev/test_pr_loop_policy.py +++ b/tests/dev/test_pr_loop_policy.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from datetime import UTC, datetime from io import StringIO from pathlib import Path from unittest.mock import patch @@ -14,14 +15,20 @@ VALID_ACTIONS, VALID_STATES, PolicyDecision, + ReviewClaim, ShaCarrier, _accepted_gate_verdict_shas, + _has_decision_packet_heading, + _parse_review_claim_marker, + _review_claim_released_shas, _review_state, _sha_matches_head, + active_review_claim, classify_pr_state, evaluate_queue, extract_sha_carriers, format_text, + has_author_decision_packet, has_current_accepted_gate_verdict, has_current_pr_metadata_verdict, invalid_sha_carriers, @@ -81,6 +88,75 @@ def _pr( return result +# Fixed evaluation instant for the issue #7508 marker tests, so claim expiry is +# deterministic without monkeypatching global time. +NOW_UTC = datetime(2026, 8, 18, 12, 0, 0, tzinfo=UTC) +FUTURE_UTC = "2026-08-18T13:30:00Z" +PAST_UTC = "2026-08-18T11:00:00Z" + + +def _with_review_claim( + result: dict[str, object], + *, + lane: str = "lane-a", + sha: str | None = None, + until: str = FUTURE_UTC, + association: str = "COLLABORATOR", + author: str = "lane-a-bot", +) -> dict[str, object]: + """Attach a trusted ``review-claim: @ until `` comment.""" + claimed = sha if sha is not None else str(result.get("head_sha", "")) + result["comments"] = [ + { + "author": author, + "authorAssociation": association, + "createdAt": "2026-08-18T12:00:00Z", + "body": f"review-claim: {lane} @ {claimed} until {until}", + } + ] + return result + + +def _with_released_review_claim( + result: dict[str, object], *, sha: str | None = None, association: str = "COLLABORATOR" +) -> dict[str, object]: + """Attach a released ``review-claim: released @ `` comment.""" + claimed = sha if sha is not None else str(result.get("head_sha", "")) + result["comments"] = [ + { + "author": "lane-a-bot", + "authorAssociation": association, + "createdAt": "2026-08-18T12:00:00Z", + "body": f"review-claim: released @ {claimed}", + } + ] + return result + + +def _with_decision_packet( + result: dict[str, object], + *, + association: str = "OWNER", + author: str = "reviewer-bot", + body: str = "### Decision packet\nShould we merge this lane?", + in_review: bool = False, +) -> dict[str, object]: + """Attach a trusted ``### Decision packet`` comment at the PR head.""" + entry: dict[str, object] = { + "author": author, + "authorAssociation": association, + "createdAt": "2026-08-18T12:00:00Z", + "body": body, + } + if in_review: + entry["state"] = "COMMENTED" + entry["submittedAt"] = "2026-08-18T12:00:00Z" + result["reviews"] = [entry] + else: + result["comments"] = [entry] + return result + + def _with_merge_base( result: dict[str, object], *, base_sha: str = "", main_sha: str = "" ) -> dict[str, object]: @@ -678,6 +754,289 @@ def test_classify_error_status_is_no_action() -> None: assert classify_pr_state(pr) == "no_action" +# --------------------------------------------------------------------------- +# Issue #7508: review-claim -> active_writer +# --------------------------------------------------------------------------- + + +def test_active_writer_parks_on_unexpired_trusted_review_claim() -> None: + """An unexpired review-claim from another lane parks the PR as active_writer.""" + pr = _with_review_claim( + _pr(7500, overall="success", labels=["merge-ready"], head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=FUTURE_UTC, + ) + state = classify_pr_state(pr, now=NOW_UTC) + assert state == "active_writer" + decision = recommend_action(state, pr_number=7500, actions_remaining=3) + assert decision.action == "no_action" + assert decision.flow_decision == "stop" + assert "review-claim" in decision.reason + + +def test_active_writer_untrusted_marker_does_not_park() -> None: + """A contributor-authored review-claim must not park the PR.""" + pr = _with_review_claim( + _pr(7501, overall="success", head_sha=FULL_SHA), + association="CONTRIBUTOR", + sha=FULL_SHA, + ) + assert classify_pr_state(pr, now=NOW_UTC) != "active_writer" + + +def test_released_review_claim_clears_active_writer() -> None: + """``review-claim: released @ `` clears the claim regardless of head.""" + pr = _pr(7502, overall="success", head_sha=FULL_SHA) + _with_review_claim(pr, lane="lane-a", sha=FULL_SHA, until=FUTURE_UTC) + _with_released_review_claim(pr, sha=FULL_SHA) + assert classify_pr_state(pr, now=NOW_UTC) != "active_writer" + + +def test_expired_review_claim_does_not_park() -> None: + """A claim whose ``until`` is in the past does not park the PR.""" + pr = _with_review_claim( + _pr(7503, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=PAST_UTC, + ) + assert classify_pr_state(pr, now=NOW_UTC) != "active_writer" + + +def test_review_claim_expiry_at_equality_is_expired() -> None: + """``now == until`` counts as expiry (now >= until clears the claim).""" + exact = "2026-08-18T12:00:00Z" + pr = _with_review_claim( + _pr(7504, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=exact, + ) + assert classify_pr_state(pr, now=NOW_UTC) != "active_writer" + + +def test_review_claim_head_mismatch_does_not_park() -> None: + """A claim bound to a different head SHA does not park the current head.""" + other_sha = "deadbeef00000000000000000000000000000001" + pr = _with_review_claim( + _pr(7505, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=other_sha, + until=FUTURE_UTC, + ) + assert classify_pr_state(pr, now=NOW_UTC) != "active_writer" + + +def test_unparseable_review_claim_expiry_fails_closed() -> None: + """A claim with an unparseable timestamp is treated as expired, not parked.""" + pr = _with_review_claim( + _pr(7506, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until="not-a-timestamp", + ) + assert classify_pr_state(pr, now=NOW_UTC) != "active_writer" + + +def test_review_claim_in_review_body_parks() -> None: + """A review body can carry the claim just like a conversation comment.""" + pr = _pr(7507, overall="success", head_sha=FULL_SHA) + pr["reviews"] = [ + { + "state": "COMMENTED", + "author": "lane-a-bot", + "authorAssociation": "COLLABORATOR", + "submittedAt": "2026-08-18T12:00:00Z", + "body": f"review-claim: lane-a @ {FULL_SHA} until {FUTURE_UTC}", + } + ] + assert classify_pr_state(pr, now=NOW_UTC) == "active_writer" + + +def test_active_review_claim_helper_reports_lane_and_expiry() -> None: + """The pure helper returns the claim with lane and parsed expiry.""" + pr = _with_review_claim( + _pr(7508, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=FUTURE_UTC, + ) + claim = active_review_claim(pr, FULL_SHA, NOW_UTC) + assert isinstance(claim, ReviewClaim) + assert claim.lane == "lane-a" + assert claim.sha == FULL_SHA + assert claim.expires_at is not None + assert claim.expires_at == datetime(2026, 8, 18, 13, 30, 0, tzinfo=UTC) + + +def test_parse_review_claim_marker_roundtrip() -> None: + """Marker parsing normalizes the lane, SHA, and UTC expiry.""" + claim = _parse_review_claim_marker( + f"review-claim: Lane-B @ {FULL_SHA.upper()} until 2026-08-18T13:30:00+00:00" + ) + assert claim is not None + assert claim.lane == "lane-b" + assert claim.sha == FULL_SHA + assert claim.expires_at == datetime(2026, 8, 18, 13, 30, 0, tzinfo=UTC) + assert _parse_review_claim_marker("no marker here") is None + assert _parse_review_claim_marker("") is None + + +def test_review_claim_released_shas_collects_markers() -> None: + """Released-marker extraction returns lowercased SHAs.""" + body = f"review-claim: released @ {FULL_SHA.upper()}\nother text" + assert _review_claim_released_shas(body) == {FULL_SHA} + assert _review_claim_released_shas("no release") == set() + + +# --------------------------------------------------------------------------- +# Issue #7508: decision-required + Decision packet -> author_decision +# --------------------------------------------------------------------------- + + +def test_author_decision_parks_on_live_head_decision_packet() -> None: + """decision-required + a live-head Decision packet parks as author_decision.""" + pr = _with_decision_packet( + _pr(7510, overall="success", labels=["decision-required"], head_sha=FULL_SHA) + ) + state = classify_pr_state(pr, now=NOW_UTC) + assert state == "author_decision" + assert state != "blocked_preflight" + decision = recommend_action(state, pr_number=7510, actions_remaining=3) + assert decision.action == "no_action" + assert decision.flow_decision == "stop" + assert "Decision packet" in decision.reason + + +def test_author_decision_requires_the_label() -> None: + """A Decision packet without the decision-required label is not parked.""" + pr = _with_decision_packet(_pr(7511, overall="success", head_sha=FULL_SHA)) + assert classify_pr_state(pr, now=NOW_UTC) != "author_decision" + + +def test_author_decision_requires_a_decision_packet() -> None: + """The decision-required label alone must not park the PR.""" + pr = _pr(7512, overall="success", labels=["decision-required"], head_sha=FULL_SHA) + assert has_author_decision_packet(pr, FULL_SHA) is False + assert classify_pr_state(pr, now=NOW_UTC) != "author_decision" + + +def test_author_decision_review_body_at_live_head() -> None: + """A Decision packet inside a review body bound to the live head parks.""" + pr = _with_decision_packet( + _pr(7513, overall="success", labels=["decision-required"], head_sha=FULL_SHA), + in_review=True, + body=f"### Decision packet\nShould we split this PR?\ngate-verdict: accepted @ {FULL_SHA}", + ) + assert classify_pr_state(pr, now=NOW_UTC) == "author_decision" + + +def test_author_decision_review_body_at_stale_head_does_not_park() -> None: + """A packet review naming a stale head SHA is not at the live head.""" + other_sha = "deadbeef00000000000000000000000000000001" + pr = _with_decision_packet( + _pr(7514, overall="success", labels=["decision-required"], head_sha=FULL_SHA), + in_review=True, + body=f"### Decision packet\nSplit?\ngate-verdict: accepted @ {other_sha}", + ) + assert classify_pr_state(pr, now=NOW_UTC) != "author_decision" + + +def test_author_decision_untrusted_packet_does_not_park() -> None: + """A contributor-authored Decision packet must not park the PR.""" + pr = _with_decision_packet( + _pr(7515, overall="success", labels=["decision-required"], head_sha=FULL_SHA), + association="CONTRIBUTOR", + ) + assert classify_pr_state(pr, now=NOW_UTC) != "author_decision" + + +def test_has_decision_packet_heading_matches() -> None: + """The heading matcher recognizes the canonical packet heading only.""" + assert _has_decision_packet_heading("### Decision packet\nQuestion?") is True + assert _has_decision_packet_heading("## Decision packet") is False + assert _has_decision_packet_heading("no heading") is False + assert _has_decision_packet_heading("") is False + + +# --------------------------------------------------------------------------- +# Issue #7508: precedence and regressions +# --------------------------------------------------------------------------- + + +def test_active_writer_precedes_blocked_preflight() -> None: + """A live review-claim parks the PR even when preflight is blocked.""" + pr = _with_review_claim( + _pr(7520, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=FUTURE_UTC, + ) + pr["preflight"] = {"status": "blocked", "reasons": ["missing_authority"]} + assert classify_pr_state(pr, now=NOW_UTC) == "active_writer" + + +def test_author_decision_precedes_blocked_preflight() -> None: + """An author decision parks the PR even when preflight is blocked.""" + pr = _with_decision_packet( + _pr(7521, overall="success", labels=["decision-required"], head_sha=FULL_SHA) + ) + pr["preflight"] = {"status": "blocked", "reasons": ["missing_authority"]} + assert classify_pr_state(pr, now=NOW_UTC) == "author_decision" + + +def test_active_writer_precedes_pending_ci_and_ready_merge() -> None: + """A live review-claim parks even a green merge-ready head.""" + pr = _with_review_claim( + _pr( + 7522, + overall="success", + labels=["merge-ready"], + head_sha=FULL_SHA, + gate_verdict=FULL_SHA, + ), + lane="lane-a", + sha=FULL_SHA, + until=FUTURE_UTC, + ) + assert classify_pr_state(pr, now=NOW_UTC) == "active_writer" + + +def test_draft_with_live_review_claim_stays_no_action() -> None: + """Draft PRs remain no_action before marker classification.""" + pr = _with_review_claim( + _pr(7523, overall="pending", draft=True, head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=FUTURE_UTC, + ) + assert classify_pr_state(pr, now=NOW_UTC) == "no_action" + + +def test_marker_states_are_in_valid_states_contract() -> None: + """The new parking states are part of the exported policy contract.""" + assert "active_writer" in VALID_STATES + assert "author_decision" in VALID_STATES + + +def test_evaluate_queue_parks_active_writer() -> None: + """Queue evaluation parks an active_writer PR with no_action.""" + prs = [ + _with_review_claim( + _pr(7524, overall="success", head_sha=FULL_SHA), + lane="lane-a", + sha=FULL_SHA, + until=FUTURE_UTC, + ) + ] + result = evaluate_queue(prs, max_actions=3, now=NOW_UTC) + decision = result["decisions"][0] + assert decision["state"] == "active_writer" + assert decision["action"] == "no_action" + assert decision["flow_decision"] == "stop" + + def test_classify_non_dict_input() -> None: """Non-dict input should classify as no_action.""" assert classify_pr_state("not a dict") == "no_action" # type: ignore[arg-type]