status:pr-open was applied by gitea_create_pr and never removed again. Every
terminal path finished without touching it, so a repository audit found 40
closed issues still advertising an open PR.
Add terminal_pr_label_cleanup.py as the single authoritative rule and route
every sanctioned terminal path through it, so the paths cannot drift:
- merge (gitea_merge_pr)
- close without merge (gitea_edit_pr)
- supersession/abandonment (gitea_reconcile_superseded_by_merged_pr)
- already-landed reconciliation (gitea_reconcile_already_landed_pr)
- controller closure (gitea_close_issue)
- retry/recovery (new gitea_cleanup_terminal_pr_labels)
The rule removes only status:pr-open, preserves every other label, allows an
empty resulting set, is a no-op when the label is absent (so retries are
safe), and confirms the outcome by read-after-write rather than assumption.
Controller closure runs the cleanup before the state change and fails closed
if it cannot be completed and verified; closing first would bake in the stale
label with no later step to catch it. Post-merge cleanup never blocks the
merge, which already happened, and reports failures with a safe next action.
Also:
- gitea_assess_terminal_label_hygiene: read-only terminal validation that
reports residual status:pr-open, exempting issues with a genuinely open PR.
- _put_issue_label_names now accepts Gitea's empty response body when the
requested set is empty, so clearing the last label works.
- test_audit's close_issue fixture keys on the request instead of call order,
since closing now also reads labels for the cleanup and its read-back.
Docs: label-taxonomy terminal-transition section, runbook pointer, and the
review-merge / reconcile-landed final-report terminal-label requirements.
Suite: 4045 passed, 11 failed, 6 skipped. The same 11 failures reproduce on
clean master df31674 (4010 passed, 11 failed) and are pre-existing.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
309 lines
10 KiB
Python
309 lines
10 KiB
Python
"""Authoritative terminal-transition cleanup for ``status:pr-open`` (#780).
|
|
|
|
``status:pr-open`` is applied by ``gitea_create_pr`` while a linked pull
|
|
request is open. Nothing removed it again: the workflow's terminal paths
|
|
(merge, close-without-merge, supersession, already-landed reconciliation,
|
|
controller closure) each ended without touching the label, so a repository
|
|
audit found 40 closed issues still carrying it.
|
|
|
|
This module is the single source of truth for that cleanup. Every sanctioned
|
|
terminal path plans its label mutation here rather than implementing its own
|
|
rule, so the paths cannot drift apart:
|
|
|
|
- :func:`plan_pr_open_cleanup` decides the exact resulting label set. It only
|
|
ever removes ``status:pr-open``; every other label is preserved verbatim,
|
|
including the case where the result is an empty label set.
|
|
- :func:`verify_pr_open_cleanup` is the read-after-write check. It proves the
|
|
label is gone *and* that no unrelated label was dropped or added.
|
|
- :func:`detect_residual_pr_open` is the terminal validation: given issues, it
|
|
reports any that still carry the label, so a controller closure or audit
|
|
fails loudly instead of leaving the leak behind.
|
|
|
|
The rule is idempotent by construction: an issue without the label plans no
|
|
mutation, so retries and recovery re-runs are harmless.
|
|
|
|
This module performs no I/O — callers own the Gitea API calls.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import Any, Iterable, Mapping, Sequence
|
|
|
|
import issue_workflow_labels
|
|
|
|
#: The single label this module is responsible for retiring.
|
|
PR_OPEN_LABEL = "status:pr-open"
|
|
|
|
# Canonical terminal reasons — the sanctioned ways an issue can end up
|
|
# associated with a pull request that is no longer open.
|
|
MERGED = "merged"
|
|
CLOSED_WITHOUT_MERGE = "closed_without_merge"
|
|
SUPERSEDED = "superseded"
|
|
ALREADY_LANDED = "already_landed"
|
|
CONTROLLER_CLOSURE = "controller_closure"
|
|
ABANDONED = "abandoned"
|
|
RETRY_RECOVERY = "retry_recovery"
|
|
|
|
TERMINAL_REASONS: tuple[str, ...] = (
|
|
MERGED,
|
|
CLOSED_WITHOUT_MERGE,
|
|
SUPERSEDED,
|
|
ALREADY_LANDED,
|
|
CONTROLLER_CLOSURE,
|
|
ABANDONED,
|
|
RETRY_RECOVERY,
|
|
)
|
|
|
|
_REASON_ALIASES: dict[str, str] = {
|
|
"merge": MERGED,
|
|
"merged": MERGED,
|
|
"pr_merged": MERGED,
|
|
"closed": CLOSED_WITHOUT_MERGE,
|
|
"close": CLOSED_WITHOUT_MERGE,
|
|
"closed_without_merge": CLOSED_WITHOUT_MERGE,
|
|
"pr_closed": CLOSED_WITHOUT_MERGE,
|
|
"supersede": SUPERSEDED,
|
|
"superseded": SUPERSEDED,
|
|
"supersession": SUPERSEDED,
|
|
"already_landed": ALREADY_LANDED,
|
|
"reconcile_already_landed": ALREADY_LANDED,
|
|
"controller_closure": CONTROLLER_CLOSURE,
|
|
"close_issue": CONTROLLER_CLOSURE,
|
|
"abandon": ABANDONED,
|
|
"abandoned": ABANDONED,
|
|
"retry": RETRY_RECOVERY,
|
|
"recovery": RETRY_RECOVERY,
|
|
"retry_recovery": RETRY_RECOVERY,
|
|
}
|
|
|
|
#: Human-readable phrasing used in audit comments and diagnostics.
|
|
REASON_DESCRIPTIONS: dict[str, str] = {
|
|
MERGED: "the linked PR was merged",
|
|
CLOSED_WITHOUT_MERGE: "the linked PR was closed without merging",
|
|
SUPERSEDED: "the linked PR was superseded by another merged PR",
|
|
ALREADY_LANDED: "the linked PR's change was already on the target branch",
|
|
CONTROLLER_CLOSURE: "the issue reached controller closure",
|
|
ABANDONED: "the linked PR was abandoned",
|
|
RETRY_RECOVERY: "a partial terminal transition is being recovered",
|
|
}
|
|
|
|
|
|
def canonical_terminal_reason(reason: str | None) -> str:
|
|
"""Normalize a terminal reason, failing closed on anything unrecognized."""
|
|
name = (reason or "").strip()
|
|
if name in TERMINAL_REASONS:
|
|
return name
|
|
normalized = name.lower().replace("-", "_").replace(" ", "_")
|
|
try:
|
|
return _REASON_ALIASES[normalized]
|
|
except KeyError as exc:
|
|
raise ValueError(
|
|
f"unknown terminal PR reason '{reason}' (expected one of: "
|
|
+ ", ".join(TERMINAL_REASONS)
|
|
+ ")"
|
|
) from exc
|
|
|
|
|
|
def plan_pr_open_cleanup(
|
|
current_labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
|
|
*,
|
|
terminal_reason: str,
|
|
) -> dict[str, Any]:
|
|
"""Plan the label set an issue must carry after a terminal PR transition.
|
|
|
|
The plan removes ``status:pr-open`` and nothing else. When the label is
|
|
absent the plan is an explicit no-op (``cleanup_required`` False), which is
|
|
what makes repeated cleanup calls harmless. When it was the only label the
|
|
resulting set is legitimately empty.
|
|
"""
|
|
reason = canonical_terminal_reason(terminal_reason)
|
|
before = issue_workflow_labels.label_names(current_labels)
|
|
after = [name for name in before if name != PR_OPEN_LABEL]
|
|
present = len(after) != len(before)
|
|
return {
|
|
"terminal_reason": reason,
|
|
"terminal_reason_description": REASON_DESCRIPTIONS[reason],
|
|
"label": PR_OPEN_LABEL,
|
|
"label_present": present,
|
|
"cleanup_required": present,
|
|
"idempotent_noop": not present,
|
|
"labels_before": before,
|
|
"labels_after": after,
|
|
"removed": [PR_OPEN_LABEL] if present else [],
|
|
"preserved": list(after),
|
|
"empty_label_set": not after,
|
|
}
|
|
|
|
|
|
def verify_pr_open_cleanup(
|
|
observed_labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
|
|
*,
|
|
plan: Mapping[str, Any],
|
|
) -> dict[str, Any]:
|
|
"""Read-after-write check for a planned cleanup.
|
|
|
|
Verifies the label is gone and that the observed set matches the plan
|
|
exactly, so an unrelated label silently dropped (or re-added) by the API is
|
|
reported rather than accepted.
|
|
"""
|
|
observed = issue_workflow_labels.label_names(observed_labels)
|
|
expected = list(plan.get("labels_after") or [])
|
|
observed_set = set(observed)
|
|
expected_set = set(expected)
|
|
residual = PR_OPEN_LABEL in observed_set
|
|
unexpected_removals = sorted(expected_set - observed_set)
|
|
unexpected_additions = sorted(observed_set - expected_set - {PR_OPEN_LABEL})
|
|
|
|
reasons: list[str] = []
|
|
if residual:
|
|
reasons.append(
|
|
f"'{PR_OPEN_LABEL}' is still present after terminal cleanup"
|
|
)
|
|
if unexpected_removals:
|
|
reasons.append(
|
|
"unrelated labels were dropped by the cleanup: "
|
|
+ ", ".join(unexpected_removals)
|
|
)
|
|
if unexpected_additions:
|
|
reasons.append(
|
|
"unexpected labels appeared during the cleanup: "
|
|
+ ", ".join(unexpected_additions)
|
|
)
|
|
|
|
verified = not reasons
|
|
return {
|
|
"verified": verified,
|
|
"residual": residual,
|
|
"observed_labels": observed,
|
|
"expected_labels": expected,
|
|
"unexpected_removals": unexpected_removals,
|
|
"unexpected_additions": unexpected_additions,
|
|
"empty_label_set": not observed,
|
|
"reasons": reasons,
|
|
"safe_next_action": (
|
|
""
|
|
if verified
|
|
else (
|
|
"Re-run the terminal cleanup for this issue with "
|
|
f"terminal_reason='{RETRY_RECOVERY}' and confirm the read-back "
|
|
f"no longer reports '{PR_OPEN_LABEL}'."
|
|
)
|
|
),
|
|
}
|
|
|
|
|
|
def summarize_cleanup_results(
|
|
results: Sequence[Mapping[str, Any]],
|
|
*,
|
|
terminal_reason: str,
|
|
) -> dict[str, Any]:
|
|
"""Aggregate per-issue cleanup outcomes into one reportable record."""
|
|
reason = canonical_terminal_reason(terminal_reason)
|
|
entries = [dict(entry) for entry in results]
|
|
removed = [e.get("issue_number") for e in entries if e.get("status") == "removed"]
|
|
absent = [
|
|
e.get("issue_number") for e in entries if e.get("status") == "not present"
|
|
]
|
|
failed = [
|
|
e.get("issue_number")
|
|
for e in entries
|
|
if e.get("status") not in ("removed", "not present") or not e.get("verified")
|
|
]
|
|
reasons: list[str] = []
|
|
for entry in entries:
|
|
for text in entry.get("reasons") or []:
|
|
reasons.append(f"issue #{entry.get('issue_number')}: {text}")
|
|
clean = not failed
|
|
return {
|
|
"label": PR_OPEN_LABEL,
|
|
"terminal_reason": reason,
|
|
"clean": clean,
|
|
"checked": [e.get("issue_number") for e in entries],
|
|
"removed": removed,
|
|
"already_absent": absent,
|
|
"failed": failed,
|
|
"results": entries,
|
|
"reasons": reasons,
|
|
"safe_next_action": (
|
|
""
|
|
if clean
|
|
else (
|
|
"Terminal label cleanup did not complete for "
|
|
+ ", ".join(f"#{num}" for num in failed)
|
|
+ ". Re-run gitea_cleanup_terminal_pr_labels with "
|
|
f"terminal_reason='{RETRY_RECOVERY}' for those issues."
|
|
)
|
|
),
|
|
}
|
|
|
|
|
|
def detect_residual_pr_open(
|
|
issues: Iterable[Mapping[str, Any]],
|
|
*,
|
|
open_pr_issue_numbers: Iterable[int] = (),
|
|
) -> dict[str, Any]:
|
|
"""Terminal validation: report issues still carrying ``status:pr-open``.
|
|
|
|
An issue with a genuinely open pull request is allowed to keep the label,
|
|
so *open_pr_issue_numbers* is excluded from the residual set rather than
|
|
being reported as a leak.
|
|
"""
|
|
legitimate: set[int] = set()
|
|
for num in open_pr_issue_numbers or ():
|
|
try:
|
|
legitimate.add(int(num))
|
|
except (TypeError, ValueError):
|
|
continue
|
|
|
|
checked = 0
|
|
residual: list[dict[str, Any]] = []
|
|
exempt: list[int] = []
|
|
|
|
for issue in issues or []:
|
|
checked += 1
|
|
names = issue_workflow_labels.label_names(issue)
|
|
if PR_OPEN_LABEL not in names:
|
|
continue
|
|
try:
|
|
number = int(issue.get("number"))
|
|
except (TypeError, ValueError):
|
|
number = None
|
|
if number is not None and number in legitimate:
|
|
exempt.append(number)
|
|
continue
|
|
residual.append(
|
|
{
|
|
"number": number,
|
|
"state": issue.get("state"),
|
|
"labels": names,
|
|
}
|
|
)
|
|
|
|
clean = not residual
|
|
reasons = [
|
|
(
|
|
f"issue #{entry['number']} ({entry.get('state') or 'unknown state'}) "
|
|
f"still carries '{PR_OPEN_LABEL}' with no open PR"
|
|
)
|
|
for entry in residual
|
|
]
|
|
return {
|
|
"label": PR_OPEN_LABEL,
|
|
"clean": clean,
|
|
"checked_count": checked,
|
|
"residual_count": len(residual),
|
|
"residual_issues": residual,
|
|
"exempt_open_pr_issues": sorted(exempt),
|
|
"reasons": reasons,
|
|
"safe_next_action": (
|
|
""
|
|
if clean
|
|
else (
|
|
"Run gitea_cleanup_terminal_pr_labels with "
|
|
f"terminal_reason='{RETRY_RECOVERY}' for issues "
|
|
+ ", ".join(f"#{entry['number']}" for entry in residual)
|
|
+ " before declaring the terminal transition complete."
|
|
)
|
|
),
|
|
}
|