feat: preserve and resume prepared review verdict drafts (#609) #611

Open
jcwalker3 wants to merge 1 commits from feat/issue-609-prepared-review-verdict-resume into master
3 changed files with 717 additions and 0 deletions
+329
View File
@@ -4162,6 +4162,335 @@ def gitea_submit_pr_review(
)
@mcp.tool()
def gitea_save_review_draft(
pr_number: int,
action: str,
body: str = "",
expected_head_sha: str | None = None,
validation_commands: str | None = None,
validation_results: str | None = None,
blocker_reason: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
worktree_path: str | None = None,
) -> dict:
"""Save a prepared review verdict as non-terminal draft evidence without submitting a formal review.
This permits resuming the review later (e.g. after a stale runtime restart or connection drop).
"""
action = (action or "").strip().lower()
if action not in _REVIEW_ACTIONS:
return {
"success": False,
"reasons": [
f"unknown review action '{action}'; expected one of "
f"{sorted(_REVIEW_ACTIONS)}"
],
}
# 1. Resolve remote context
try:
h, resolved_org, resolved_repo = _resolve(remote, host, org, repo)
except ValueError as exc:
return {"success": False, "reasons": [str(exc)]}
auth = _auth(h)
# 2. Fetch current PR details to get target base branch and base branch SHA
try:
pr_url = f"{repo_api_url(h, resolved_org, resolved_repo)}/pulls/{pr_number}"
pr = api_request("GET", pr_url, auth) or {}
except Exception as exc:
return {
"success": False,
"reasons": [f"failed to fetch PR #{pr_number} from Gitea: {str(exc)}"],
}
current_head = (pr.get("head") or {}).get("sha")
target_branch = (pr.get("base") or {}).get("ref")
target_branch_sha = (pr.get("base") or {}).get("sha")
if expected_head_sha and current_head and expected_head_sha != current_head:
return {
"success": False,
"reasons": [
f"expected_head_sha '{expected_head_sha}' does not match current PR head SHA '{current_head}'"
],
}
# 3. Resolve worktree path
try:
resolved_worktree = _resolve_preflight_workspace_path(worktree_path)
except Exception as exc:
return {"success": False, "reasons": [f"failed to resolve worktree path: {str(exc)}"]}
# 4. Save payload to durable session state
profile = get_profile()
payload = {
"pr_number": pr_number,
"action": action,
"body": body,
"head_sha": expected_head_sha or current_head,
"target_branch": target_branch,
"target_branch_sha": target_branch_sha,
"worktree_path": resolved_worktree,
"validation_commands": validation_commands,
"validation_results": validation_results,
"blocker_reason": blocker_reason,
"saved_by_identity": _authenticated_username(h) or "",
"saved_by_profile": profile.get("profile_name"),
"remote": remote,
"org": resolved_org,
"repo": resolved_repo,
}
try:
mcp_session_state.save_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
payload=payload,
remote=remote,
org=resolved_org,
repo=resolved_repo,
profile_identity=profile.get("profile_name"),
)
except Exception as exc:
return {"success": False, "reasons": [f"failed to save draft state: {str(exc)}"]}
return {
"success": True,
"pr_number": pr_number,
"action": action,
"head_sha": payload["head_sha"],
"target_branch": target_branch,
"target_branch_sha": target_branch_sha,
"worktree_path": resolved_worktree,
}
@mcp.tool()
def gitea_resume_review_draft(
pr_number: int,
submit: bool = False,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
worktree_path: str | None = None,
) -> dict:
"""Resume a previously saved review draft for the given PR number.
Performs live-state checks (PR head SHA, target branch SHA, terminal lock, active leases,
runtime/master parity, reviewer identity, worktree binding, validation freshness).
Refuses to submit or mark ready if any state has changed unsafely.
"""
profile = get_profile()
active_profile = profile.get("profile_name")
# 1. Load draft state
try:
draft = mcp_session_state.load_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
remote=remote,
org=org,
repo=repo,
profile_identity=active_profile,
)
except Exception as exc:
return {"success": False, "reasons": [f"failed to load draft state: {str(exc)}"]}
if not draft:
return {
"success": False,
"reasons": [f"no review draft found for PR #{pr_number} under profile '{active_profile}'"],
}
if draft.get("pr_number") != pr_number:
return {
"success": False,
"reasons": [
f"loaded draft is for PR #{draft.get('pr_number')}, but requested PR #{pr_number}"
],
}
# 2. Resolve remote context
try:
h, resolved_org, resolved_repo = _resolve(remote, host, org, repo)
except ValueError as exc:
return {"success": False, "reasons": [str(exc)]}
auth = _auth(h)
# 3. Fetch live PR state
try:
pr_url = f"{repo_api_url(h, resolved_org, resolved_repo)}/pulls/{pr_number}"
pr = api_request("GET", pr_url, auth) or {}
except Exception as exc:
return {
"success": False,
"reasons": [f"failed to fetch PR #{pr_number} from Gitea: {str(exc)}"],
}
# 4. Perform live-state checks
reasons = []
# PR must be open
if pr.get("state") != "open":
reasons.append(f"PR #{pr_number} is not open (state: '{pr.get('state')}')")
# Live head SHA must match saved head_sha
live_head = (pr.get("head") or {}).get("sha")
saved_head = draft.get("head_sha")
if live_head != saved_head:
reasons.append(
f"PR head SHA changed (live={live_head!r}, saved={saved_head!r})"
)
# Live target branch SHA must match saved target_branch_sha
live_target_sha = (pr.get("base") or {}).get("sha")
saved_target_sha = draft.get("target_branch_sha")
if live_target_sha != saved_target_sha:
reasons.append(
f"target branch SHA changed (live={live_target_sha!r}, saved={saved_target_sha!r})"
)
# Check #332 terminal lock
hard_stop = terminal_review_hard_stop_reasons(pr_number, "resume")
if hard_stop:
reasons.extend(hard_stop)
# Check active reviewer lease (must be claimed by the current session)
try:
comments = _fetch_pr_comments(pr_number, remote=remote, host=host, org=resolved_org, repo=resolved_repo)
except Exception as exc:
reasons.append(f"failed to fetch PR comments: {str(exc)}")
comments = []
active_lease = reviewer_pr_lease.find_active_reviewer_lease(comments, pr_number=pr_number)
if not active_lease:
reasons.append(f"no active reviewer lease found on PR #{pr_number}")
else:
# Check that lease owner is our session_id
session = reviewer_pr_lease.get_session_lease()
session_id = (session or {}).get("session_id")
owner_session_id = active_lease.get("session_id")
if owner_session_id != session_id:
reasons.append(
f"active reviewer lease belongs to session_id={owner_session_id!r}, "
f"but current session has session_id={session_id!r}"
)
# Check runtime/master parity
if "PYTEST_CURRENT_TEST" not in os.environ or "GITEA_FORCE_MCP_RUNTIME_CHECK" in os.environ:
config = gitea_config.load_config()
matching_profiles = []
if config and "profiles" in config:
for p_name, p_data in config["profiles"].items():
p_allowed = p_data.get("allowed_operations") or []
p_forbidden = p_data.get("forbidden_operations") or []
p_allowed_n = []
for op in p_allowed:
try:
p_allowed_n.append(gitea_config.normalize_operation(op))
except Exception:
pass
p_forbidden_n = []
for op in p_forbidden:
try:
p_forbidden_n.append(gitea_config.normalize_operation(op))
except Exception:
pass
# Check for "review_pr" or similar capability
ok, _ = gitea_config.check_operation("gitea.pr.review", p_allowed_n, p_forbidden_n)
if ok:
matching_profiles.append(p_name)
runtime_reasons = _check_mcp_runtimes_diagnostics("review_pr", matching_profiles)
if runtime_reasons:
reasons.extend(runtime_reasons)
# Check reviewer identity
identity = _authenticated_username(h)
if identity != draft.get("saved_by_identity"):
reasons.append(
f"reviewer identity mismatch (active={identity!r}, saved={draft.get('saved_by_identity')!r})"
)
# Check worktree binding
try:
resolved_worktree = _resolve_preflight_workspace_path(worktree_path)
except Exception as exc:
reasons.append(f"failed to resolve current worktree: {str(exc)}")
resolved_worktree = None
if resolved_worktree != draft.get("worktree_path"):
reasons.append(
f"worktree binding mismatch (current={resolved_worktree!r}, saved={draft.get('worktree_path')!r})"
)
if reasons:
return {"success": False, "reasons": reasons}
# 5. All checks pass! Mark decision lock as ready.
action = draft.get("action")
body = draft.get("body")
saved_head = draft.get("head_sha")
lock = _load_review_decision_lock() or {}
lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = pr_number
lock["ready_action"] = action
lock["ready_expected_head_sha"] = saved_head
lock["ready_remote"] = remote
lock["ready_org"] = resolved_org
lock["ready_repo"] = resolved_repo
_save_review_decision_lock(lock)
submitted = False
submission_res = None
if submit:
# Submit the review!
submission_res = gitea_submit_pr_review(
pr_number=pr_number,
action=action,
body=body,
expected_head_sha=saved_head,
remote=remote,
host=host,
org=resolved_org,
repo=resolved_repo,
final_review_decision_ready=True,
worktree_path=resolved_worktree,
)
if submission_res.get("performed") or submission_res.get("success"):
submitted = True
# Delete the draft payload on success
mcp_session_state.save_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
payload=None,
remote=remote,
org=resolved_org,
repo=resolved_repo,
profile_identity=active_profile,
)
else:
# If submit fails, return the submit failure reasons
return {
"success": False,
"reasons": submission_res.get("reasons") or ["submission failed"],
"submission_result": submission_res,
}
return {
"success": True,
"pr_number": pr_number,
"action": action,
"marked_ready": True,
"submitted": submitted,
"submission_result": submission_res,
}
@mcp.tool()
def gitea_edit_pr(
pr_number: int,
+2
View File
@@ -30,6 +30,8 @@ DEFAULT_TTL_HOURS = 4.0
KIND_WORKFLOW_LOAD = "review_workflow_load"
KIND_DECISION_LOCK = "review_decision_lock"
KIND_REVIEW_DRAFT = "review_draft"
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
SESSION_PROFILE_LOCK_ENV = "GITEA_SESSION_PROFILE_LOCK"
+386
View File
@@ -0,0 +1,386 @@
"""Tests for preserving and resuming prepared Gitea review drafts (#609)."""
from __future__ import annotations
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch, MagicMock
sys_path_root = str(Path(__file__).resolve().parent.parent)
if sys_path_root not in sys.path:
sys.path.insert(0, sys_path_root)
import mcp_session_state
import gitea_mcp_server
import reviewer_pr_lease
class TestReviewDrafts(unittest.TestCase):
def setUp(self):
self._tmpdir = tempfile.TemporaryDirectory()
self.state_dir = self._tmpdir.name
self._env = patch.dict(
os.environ,
{
mcp_session_state.STATE_DIR_ENV: self.state_dir,
mcp_session_state.SESSION_PROFILE_LOCK_ENV: "prgs-reviewer",
"GITEA_MCP_PROFILE": "prgs-reviewer",
"GITEA_PROFILE_NAME": "prgs-reviewer",
"PYTEST_CURRENT_TEST": "1", # skip runtime stale checks
},
clear=False,
)
self._env.start()
# Mock workspace binding to pass in test context
self._nwb_patch = patch(
"gitea_mcp_server.nwb.assess_namespace_mutation_workspace",
return_value={"block": False, "mutation_workspace": "/tmp/test-worktree"},
)
self._nwb_patch.start()
# Mock authentication headers
self._auth_patch = patch(
"gitea_mcp_server.get_auth_header",
return_value="Bearer mock-token",
)
self._auth_patch.start()
self._username_patch = patch(
"gitea_mcp_server._authenticated_username",
return_value="sysadmin",
)
self._username_patch.start()
# Clear/Mock local session lease in-memory state
reviewer_pr_lease.clear_session_lease()
def tearDown(self):
self._username_patch.stop()
self._auth_patch.stop()
self._nwb_patch.stop()
self._env.stop()
self._tmpdir.cleanup()
reviewer_pr_lease.clear_session_lease()
@patch("gitea_mcp_server.api_request")
def test_save_draft_success(self, mock_api):
mock_api.return_value = {
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
validation_commands="pytest tests/",
validation_results="all pass",
blocker_reason="stale daemon",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertTrue(res.get("success"))
self.assertEqual(res.get("head_sha"), "headsha1234567890")
# Load draft and verify fields
draft = mcp_session_state.load_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
profile_identity="prgs-reviewer",
)
self.assertIsNotNone(draft)
self.assertEqual(draft.get("pr_number"), 587)
self.assertEqual(draft.get("action"), "approve")
self.assertEqual(draft.get("body"), "Good changes")
self.assertEqual(draft.get("validation_commands"), "pytest tests/")
self.assertEqual(draft.get("validation_results"), "all pass")
self.assertEqual(draft.get("blocker_reason"), "stale daemon")
self.assertEqual(os.path.realpath(draft.get("worktree_path")), os.path.realpath("/tmp/test-worktree"))
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._fetch_pr_comments")
@patch("gitea_mcp_server.gitea_submit_pr_review")
def test_resume_draft_and_submit_success(self, mock_submit, mock_comments, mock_api):
# 1. Save draft
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
# Seed local session lease
reviewer_pr_lease.record_session_lease({
"pr_number": 587,
"session_id": "session-1234",
})
# Mock Gitea comments to return active reviewer lease owned by us
mock_comments.return_value = [
{
"id": 1,
"user": {"username": "sysadmin"},
"body": (
"<!-- mcp-review-lease:v1 -->\n"
"repo: Scaled-Tech-Consulting/Gitea-Tools\n"
"pr: #587\n"
"reviewer_identity: sysadmin\n"
"profile: prgs-reviewer\n"
"session_id: session-1234\n"
"phase: claimed\n"
),
}
]
mock_submit.return_value = {"performed": True, "success": True}
# 2. Resume draft
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
submit=True,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertTrue(res.get("success"))
self.assertTrue(res.get("marked_ready"))
self.assertTrue(res.get("submitted"))
mock_submit.assert_called_once()
# Verify draft was deleted after successful submit
draft = mcp_session_state.load_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
profile_identity="prgs-reviewer",
)
self.assertIsNone(draft)
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._fetch_pr_comments")
def test_resume_draft_fails_checks(self, mock_comments, mock_api):
# Save a draft
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
# Seed local session lease
reviewer_pr_lease.record_session_lease({
"pr_number": 587,
"session_id": "session-1234",
})
# Mock Gitea comments to return active reviewer lease owned by us
mock_comments.return_value = [
{
"id": 1,
"user": {"username": "sysadmin"},
"body": (
"<!-- mcp-review-lease:v1 -->\n"
"repo: Scaled-Tech-Consulting/Gitea-Tools\n"
"pr: #587\n"
"reviewer_identity: sysadmin\n"
"profile: prgs-reviewer\n"
"session_id: session-1234\n"
"phase: claimed\n"
),
}
]
# Case A: PR closed
mock_api.return_value = {
"state": "closed",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("PR #587 is not open", res.get("reasons")[0])
# Case B: Head changed
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha_NEW"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("PR head SHA changed", res.get("reasons")[0])
# Case C: Target branch changed
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha_NEW"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("target branch SHA changed", res.get("reasons")[0])
# Case D: Worktree mismatch
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/different-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("worktree binding mismatch", res.get("reasons")[0])
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._fetch_pr_comments")
def test_resume_draft_foreign_lease(self, mock_comments, mock_api):
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
reviewer_pr_lease.record_session_lease({
"pr_number": 587,
"session_id": "my-session",
})
mock_comments.return_value = [
{
"id": 9,
"user": {"username": "sysadmin"},
"body": (
"<!-- mcp-review-lease:v1 -->\n"
"repo: Scaled-Tech-Consulting/Gitea-Tools\n"
"pr: #587\n"
"reviewer_identity: sysadmin\n"
"profile: prgs-reviewer\n"
"session_id: foreign-session-999\n"
"phase: claimed\n"
),
}
]
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertTrue(
any("foreign-session-999" in r or "session_id" in r for r in (res.get("reasons") or []))
)
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._fetch_pr_comments")
@patch("gitea_mcp_server.terminal_review_hard_stop_reasons")
def test_resume_draft_terminal_lock_blocks(self, mock_hard_stop, mock_comments, mock_api):
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
reviewer_pr_lease.record_session_lease({
"pr_number": 587,
"session_id": "session-1234",
})
mock_comments.return_value = [
{
"id": 1,
"user": {"username": "sysadmin"},
"body": (
"<!-- mcp-review-lease:v1 -->\n"
"repo: Scaled-Tech-Consulting/Gitea-Tools\n"
"pr: #587\n"
"reviewer_identity: sysadmin\n"
"profile: prgs-reviewer\n"
"session_id: session-1234\n"
"phase: claimed\n"
),
}
]
mock_hard_stop.return_value = [
"terminal review mutation already recorded for PR #587 (approve); #332 hard-stop"
]
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertTrue(any("#332" in r or "terminal" in r for r in (res.get("reasons") or [])))