feat(review-workflow): enforce single terminal review decision per review run (#211)
This commit is contained in:
+72
-18
@@ -925,24 +925,17 @@ _REVIEW_ACTIONS = {
|
||||
_TERMINAL_REVIEW_ACTIONS = frozenset({"approve", "request_changes"})
|
||||
|
||||
REVIEW_DECISION_FILE = "/tmp/gitea_review_decision.lock"
|
||||
_REVIEW_DECISION_LOCK: dict | None = None
|
||||
|
||||
|
||||
def _load_review_decision_lock():
|
||||
if not os.path.exists(REVIEW_DECISION_FILE):
|
||||
return None
|
||||
try:
|
||||
with open(REVIEW_DECISION_FILE, "r", encoding="utf-8") as f:
|
||||
return json.load(f)
|
||||
except Exception:
|
||||
return None
|
||||
global _REVIEW_DECISION_LOCK
|
||||
return _REVIEW_DECISION_LOCK
|
||||
|
||||
|
||||
def _save_review_decision_lock(data):
|
||||
try:
|
||||
with open(REVIEW_DECISION_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(data, f)
|
||||
except Exception:
|
||||
pass
|
||||
global _REVIEW_DECISION_LOCK
|
||||
_REVIEW_DECISION_LOCK = data
|
||||
|
||||
|
||||
def init_review_decision_lock(remote: str | None, task: str | None):
|
||||
@@ -956,6 +949,9 @@ def init_review_decision_lock(remote: str | None, task: str | None):
|
||||
"ready_pr_number": None,
|
||||
"ready_action": None,
|
||||
"ready_expected_head_sha": None,
|
||||
"ready_remote": None,
|
||||
"ready_org": None,
|
||||
"ready_repo": None,
|
||||
"live_mutations": [],
|
||||
"correction_authorized": False,
|
||||
"correction_reason": None,
|
||||
@@ -967,6 +963,9 @@ def check_review_decision_gate(
|
||||
action: str,
|
||||
*,
|
||||
final_review_decision_ready: bool,
|
||||
remote: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
) -> list[str]:
|
||||
"""Fail closed unless validation completed and the final decision is ready."""
|
||||
reasons = []
|
||||
@@ -1004,6 +1003,21 @@ def check_review_decision_gate(
|
||||
f"ready action '{lock.get('ready_action')}' does not match "
|
||||
f"requested action '{action}' (fail closed)"
|
||||
)
|
||||
if lock.get("ready_remote") != remote:
|
||||
reasons.append(
|
||||
f"ready remote '{lock.get('ready_remote')}' does not match "
|
||||
f"requested remote '{remote}' (fail closed)"
|
||||
)
|
||||
if lock.get("ready_org") != org:
|
||||
reasons.append(
|
||||
f"ready org '{lock.get('ready_org')}' does not match "
|
||||
f"requested org '{org}' (fail closed)"
|
||||
)
|
||||
if lock.get("ready_repo") != repo:
|
||||
reasons.append(
|
||||
f"ready repo '{lock.get('ready_repo')}' does not match "
|
||||
f"requested repo '{repo}' (fail closed)"
|
||||
)
|
||||
|
||||
prior = list(lock.get("live_mutations") or [])
|
||||
if prior and not lock.get("correction_authorized"):
|
||||
@@ -1026,10 +1040,15 @@ def check_review_decision_gate(
|
||||
return reasons
|
||||
|
||||
|
||||
def record_live_review_mutation(pr_number: int, action: str):
|
||||
def record_live_review_mutation(pr_number: int, action: str, review_id: int | None = None):
|
||||
lock = _load_review_decision_lock() or {}
|
||||
mutations = list(lock.get("live_mutations") or [])
|
||||
mutations.append({"pr_number": pr_number, "action": action})
|
||||
mutations.append({
|
||||
"pr_number": pr_number,
|
||||
"action": action,
|
||||
"review_id": review_id,
|
||||
"review_state": action,
|
||||
})
|
||||
lock["live_mutations"] = mutations
|
||||
if lock.get("correction_authorized"):
|
||||
lock["correction_authorized"] = False
|
||||
@@ -1245,6 +1264,9 @@ def _evaluate_pr_review_submission(
|
||||
pr_number,
|
||||
action,
|
||||
final_review_decision_ready=final_review_decision_ready,
|
||||
remote=remote,
|
||||
org=org,
|
||||
repo=repo,
|
||||
))
|
||||
if reasons:
|
||||
return result
|
||||
@@ -1332,6 +1354,8 @@ def gitea_mark_final_review_decision(
|
||||
action: str,
|
||||
expected_head_sha: str | None = None,
|
||||
remote: str = "dadeschools",
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
) -> dict:
|
||||
"""Mark validation complete; the final review decision is ready to submit."""
|
||||
action = (action or "").strip().lower()
|
||||
@@ -1363,31 +1387,59 @@ def gitea_mark_final_review_decision(
|
||||
lock["ready_pr_number"] = pr_number
|
||||
lock["ready_action"] = action
|
||||
lock["ready_expected_head_sha"] = expected_head_sha
|
||||
lock["ready_remote"] = remote
|
||||
lock["ready_org"] = org
|
||||
lock["ready_repo"] = repo
|
||||
_save_review_decision_lock(lock)
|
||||
return {
|
||||
"marked_ready": True,
|
||||
"pr_number": pr_number,
|
||||
"action": action,
|
||||
"expected_head_sha": expected_head_sha,
|
||||
"remote": remote,
|
||||
"org": org,
|
||||
"repo": repo,
|
||||
"final_review_decision_ready": True,
|
||||
"reasons": [],
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_authorize_review_correction(reason: str) -> dict:
|
||||
def gitea_authorize_review_correction(
|
||||
prior_review_id: int,
|
||||
prior_review_state: str,
|
||||
reason: str,
|
||||
) -> dict:
|
||||
"""Authorize one operator-approved correction after a mistaken live review."""
|
||||
reason = (reason or "").strip()
|
||||
prior_review_state = (prior_review_state or "").strip().lower()
|
||||
lock = _load_review_decision_lock()
|
||||
if lock is None:
|
||||
return {"authorized": False, "reasons": ["review decision lock missing"]}
|
||||
if not lock.get("live_mutations"):
|
||||
prior = list(lock.get("live_mutations") or [])
|
||||
if not prior:
|
||||
return {
|
||||
"authorized": False,
|
||||
"reasons": ["no prior live review mutation to correct"],
|
||||
}
|
||||
last_mutation = prior[-1]
|
||||
last_review_id = last_mutation.get("review_id")
|
||||
last_review_state = last_mutation.get("action")
|
||||
reasons = []
|
||||
if last_review_id is not None and last_review_id != prior_review_id:
|
||||
reasons.append(
|
||||
f"prior review ID '{prior_review_id}' does not match last "
|
||||
f"recorded review ID '{last_review_id}' (fail closed)"
|
||||
)
|
||||
if last_review_state != prior_review_state:
|
||||
reasons.append(
|
||||
f"prior review state '{prior_review_state}' does not match last "
|
||||
f"recorded review state '{last_review_state}' (fail closed)"
|
||||
)
|
||||
if not reason:
|
||||
return {"authorized": False, "reasons": ["correction reason is required"]}
|
||||
reasons.append("correction reason is required")
|
||||
if reasons:
|
||||
return {"authorized": False, "reasons": reasons}
|
||||
lock["correction_authorized"] = True
|
||||
lock["correction_reason"] = reason
|
||||
_save_review_decision_lock(lock)
|
||||
@@ -1885,6 +1937,7 @@ def gitea_review_pr(
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
final_review_decision_ready: bool = False,
|
||||
) -> dict:
|
||||
"""Submit a review on a Gitea pull request (Legacy wrapper).
|
||||
|
||||
@@ -2059,7 +2112,8 @@ def gitea_review_pr(
|
||||
remote=remote,
|
||||
host=host,
|
||||
org=org,
|
||||
repo=repo
|
||||
repo=repo,
|
||||
final_review_decision_ready=final_review_decision_ready
|
||||
)
|
||||
|
||||
# Include the inventory report in the response message
|
||||
|
||||
Reference in New Issue
Block a user