feat: canonical validation wording, baseline proof, and closure gate (Closes #529) #598
@@ -0,0 +1,63 @@
|
|||||||
|
"""Controller-closure baseline-proof gate (#529, criterion 6).
|
||||||
|
|
||||||
|
A controller (or any session) may close a tracking issue with a closure
|
||||||
|
report that summarizes validation. When that report calls a non-zero
|
||||||
|
test-suite exit an "expected pre-existing failure" (or baseline / known
|
||||||
|
failure) it must carry pre-merge proof; otherwise the closure buries an
|
||||||
|
unproven regression as an accepted baseline and weakens controller
|
||||||
|
confidence in the durable state.
|
||||||
|
|
||||||
|
This module fails closed on exactly that case by reusing the pre-merge
|
||||||
|
baseline proof verifier (#533). A closure report is allowed when it is
|
||||||
|
empty (no validation claim), a clean pass, or a baseline failure proven on
|
||||||
|
the PR pre-merge base commit (or a documented known-failure record that
|
||||||
|
predates the PR).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
from premerge_baseline_proof import CLEAN_PASS, assess_premerge_baseline_proof
|
||||||
|
|
||||||
|
|
||||||
|
def assess_controller_closure_baseline_proof(
|
||||||
|
closure_report: str | None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Assess whether an issue-closure report may proceed.
|
||||||
|
|
||||||
|
Returns a dict with ``block``, ``proven``, ``label``, ``reasons``,
|
||||||
|
``skipped`` and ``safe_next_action``. Only blocks when the closure
|
||||||
|
report claims a non-zero validation exit is an expected
|
||||||
|
pre-existing/baseline failure without valid pre-merge proof.
|
||||||
|
"""
|
||||||
|
text = (closure_report or "").strip()
|
||||||
|
if not text:
|
||||||
|
return {
|
||||||
|
"block": False,
|
||||||
|
"proven": True,
|
||||||
|
"label": CLEAN_PASS,
|
||||||
|
"reasons": [],
|
||||||
|
"skipped": True,
|
||||||
|
"safe_next_action": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
result = assess_premerge_baseline_proof(text)
|
||||||
|
block = bool(result.get("block"))
|
||||||
|
return {
|
||||||
|
"block": block,
|
||||||
|
"proven": not block,
|
||||||
|
"label": result.get("label"),
|
||||||
|
"reasons": list(result.get("reasons") or []),
|
||||||
|
"skipped": bool(result.get("skipped", False)),
|
||||||
|
"safe_next_action": (
|
||||||
|
result.get("safe_next_action")
|
||||||
|
or (
|
||||||
|
"provide pre-merge baseline proof (base commit, tested commit, "
|
||||||
|
"command, exit status, failure signature) before closing on an "
|
||||||
|
"expected pre-existing failure"
|
||||||
|
)
|
||||||
|
)
|
||||||
|
if block
|
||||||
|
else "",
|
||||||
|
}
|
||||||
@@ -40,6 +40,7 @@ FINAL_REPORT_TASK_KINDS = frozenset({
|
|||||||
"issue_filing",
|
"issue_filing",
|
||||||
"issue_selection",
|
"issue_selection",
|
||||||
"inventory",
|
"inventory",
|
||||||
|
"controller_close",
|
||||||
})
|
})
|
||||||
|
|
||||||
_TASK_KIND_ALIASES = {
|
_TASK_KIND_ALIASES = {
|
||||||
@@ -51,6 +52,9 @@ _TASK_KIND_ALIASES = {
|
|||||||
"reconcile_already_landed": "reconcile_already_landed",
|
"reconcile_already_landed": "reconcile_already_landed",
|
||||||
"work-issue": "work_issue",
|
"work-issue": "work_issue",
|
||||||
"create_issue": "issue_filing",
|
"create_issue": "issue_filing",
|
||||||
|
"close_issue": "controller_close",
|
||||||
|
"close-issue": "controller_close",
|
||||||
|
"controller-close": "controller_close",
|
||||||
}
|
}
|
||||||
|
|
||||||
_HANDOFF_ROLE_BY_TASK = {
|
_HANDOFF_ROLE_BY_TASK = {
|
||||||
@@ -62,6 +66,7 @@ _HANDOFF_ROLE_BY_TASK = {
|
|||||||
"issue_filing": "issue_filing",
|
"issue_filing": "issue_filing",
|
||||||
"issue_selection": None,
|
"issue_selection": None,
|
||||||
"inventory": "inventory",
|
"inventory": "inventory",
|
||||||
|
"controller_close": None,
|
||||||
}
|
}
|
||||||
|
|
||||||
_LEGACY_WORKSPACE_MUTATIONS_RE = re.compile(
|
_LEGACY_WORKSPACE_MUTATIONS_RE = re.compile(
|
||||||
@@ -850,6 +855,27 @@ def _rule_reviewer_premerge_baseline_proof(report_text: str) -> list[dict[str, s
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _rule_reviewer_post_merge_validation(report_text: str) -> list[dict[str, str]]:
|
||||||
|
"""Block active approval on an already-merged/closed PR, and post-merge moot
|
||||||
|
validation claimed without merged-state + merge-commit proof (#529)."""
|
||||||
|
from post_merge_validation import assess_post_merge_validation
|
||||||
|
|
||||||
|
result = assess_post_merge_validation(report_text)
|
||||||
|
if not result.get("block"):
|
||||||
|
return []
|
||||||
|
return _findings_from_reasons(
|
||||||
|
"reviewer.post_merge_validation",
|
||||||
|
result.get("reasons") or [],
|
||||||
|
field="Validation status",
|
||||||
|
severity="block",
|
||||||
|
safe_next_action=result.get("safe_next_action")
|
||||||
|
or (
|
||||||
|
"record post-merge moot validation instead of an active approval; "
|
||||||
|
"cite PR state merged/closed and the merge commit SHA"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _rule_reviewer_validation_status_vocabulary(
|
def _rule_reviewer_validation_status_vocabulary(
|
||||||
report_text: str,
|
report_text: str,
|
||||||
*,
|
*,
|
||||||
@@ -1514,6 +1540,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
|||||||
_rule_reviewer_linked_issue,
|
_rule_reviewer_linked_issue,
|
||||||
_rule_reviewer_baseline_on_failure,
|
_rule_reviewer_baseline_on_failure,
|
||||||
_rule_reviewer_premerge_baseline_proof,
|
_rule_reviewer_premerge_baseline_proof,
|
||||||
|
_rule_reviewer_post_merge_validation,
|
||||||
_rule_reviewer_validation_status_vocabulary,
|
_rule_reviewer_validation_status_vocabulary,
|
||||||
_rule_reviewer_main_checkout_baseline,
|
_rule_reviewer_main_checkout_baseline,
|
||||||
_rule_reviewer_main_checkout_path,
|
_rule_reviewer_main_checkout_path,
|
||||||
@@ -1606,6 +1633,13 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
|||||||
*_SHARED_CANONICAL_COMMENT_RULES,
|
*_SHARED_CANONICAL_COMMENT_RULES,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
*_SHARED_ISSUE_LOCK_RULES,
|
||||||
],
|
],
|
||||||
|
# Controller issue closure (#529): a closure report must not bury an
|
||||||
|
# unproven non-zero suite exit as an "expected pre-existing failure".
|
||||||
|
# Kept intentionally narrow so a closure pre-check does not demand the
|
||||||
|
# full reviewer/author handoff schema.
|
||||||
|
"controller_close": [
|
||||||
|
_rule_reviewer_premerge_baseline_proof,
|
||||||
|
],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -6070,6 +6070,7 @@ def gitea_close_issue(
|
|||||||
host: str | None = None,
|
host: str | None = None,
|
||||||
org: str | None = None,
|
org: str | None = None,
|
||||||
repo: str | None = None,
|
repo: str | None = None,
|
||||||
|
closure_report: str | None = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Close an issue by setting its state to 'closed'.
|
"""Close an issue by setting its state to 'closed'.
|
||||||
|
|
||||||
@@ -6079,6 +6080,9 @@ def gitea_close_issue(
|
|||||||
host: Override the Gitea host.
|
host: Override the Gitea host.
|
||||||
org: Override the owner/organization.
|
org: Override the owner/organization.
|
||||||
repo: Override the repository name.
|
repo: Override the repository name.
|
||||||
|
closure_report: Optional validation/closure summary. When it claims a
|
||||||
|
non-zero test-suite exit is an "expected pre-existing"/baseline
|
||||||
|
failure without pre-merge proof, the close fails closed (#529).
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
dict with 'success' boolean and 'message'.
|
dict with 'success' boolean and 'message'.
|
||||||
@@ -6088,6 +6092,25 @@ def gitea_close_issue(
|
|||||||
if blocked:
|
if blocked:
|
||||||
return blocked
|
return blocked
|
||||||
verify_preflight_purity(remote, task="close_issue")
|
verify_preflight_purity(remote, task="close_issue")
|
||||||
|
|
||||||
|
# #529: block closure that buries an unproven non-zero suite exit as an
|
||||||
|
# "expected pre-existing failure". Skipped when no closure report is given.
|
||||||
|
from controller_closure_baseline_proof import (
|
||||||
|
assess_controller_closure_baseline_proof,
|
||||||
|
)
|
||||||
|
closure_gate = assess_controller_closure_baseline_proof(closure_report)
|
||||||
|
if closure_gate["block"]:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"blocked": True,
|
||||||
|
"message": (
|
||||||
|
f"Issue #{issue_number} close blocked (#529): closure report "
|
||||||
|
"claims an expected pre-existing/baseline failure without "
|
||||||
|
"pre-merge proof."
|
||||||
|
),
|
||||||
|
"reasons": closure_gate["reasons"],
|
||||||
|
"safe_next_action": closure_gate["safe_next_action"],
|
||||||
|
}
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
auth = _auth(h)
|
auth = _auth(h)
|
||||||
url = f"{repo_api_url(h, o, r)}/issues/{issue_number}"
|
url = f"{repo_api_url(h, o, r)}/issues/{issue_number}"
|
||||||
|
|||||||
@@ -42,12 +42,38 @@ STATUS_LABEL_SPECS: tuple[LabelSpec, ...] = (
|
|||||||
LabelSpec("status:wontfix", "000000", "Issue will not be fixed"),
|
LabelSpec("status:wontfix", "000000", "Issue will not be fixed"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Durable validation-outcome labels (#529): the four canonical distinctions a
|
||||||
|
# reviewer report can carry. These are orthogonal to the single active status:*
|
||||||
|
# label, so they use their own prefix and are not subject to the one-status
|
||||||
|
# invariant.
|
||||||
|
VALIDATION_LABEL_SPECS: tuple[LabelSpec, ...] = (
|
||||||
|
LabelSpec("validation:clean-pass", "0e8a16", "Validation was a clean pass"),
|
||||||
|
LabelSpec(
|
||||||
|
"validation:baseline-accepted",
|
||||||
|
"fbca04",
|
||||||
|
"Validation passed with a baseline-proven unrelated failure",
|
||||||
|
),
|
||||||
|
LabelSpec(
|
||||||
|
"validation:blocked",
|
||||||
|
"b60205",
|
||||||
|
"Validation blocked by an unresolved failure",
|
||||||
|
),
|
||||||
|
LabelSpec(
|
||||||
|
"validation:post-merge-moot",
|
||||||
|
"5319e7",
|
||||||
|
"Validation is post-merge moot (PR already merged/closed before review)",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
CANONICAL_LABEL_SPECS: tuple[LabelSpec, ...] = (
|
CANONICAL_LABEL_SPECS: tuple[LabelSpec, ...] = (
|
||||||
TYPE_LABEL_SPECS + STATUS_LABEL_SPECS
|
TYPE_LABEL_SPECS + STATUS_LABEL_SPECS + VALIDATION_LABEL_SPECS
|
||||||
)
|
)
|
||||||
|
|
||||||
TYPE_LABELS: frozenset[str] = frozenset(spec.name for spec in TYPE_LABEL_SPECS)
|
TYPE_LABELS: frozenset[str] = frozenset(spec.name for spec in TYPE_LABEL_SPECS)
|
||||||
STATUS_LABELS: frozenset[str] = frozenset(spec.name for spec in STATUS_LABEL_SPECS)
|
STATUS_LABELS: frozenset[str] = frozenset(spec.name for spec in STATUS_LABEL_SPECS)
|
||||||
|
VALIDATION_LABELS: frozenset[str] = frozenset(
|
||||||
|
spec.name for spec in VALIDATION_LABEL_SPECS
|
||||||
|
)
|
||||||
CANONICAL_LABELS: frozenset[str] = frozenset(
|
CANONICAL_LABELS: frozenset[str] = frozenset(
|
||||||
spec.name for spec in CANONICAL_LABEL_SPECS
|
spec.name for spec in CANONICAL_LABEL_SPECS
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -0,0 +1,222 @@
|
|||||||
|
"""Canonical post-merge (moot) validation outcome and wording gate (#529).
|
||||||
|
|
||||||
|
When a PR is already merged or closed *before* a reviewer submits their
|
||||||
|
verdict, an ordinary "approve" is misleading: the review never gated the
|
||||||
|
merge, so a normal active approval overstates controller confidence. The
|
||||||
|
sanctioned outcome for that case is a **post-merge moot validation** — the
|
||||||
|
reviewer records what validation found, but classifies it as moot rather than
|
||||||
|
an active approval.
|
||||||
|
|
||||||
|
This module defines the four canonical validation distinctions #529 requires
|
||||||
|
and enforces the post-merge case:
|
||||||
|
|
||||||
|
- ``CLEAN_PASS_OUTCOME`` — clean validation pass on an open, reviewable PR.
|
||||||
|
- ``BASELINE_ACCEPTED_OUTCOME`` — validation pass with a baseline-proven
|
||||||
|
unrelated failure (proof handled by :mod:`premerge_baseline_proof`).
|
||||||
|
- ``BLOCKED_OUTCOME`` — validation blocked by an unresolved failure.
|
||||||
|
- ``POST_MERGE_MOOT_OUTCOME`` — the PR was already merged/closed before review
|
||||||
|
submission; validation is recorded as post-merge moot, not an active
|
||||||
|
approval.
|
||||||
|
|
||||||
|
The gate blocks two mistakes:
|
||||||
|
|
||||||
|
1. Claiming an *active approval* on a PR that was already merged/closed before
|
||||||
|
review submission (must be recorded as post-merge moot validation instead).
|
||||||
|
2. Claiming post-merge moot validation without proof the PR is actually
|
||||||
|
merged/closed (a merged-state field plus a merge commit SHA).
|
||||||
|
|
||||||
|
Each outcome maps to a durable ``validation:*`` process-state label so PRs and
|
||||||
|
issues carry the distinction (#529 acceptance criterion).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
CLEAN_PASS_OUTCOME = "clean validation pass"
|
||||||
|
BASELINE_ACCEPTED_OUTCOME = "validation pass with baseline-proven unrelated failure"
|
||||||
|
BLOCKED_OUTCOME = "validation blocked by unresolved failure"
|
||||||
|
POST_MERGE_MOOT_OUTCOME = "post-merge moot validation"
|
||||||
|
|
||||||
|
# Canonical validation status label a reviewer writes in the report body for the
|
||||||
|
# post-merge case; kept in sync with validation_status_vocabulary.
|
||||||
|
POST_MERGE_MOOT_STATUS = "post-merge moot validation"
|
||||||
|
|
||||||
|
# Durable process-state labels (registered in issue_workflow_labels).
|
||||||
|
LABEL_CLEAN_PASS = "validation:clean-pass"
|
||||||
|
LABEL_BASELINE_ACCEPTED = "validation:baseline-accepted"
|
||||||
|
LABEL_BLOCKED = "validation:blocked"
|
||||||
|
LABEL_POST_MERGE_MOOT = "validation:post-merge-moot"
|
||||||
|
|
||||||
|
_OUTCOME_TO_LABEL: dict[str, str] = {
|
||||||
|
CLEAN_PASS_OUTCOME: LABEL_CLEAN_PASS,
|
||||||
|
BASELINE_ACCEPTED_OUTCOME: LABEL_BASELINE_ACCEPTED,
|
||||||
|
BLOCKED_OUTCOME: LABEL_BLOCKED,
|
||||||
|
POST_MERGE_MOOT_OUTCOME: LABEL_POST_MERGE_MOOT,
|
||||||
|
}
|
||||||
|
|
||||||
|
# The PR was already merged/closed before the review was submitted.
|
||||||
|
_MERGED_BEFORE_REVIEW_RE = re.compile(
|
||||||
|
r"(?:already[- ]merged"
|
||||||
|
r"|merged\s+before\s+review"
|
||||||
|
r"|closed\s+before\s+review"
|
||||||
|
r"|pr\s+(?:is|was)\s+(?:already\s+)?(?:merged|closed)"
|
||||||
|
r"|pr\s+state\s*[:=]\s*(?:merged|closed)"
|
||||||
|
r"|merged\s*/\s*closed)",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
# An active approval verdict is being claimed.
|
||||||
|
_ACTIVE_APPROVAL_RE = re.compile(
|
||||||
|
r"(?:review\s+decision\s*[:=]\s*approve"
|
||||||
|
r"|submitted\s+['\"]?approve['\"]?"
|
||||||
|
r"|active\s+approval"
|
||||||
|
r"|approving\s+the\s+pr"
|
||||||
|
r"|posting\s+an?\s+approval)",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
# The sanctioned post-merge moot validation wording.
|
||||||
|
_POST_MERGE_MOOT_RE = re.compile(
|
||||||
|
r"post[- ]merge\s+(?:moot\s+)?validation"
|
||||||
|
r"|post[- ]merge\s+moot"
|
||||||
|
r"|moot\s+validation",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
# Proof the PR is genuinely merged/closed.
|
||||||
|
_MERGED_STATE_PROOF_RE = re.compile(
|
||||||
|
r"(?:pr\s+state\s*[:=]\s*(?:merged|closed)"
|
||||||
|
r"|merged_at\s*[:=]\s*\S"
|
||||||
|
r"|closed_at\s*[:=]\s*\S"
|
||||||
|
r"|state\s*[:=]\s*(?:merged|closed))",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
_MERGE_COMMIT_SHA_RE = re.compile(
|
||||||
|
r"merge\s+commit(?:\s+sha)?\s*[:=]\s*[0-9a-f]{7,40}",
|
||||||
|
re.IGNORECASE,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
POST_MERGE_VALIDATION_TEMPLATE = (
|
||||||
|
"Validation status: post-merge moot validation\n"
|
||||||
|
"PR state: merged\n"
|
||||||
|
"Merge commit sha: <40-hex merge commit>\n"
|
||||||
|
"Validation finding: <what the validation run showed, for the record>\n"
|
||||||
|
"Note: PR merged/closed before review submission; recorded as post-merge "
|
||||||
|
"moot validation, not an active approval."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def process_state_label(outcome: str) -> str | None:
|
||||||
|
"""Return the durable ``validation:*`` label for a canonical outcome."""
|
||||||
|
return _OUTCOME_TO_LABEL.get(outcome)
|
||||||
|
|
||||||
|
|
||||||
|
def assess_post_merge_validation(
|
||||||
|
report_text: str,
|
||||||
|
*,
|
||||||
|
pr_merged_or_closed: bool | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Assess post-merge moot validation wording and proof (#529).
|
||||||
|
|
||||||
|
Args:
|
||||||
|
report_text: The reviewer final report text.
|
||||||
|
pr_merged_or_closed: Optional structured signal that the PR is already
|
||||||
|
merged/closed. When ``True`` it forces the merged-before-review
|
||||||
|
path even if the report text omits the phrasing; proof fields are
|
||||||
|
still required in the report body for durability.
|
||||||
|
|
||||||
|
Returns a dict with ``proven``, ``block``, ``outcome``,
|
||||||
|
``process_state_label``, ``reasons``, ``skipped`` and ``safe_next_action``.
|
||||||
|
Only blocks when the report either claims an active approval on a
|
||||||
|
merged/closed PR, or claims post-merge moot validation without proof.
|
||||||
|
"""
|
||||||
|
text = report_text or ""
|
||||||
|
|
||||||
|
merged_signal = bool(_MERGED_BEFORE_REVIEW_RE.search(text)) or bool(
|
||||||
|
pr_merged_or_closed
|
||||||
|
)
|
||||||
|
active_approval = bool(_ACTIVE_APPROVAL_RE.search(text))
|
||||||
|
moot_wording = bool(_POST_MERGE_MOOT_RE.search(text))
|
||||||
|
|
||||||
|
# Nothing about merged-state or moot validation — not applicable here.
|
||||||
|
if not merged_signal and not moot_wording:
|
||||||
|
return {
|
||||||
|
"proven": True,
|
||||||
|
"block": False,
|
||||||
|
"outcome": None,
|
||||||
|
"process_state_label": None,
|
||||||
|
"reasons": [],
|
||||||
|
"skipped": True,
|
||||||
|
"safe_next_action": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
# An active approval on a PR already merged/closed before review, without
|
||||||
|
# the sanctioned moot wording, overstates confidence.
|
||||||
|
if merged_signal and active_approval and not moot_wording:
|
||||||
|
return {
|
||||||
|
"proven": False,
|
||||||
|
"block": True,
|
||||||
|
"outcome": POST_MERGE_MOOT_OUTCOME,
|
||||||
|
"process_state_label": LABEL_POST_MERGE_MOOT,
|
||||||
|
"reasons": [
|
||||||
|
"PR was already merged/closed before review submission; an "
|
||||||
|
"active approval overstates confidence — record 'post-merge "
|
||||||
|
"moot validation' instead of a normal approval"
|
||||||
|
],
|
||||||
|
"skipped": False,
|
||||||
|
"safe_next_action": (
|
||||||
|
"classify the outcome as post-merge moot validation (not an "
|
||||||
|
"active approval); cite PR state merged/closed and the merge "
|
||||||
|
"commit SHA. Template:\n" + POST_MERGE_VALIDATION_TEMPLATE
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
# Post-merge moot validation claimed — require merged-state + commit proof.
|
||||||
|
if moot_wording:
|
||||||
|
reasons: list[str] = []
|
||||||
|
if not _MERGED_STATE_PROOF_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"post-merge moot validation claimed without merged/closed state "
|
||||||
|
"proof (state: merged/closed, or merged_at/closed_at)"
|
||||||
|
)
|
||||||
|
if not _MERGE_COMMIT_SHA_RE.search(text):
|
||||||
|
reasons.append(
|
||||||
|
"post-merge moot validation claimed without a merge commit SHA"
|
||||||
|
)
|
||||||
|
if reasons:
|
||||||
|
return {
|
||||||
|
"proven": False,
|
||||||
|
"block": True,
|
||||||
|
"outcome": POST_MERGE_MOOT_OUTCOME,
|
||||||
|
"process_state_label": LABEL_POST_MERGE_MOOT,
|
||||||
|
"reasons": reasons,
|
||||||
|
"skipped": False,
|
||||||
|
"safe_next_action": (
|
||||||
|
"prove the PR is merged/closed: cite PR state and the merge "
|
||||||
|
"commit SHA. Template:\n" + POST_MERGE_VALIDATION_TEMPLATE
|
||||||
|
),
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
"proven": True,
|
||||||
|
"block": False,
|
||||||
|
"outcome": POST_MERGE_MOOT_OUTCOME,
|
||||||
|
"process_state_label": LABEL_POST_MERGE_MOOT,
|
||||||
|
"reasons": [],
|
||||||
|
"skipped": False,
|
||||||
|
"safe_next_action": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
# Merged signal present but no approval claim and no moot wording yet —
|
||||||
|
# guide toward the moot outcome without blocking.
|
||||||
|
return {
|
||||||
|
"proven": True,
|
||||||
|
"block": False,
|
||||||
|
"outcome": POST_MERGE_MOOT_OUTCOME,
|
||||||
|
"process_state_label": LABEL_POST_MERGE_MOOT,
|
||||||
|
"reasons": [],
|
||||||
|
"skipped": False,
|
||||||
|
"safe_next_action": (
|
||||||
|
"PR appears merged/closed; if submitting a verdict, record "
|
||||||
|
"post-merge moot validation rather than an active approval"
|
||||||
|
),
|
||||||
|
}
|
||||||
@@ -0,0 +1,113 @@
|
|||||||
|
"""Controller-closure baseline-proof gate tests (#529, criterion 6)."""
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
from controller_closure_baseline_proof import (
|
||||||
|
assess_controller_closure_baseline_proof,
|
||||||
|
)
|
||||||
|
from premerge_baseline_proof import (
|
||||||
|
CLEAN_PASS,
|
||||||
|
PREMERGE_BASELINE_PROVEN_FAILURE,
|
||||||
|
UNRESOLVED_REGRESSION_RISK,
|
||||||
|
)
|
||||||
|
from final_report_validator import assess_final_report_validator
|
||||||
|
|
||||||
|
# Complete pre-merge proof fields for a baseline claim (see #533).
|
||||||
|
_PROOF_FIELDS = (
|
||||||
|
"Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||||
|
"Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||||
|
"Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
|
||||||
|
"Exit status: 1\n"
|
||||||
|
"Failure signature: AssertionError: None is not true\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestControllerClosureBaselineProof(unittest.TestCase):
|
||||||
|
def test_empty_report_skipped_and_allowed(self):
|
||||||
|
result = assess_controller_closure_baseline_proof("")
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["skipped"])
|
||||||
|
self.assertEqual(result["label"], CLEAN_PASS)
|
||||||
|
|
||||||
|
def test_none_report_allowed(self):
|
||||||
|
result = assess_controller_closure_baseline_proof(None)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["skipped"])
|
||||||
|
|
||||||
|
def test_clean_pass_closure_allowed(self):
|
||||||
|
result = assess_controller_closure_baseline_proof(
|
||||||
|
"Closing #529. Validation: full suite passed, exit status 0. Clean pass."
|
||||||
|
)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertEqual(result["label"], CLEAN_PASS)
|
||||||
|
|
||||||
|
def test_expected_preexisting_failure_without_proof_blocked(self):
|
||||||
|
report = (
|
||||||
|
"Closing #529. Full suite: 1 failed. This is an expected "
|
||||||
|
"pre-existing failure, safe to close."
|
||||||
|
)
|
||||||
|
result = assess_controller_closure_baseline_proof(report)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertFalse(result["proven"])
|
||||||
|
self.assertEqual(result["label"], UNRESOLVED_REGRESSION_RISK)
|
||||||
|
self.assertTrue(result["reasons"])
|
||||||
|
self.assertTrue(result["safe_next_action"])
|
||||||
|
|
||||||
|
def test_current_master_reproduction_only_blocked(self):
|
||||||
|
report = (
|
||||||
|
"Closing #529. Full suite: 1 failed. Pre-existing baseline failure — "
|
||||||
|
"reproduced on current master after merge.\n"
|
||||||
|
"Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
|
||||||
|
"Exit status: 1\n"
|
||||||
|
"Failure signature: AssertionError: None is not true\n"
|
||||||
|
)
|
||||||
|
result = assess_controller_closure_baseline_proof(report)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
def test_proven_baseline_closure_allowed(self):
|
||||||
|
report = (
|
||||||
|
"Closing #529. Full suite: 1 failed, an expected pre-existing "
|
||||||
|
"baseline failure proven on the PR pre-merge base commit.\n"
|
||||||
|
+ _PROOF_FIELDS
|
||||||
|
)
|
||||||
|
result = assess_controller_closure_baseline_proof(report)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertEqual(result["label"], PREMERGE_BASELINE_PROVEN_FAILURE)
|
||||||
|
|
||||||
|
|
||||||
|
class TestControllerCloseTaskKind(unittest.TestCase):
|
||||||
|
"""The composable validator must cover the controller_close task kind."""
|
||||||
|
|
||||||
|
def test_controller_close_blocks_unproven_baseline(self):
|
||||||
|
report = (
|
||||||
|
"Full suite: 1 failed. Expected pre-existing failure, closing the "
|
||||||
|
"tracking issue."
|
||||||
|
)
|
||||||
|
result = assess_final_report_validator(report, "controller_close")
|
||||||
|
self.assertTrue(result["blocked"])
|
||||||
|
self.assertTrue(
|
||||||
|
any(
|
||||||
|
"premerge_baseline_proof" in f["rule_id"]
|
||||||
|
for f in result["findings"]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_controller_close_alias_close_issue(self):
|
||||||
|
report = (
|
||||||
|
"Full suite: 1 failed. Expected pre-existing failure, closing the "
|
||||||
|
"tracking issue."
|
||||||
|
)
|
||||||
|
result = assess_final_report_validator(report, "close_issue")
|
||||||
|
self.assertTrue(result["blocked"])
|
||||||
|
|
||||||
|
def test_controller_close_allows_proven_baseline(self):
|
||||||
|
report = (
|
||||||
|
"Full suite: 1 failed, expected pre-existing baseline failure proven "
|
||||||
|
"on the PR pre-merge base commit.\n" + _PROOF_FIELDS
|
||||||
|
)
|
||||||
|
result = assess_final_report_validator(report, "controller_close")
|
||||||
|
self.assertFalse(result["blocked"])
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -2672,6 +2672,56 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
|||||||
self.assertTrue(res["success"])
|
self.assertTrue(res["success"])
|
||||||
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
||||||
|
|
||||||
|
def test_close_issue_blocks_unproven_expected_preexisting_failure(self):
|
||||||
|
# #529: a closure report calling a non-zero suite exit an "expected
|
||||||
|
# pre-existing failure" without pre-merge proof must fail closed and
|
||||||
|
# never PATCH the issue state.
|
||||||
|
patched = []
|
||||||
|
|
||||||
|
def api_side_effect(method, url, auth, payload=None):
|
||||||
|
if method == "PATCH":
|
||||||
|
patched.append(url)
|
||||||
|
return {}
|
||||||
|
self.mock_api.side_effect = api_side_effect
|
||||||
|
|
||||||
|
res = gitea_close_issue(
|
||||||
|
issue_number=1,
|
||||||
|
closure_report=(
|
||||||
|
"Full suite: 1 failed. Expected pre-existing failure, safe to close."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
self.assertFalse(res["success"])
|
||||||
|
self.assertTrue(res.get("blocked"))
|
||||||
|
self.assertTrue(res.get("reasons"))
|
||||||
|
self.assertFalse(
|
||||||
|
patched, "must not PATCH issue state when the closure gate blocks"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_close_issue_allows_proven_baseline_closure(self):
|
||||||
|
def api_side_effect(method, url, auth, payload=None):
|
||||||
|
if method == "PATCH" and "issues/1" in url:
|
||||||
|
return {"state": "closed"}
|
||||||
|
if method == "GET" and "labels" in url and "issues" not in url:
|
||||||
|
return [{"name": "bug", "id": 2}]
|
||||||
|
if method == "GET" and "issues/1" in url:
|
||||||
|
return {"labels": [{"name": "bug"}]}
|
||||||
|
return {}
|
||||||
|
self.mock_api.side_effect = api_side_effect
|
||||||
|
|
||||||
|
res = gitea_close_issue(
|
||||||
|
issue_number=1,
|
||||||
|
closure_report=(
|
||||||
|
"Full suite: 1 failed, expected pre-existing baseline failure "
|
||||||
|
"proven on the PR pre-merge base commit.\n"
|
||||||
|
"Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||||
|
"Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||||
|
"Command: venv/bin/python -m pytest -q\n"
|
||||||
|
"Exit status: 1\n"
|
||||||
|
"Failure signature: AssertionError: None is not true\n"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
self.assertTrue(res["success"])
|
||||||
|
|
||||||
def test_merge_pr_with_closes_removes_label(self):
|
def test_merge_pr_with_closes_removes_label(self):
|
||||||
import reviewer_pr_lease
|
import reviewer_pr_lease
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,119 @@
|
|||||||
|
"""Post-merge moot validation wording + label tests (#529, criteria 1/4/5)."""
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
from post_merge_validation import (
|
||||||
|
BASELINE_ACCEPTED_OUTCOME,
|
||||||
|
BLOCKED_OUTCOME,
|
||||||
|
CLEAN_PASS_OUTCOME,
|
||||||
|
LABEL_BASELINE_ACCEPTED,
|
||||||
|
LABEL_BLOCKED,
|
||||||
|
LABEL_CLEAN_PASS,
|
||||||
|
LABEL_POST_MERGE_MOOT,
|
||||||
|
POST_MERGE_MOOT_OUTCOME,
|
||||||
|
assess_post_merge_validation,
|
||||||
|
process_state_label,
|
||||||
|
)
|
||||||
|
from final_report_validator import assess_final_report_validator
|
||||||
|
from issue_workflow_labels import CANONICAL_LABELS, VALIDATION_LABELS
|
||||||
|
|
||||||
|
_MOOT_PROOF = (
|
||||||
|
"Validation status: post-merge moot validation\n"
|
||||||
|
"PR state: merged\n"
|
||||||
|
"Merge commit sha: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||||
|
"Validation finding: full suite passed, for the record.\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestPostMergeValidation(unittest.TestCase):
|
||||||
|
def test_not_applicable_when_no_merged_or_moot_signal(self):
|
||||||
|
result = assess_post_merge_validation(
|
||||||
|
"Review decision: approve. Validation: full suite passed."
|
||||||
|
)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["skipped"])
|
||||||
|
|
||||||
|
def test_active_approval_on_merged_pr_blocked(self):
|
||||||
|
report = (
|
||||||
|
"PR is already merged. Review decision: approve. "
|
||||||
|
"Validation: full suite passed."
|
||||||
|
)
|
||||||
|
result = assess_post_merge_validation(report)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertEqual(result["outcome"], POST_MERGE_MOOT_OUTCOME)
|
||||||
|
self.assertEqual(result["process_state_label"], LABEL_POST_MERGE_MOOT)
|
||||||
|
|
||||||
|
def test_active_approval_on_merged_pr_via_structured_signal_blocked(self):
|
||||||
|
report = "Review decision: approve. Validation: full suite passed."
|
||||||
|
result = assess_post_merge_validation(report, pr_merged_or_closed=True)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
|
||||||
|
def test_moot_wording_without_proof_blocked(self):
|
||||||
|
report = "Recording post-merge moot validation for this PR."
|
||||||
|
result = assess_post_merge_validation(report)
|
||||||
|
self.assertTrue(result["block"])
|
||||||
|
self.assertTrue(result["reasons"])
|
||||||
|
|
||||||
|
def test_moot_wording_with_full_proof_allowed(self):
|
||||||
|
result = assess_post_merge_validation(_MOOT_PROOF)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertEqual(result["outcome"], POST_MERGE_MOOT_OUTCOME)
|
||||||
|
|
||||||
|
def test_merged_signal_only_guides_without_blocking(self):
|
||||||
|
result = assess_post_merge_validation(
|
||||||
|
"PR was already merged before review; recording findings."
|
||||||
|
)
|
||||||
|
self.assertFalse(result["block"])
|
||||||
|
self.assertTrue(result["safe_next_action"])
|
||||||
|
|
||||||
|
def test_process_state_label_mapping(self):
|
||||||
|
self.assertEqual(process_state_label(CLEAN_PASS_OUTCOME), LABEL_CLEAN_PASS)
|
||||||
|
self.assertEqual(
|
||||||
|
process_state_label(BASELINE_ACCEPTED_OUTCOME), LABEL_BASELINE_ACCEPTED
|
||||||
|
)
|
||||||
|
self.assertEqual(process_state_label(BLOCKED_OUTCOME), LABEL_BLOCKED)
|
||||||
|
self.assertEqual(
|
||||||
|
process_state_label(POST_MERGE_MOOT_OUTCOME), LABEL_POST_MERGE_MOOT
|
||||||
|
)
|
||||||
|
self.assertIsNone(process_state_label("nonsense"))
|
||||||
|
|
||||||
|
|
||||||
|
class TestValidationLabelsRegistered(unittest.TestCase):
|
||||||
|
def test_validation_labels_are_canonical(self):
|
||||||
|
for label in (
|
||||||
|
LABEL_CLEAN_PASS,
|
||||||
|
LABEL_BASELINE_ACCEPTED,
|
||||||
|
LABEL_BLOCKED,
|
||||||
|
LABEL_POST_MERGE_MOOT,
|
||||||
|
):
|
||||||
|
self.assertIn(label, VALIDATION_LABELS)
|
||||||
|
self.assertIn(label, CANONICAL_LABELS)
|
||||||
|
|
||||||
|
|
||||||
|
class TestPostMergeValidationWiredIntoReview(unittest.TestCase):
|
||||||
|
def test_review_pr_blocks_active_approval_on_merged_pr(self):
|
||||||
|
report = (
|
||||||
|
"PR is already merged. Review decision: approve. "
|
||||||
|
"Validation: full suite passed."
|
||||||
|
)
|
||||||
|
result = assess_final_report_validator(report, "review_pr")
|
||||||
|
self.assertTrue(result["blocked"])
|
||||||
|
self.assertTrue(
|
||||||
|
any(
|
||||||
|
f["rule_id"] == "reviewer.post_merge_validation"
|
||||||
|
for f in result["findings"]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_review_pr_allows_proven_post_merge_moot(self):
|
||||||
|
result = assess_final_report_validator(_MOOT_PROOF, "review_pr")
|
||||||
|
self.assertFalse(
|
||||||
|
any(
|
||||||
|
f["rule_id"] == "reviewer.post_merge_validation"
|
||||||
|
for f in result["findings"]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user