Durable review-decision locks (#559) can outlive the PR they protect. When the last terminal mutation references a PR that is already merged/closed, expose gitea_cleanup_stale_review_decision_lock so a reviewer can clear the lock with identity/profile gates and a durable audit trail — without weakening #332 for open PRs or deleting session-state files by hand. Also auto-clear same-profile locks after a successful merge of the approved PR, and document allowed vs forbidden cleanup. Closes #594
This commit is contained in:
+229
-1
@@ -807,6 +807,7 @@ import review_proofs # noqa: E402
|
||||
import review_workflow_boundary # noqa: E402
|
||||
import review_workflow_load # noqa: E402
|
||||
import mcp_session_state # noqa: E402
|
||||
import stale_review_decision_lock # noqa: E402
|
||||
import agent_temp_artifacts
|
||||
import issue_lock_worktree # noqa: E402
|
||||
import issue_lock_provenance # noqa: E402
|
||||
@@ -3096,10 +3097,15 @@ def terminal_review_hard_stop_reasons(pr_number: int, operation: str) -> list[st
|
||||
)
|
||||
else:
|
||||
guidance = "the session must stop and produce a final report"
|
||||
recovery = (
|
||||
"if that PR is already merged/closed, use "
|
||||
"gitea_cleanup_stale_review_decision_lock (apply=true) after live-state "
|
||||
"proof (#594); never delete session-state files by hand"
|
||||
)
|
||||
return [
|
||||
"terminal review mutation already consumed in this run "
|
||||
f"({last.get('action')} on PR #{last.get('pr_number')}); {guidance} "
|
||||
"(fail closed, #332)"
|
||||
f"(fail closed, #332); {recovery}"
|
||||
]
|
||||
|
||||
|
||||
@@ -3872,6 +3878,205 @@ def gitea_authorize_review_correction(
|
||||
return {"authorized": True, "correction_reason": reason, "reasons": []}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_cleanup_stale_review_decision_lock(
|
||||
apply: bool = False,
|
||||
expected_terminal_pr: int | None = None,
|
||||
remote: str = "dadeschools",
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
post_audit_comment: bool = True,
|
||||
) -> dict:
|
||||
"""Clear a durable #332 review-decision lock only when it is moot (#594).
|
||||
|
||||
Read-first by default (``apply=False``). A lock is moot when its last
|
||||
terminal live mutation references a PR that is already **merged** or
|
||||
**closed** on live Gitea — so no same-PR merge sequence remains.
|
||||
|
||||
Fail-closed when:
|
||||
- no lock / no terminal mutation
|
||||
- live PR is still open (active #332 hard-stop preserved)
|
||||
- live PR state cannot be fetched
|
||||
- active profile identity does not match the lock
|
||||
- apply requested without reviewer capability (``gitea.pr.review``)
|
||||
|
||||
Never weakens #332 for open PRs. Never instructs manual session-state
|
||||
file deletion. On successful apply, clears memory + durable store and
|
||||
returns a durable audit record (optionally posted as a PR comment).
|
||||
"""
|
||||
read_block = _profile_operation_gate("gitea.read")
|
||||
if read_block:
|
||||
return {
|
||||
"success": False,
|
||||
"cleanup_performed": False,
|
||||
"is_moot": False,
|
||||
"cleanup_allowed": False,
|
||||
"mode": "apply" if apply else "read_only",
|
||||
"reasons": read_block,
|
||||
"permission_report": _permission_block_report("gitea.read"),
|
||||
}
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
binding = _decision_lock_binding()
|
||||
active_identity = binding.get("profile_identity")
|
||||
lock = _load_review_decision_lock()
|
||||
last = stale_review_decision_lock.last_terminal_mutation(lock)
|
||||
pr_live = None
|
||||
pr_lookup_error = None
|
||||
last_pr = last.get("pr_number") if last else None
|
||||
|
||||
if last_pr is not None:
|
||||
try:
|
||||
pr_live = api_request(
|
||||
"GET", f"{repo_api_url(h, o, r)}/pulls/{last_pr}", auth
|
||||
) or {}
|
||||
if not pr_live:
|
||||
pr_lookup_error = "empty PR payload"
|
||||
except Exception as exc: # noqa: BLE001 — surface redacted
|
||||
pr_lookup_error = _redact(str(exc))
|
||||
|
||||
assessment = stale_review_decision_lock.assess_stale_review_decision_lock(
|
||||
lock,
|
||||
pr_live=pr_live,
|
||||
pr_lookup_error=pr_lookup_error,
|
||||
active_profile_identity=active_identity,
|
||||
)
|
||||
|
||||
# Optional pin: refuse apply against a different terminal PR than expected.
|
||||
if (
|
||||
expected_terminal_pr is not None
|
||||
and assessment.get("last_terminal_pr") is not None
|
||||
and int(expected_terminal_pr) != int(assessment["last_terminal_pr"])
|
||||
):
|
||||
assessment = dict(assessment)
|
||||
assessment["cleanup_allowed"] = False
|
||||
assessment["reasons"] = list(assessment.get("reasons") or []) + [
|
||||
f"expected_terminal_pr #{expected_terminal_pr} does not match "
|
||||
f"last terminal PR #{assessment.get('last_terminal_pr')} "
|
||||
"(fail closed, #594)"
|
||||
]
|
||||
|
||||
try:
|
||||
actor = _authenticated_username(h)
|
||||
except Exception:
|
||||
actor = None
|
||||
profile = get_profile()
|
||||
profile_name = (profile.get("profile_name") or "").strip() or None
|
||||
|
||||
audit = stale_review_decision_lock.build_cleanup_audit_record(
|
||||
assessment=assessment,
|
||||
actor_username=actor,
|
||||
profile_name=profile_name,
|
||||
applied=False,
|
||||
)
|
||||
|
||||
report = {
|
||||
"success": True,
|
||||
"cleanup_performed": False,
|
||||
"mode": "apply" if apply else "read_only",
|
||||
"remote": remote,
|
||||
"org": o,
|
||||
"repo": r,
|
||||
"active_profile_identity": active_identity,
|
||||
"authenticated_username": actor,
|
||||
"has_lock": assessment.get("has_lock"),
|
||||
"is_moot": assessment.get("is_moot"),
|
||||
"cleanup_allowed": assessment.get("cleanup_allowed"),
|
||||
"last_terminal_pr": assessment.get("last_terminal_pr"),
|
||||
"last_terminal_action": assessment.get("last_terminal_action"),
|
||||
"pr_state": assessment.get("pr_state"),
|
||||
"pr_merged": assessment.get("pr_merged"),
|
||||
"pr_merged_or_closed": assessment.get("pr_merged_or_closed"),
|
||||
"merge_commit_sha": assessment.get("merge_commit_sha"),
|
||||
"lock_summary": assessment.get("lock_summary"),
|
||||
"audit": audit,
|
||||
"audit_comment_id": None,
|
||||
"reasons": list(assessment.get("reasons") or []),
|
||||
"manual_file_delete_forbidden": True,
|
||||
}
|
||||
|
||||
if not apply:
|
||||
return report
|
||||
|
||||
if not assessment.get("cleanup_allowed"):
|
||||
report["success"] = False
|
||||
report["cleanup_skipped_reason"] = list(assessment.get("reasons") or [])
|
||||
return report
|
||||
|
||||
if not actor:
|
||||
report["success"] = False
|
||||
report["reasons"] = [
|
||||
"authenticated identity could not be verified; refuse lock "
|
||||
"cleanup (fail closed, #594)"
|
||||
]
|
||||
return report
|
||||
|
||||
review_block = _profile_operation_gate("gitea.pr.review")
|
||||
if review_block:
|
||||
report["success"] = False
|
||||
report["reasons"] = review_block
|
||||
report["permission_report"] = _permission_block_report("gitea.pr.review")
|
||||
return report
|
||||
|
||||
session_reasons = _review_decision_session_reasons(lock)
|
||||
if session_reasons:
|
||||
report["success"] = False
|
||||
report["reasons"] = session_reasons
|
||||
return report
|
||||
|
||||
# Clear durable + in-memory lock only after all gates pass.
|
||||
prior_summary = assessment.get("lock_summary")
|
||||
_save_review_decision_lock(None)
|
||||
audit = stale_review_decision_lock.build_cleanup_audit_record(
|
||||
assessment=assessment,
|
||||
actor_username=actor,
|
||||
profile_name=profile_name,
|
||||
applied=True,
|
||||
)
|
||||
audit["prior_lock_summary"] = prior_summary
|
||||
report["cleanup_performed"] = True
|
||||
report["audit"] = audit
|
||||
report["reasons"] = list(assessment.get("reasons") or []) + [
|
||||
f"cleared durable review decision lock for moot terminal mutation "
|
||||
f"on PR #{assessment.get('last_terminal_pr')} (#594)"
|
||||
]
|
||||
|
||||
if post_audit_comment and assessment.get("last_terminal_pr") is not None:
|
||||
comment_block = _profile_operation_gate("gitea.pr.comment")
|
||||
if comment_block:
|
||||
report["audit_comment_skipped"] = comment_block
|
||||
else:
|
||||
try:
|
||||
body = stale_review_decision_lock.format_cleanup_audit_comment(
|
||||
audit
|
||||
)
|
||||
comment_url = (
|
||||
f"{repo_api_url(h, o, r)}/issues/"
|
||||
f"{assessment['last_terminal_pr']}/comments"
|
||||
)
|
||||
with _audited(
|
||||
"comment_pr",
|
||||
host=h,
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
pr_number=assessment["last_terminal_pr"],
|
||||
request_metadata={
|
||||
"source": "cleanup_stale_review_decision_lock"
|
||||
},
|
||||
):
|
||||
posted = api_request(
|
||||
"POST", comment_url, auth, {"body": body}
|
||||
)
|
||||
report["audit_comment_id"] = (posted or {}).get("id")
|
||||
except Exception as exc: # noqa: BLE001
|
||||
report["audit_comment_error"] = _redact(str(exc))
|
||||
|
||||
return report
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_dry_run_pr_review(
|
||||
pr_number: int,
|
||||
@@ -4609,6 +4814,29 @@ def gitea_merge_pr(
|
||||
reviewer_pr_lease.get_session_lease()
|
||||
)
|
||||
)
|
||||
# Same-profile auto-expire (#594): if *this* profile's decision lock ends
|
||||
# with an approve of the PR just merged, clear it so durable state cannot
|
||||
# block later unrelated reviews. Cross-profile locks (e.g. reviewer vs
|
||||
# merger) still require gitea_cleanup_stale_review_decision_lock.
|
||||
try:
|
||||
lock_after = _load_review_decision_lock()
|
||||
last_term = stale_review_decision_lock.last_terminal_mutation(lock_after)
|
||||
if (
|
||||
last_term
|
||||
and last_term.get("action") == "approve"
|
||||
and last_term.get("pr_number") == pr_number
|
||||
):
|
||||
_save_review_decision_lock(None)
|
||||
result["review_decision_lock_cleared_after_merge"] = True
|
||||
reasons.append(
|
||||
f"cleared same-profile review decision lock after merge of "
|
||||
f"approved PR #{pr_number} (#594)"
|
||||
)
|
||||
else:
|
||||
result["review_decision_lock_cleared_after_merge"] = False
|
||||
except Exception as clear_exc: # noqa: BLE001 — never fail the merge
|
||||
result["review_decision_lock_cleared_after_merge"] = False
|
||||
result["review_decision_lock_clear_error"] = _redact(str(clear_exc))
|
||||
reasons.append(f"all gates passed; merged PR #{pr_number} via '{do}'")
|
||||
return result
|
||||
|
||||
|
||||
Reference in New Issue
Block a user