fix: resolve conflicts for PR #392
Merge prgs/master into feat/issue-389-review-workflow-load-proof and preserve workflow-load gates while adopting master test harness patterns (_seed_ready_review_decision, reviewer leases, expected_head_sha).
This commit is contained in:
@@ -74,21 +74,24 @@ ISSUE_WRITE_ENV = {
|
||||
|
||||
class TestIssueLockArtifactWarning(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True)
|
||||
self._lock_dir = tempfile.TemporaryDirectory()
|
||||
env = {**ISSUE_WRITE_ENV, "GITEA_ISSUE_LOCK_DIR": self._lock_dir.name}
|
||||
self._env_patcher = patch.dict(os.environ, env, clear=True)
|
||||
self._env_patcher.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._env_patcher.stop()
|
||||
self._lock_dir.cleanup()
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server._auth", return_value="token x")
|
||||
@patch("mcp_server._resolve", return_value=("h", "o", "r"))
|
||||
@patch("mcp_server.ISSUE_LOCK_FILE", new_callable=lambda: tempfile.mktemp())
|
||||
@patch("issue_lock_worktree.read_worktree_git_state")
|
||||
def test_lock_success_includes_artifact_warning(self, mock_state, _lock_file, *_mocks):
|
||||
def test_lock_success_includes_artifact_warning(self, mock_state, *_mocks):
|
||||
mock_state.return_value = {
|
||||
"current_branch": "master",
|
||||
"porcelain_status": "?? _emit_payload.py\n",
|
||||
|
||||
+30
-19
@@ -285,21 +285,37 @@ class TestSimpleToolAudit(_AuditWiringBase):
|
||||
self.assertEqual(result["number"], 9)
|
||||
|
||||
|
||||
_NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True}
|
||||
|
||||
|
||||
class TestGatedToolAudit(_AuditWiringBase):
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
from tests.test_mcp_server import _install_owned_reviewer_lease
|
||||
from tests.test_mcp_server import _init_reviewer_session, _install_owned_reviewer_lease
|
||||
import reviewer_pr_lease
|
||||
|
||||
# Session init clears any prior session lease (#407) and loads workflow (#389).
|
||||
_init_reviewer_session("prgs")
|
||||
self._lease_patch = _install_owned_reviewer_lease(8)
|
||||
self._lease_patch.start()
|
||||
self._auth_identity_patch = patch(
|
||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
||||
)
|
||||
self._auth_identity_patch.start()
|
||||
self._pr_lease_comments_patch = patch(
|
||||
"mcp_server._list_pr_lease_comments", return_value=[]
|
||||
)
|
||||
self._pr_lease_comments_patch.start()
|
||||
self._pr_work_lease_patch = patch(
|
||||
"mcp_server._pr_work_lease_reviewer_block",
|
||||
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
|
||||
)
|
||||
self._pr_work_lease_patch.start()
|
||||
self.addCleanup(self._auth_identity_patch.stop)
|
||||
self.addCleanup(self._lease_patch.stop)
|
||||
self.addCleanup(self._pr_lease_comments_patch.stop)
|
||||
self.addCleanup(self._pr_work_lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
|
||||
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
||||
@@ -314,7 +330,8 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
{"login": "merger-bot"}, self._pr("author-bot"),
|
||||
self._pr("author-bot"),
|
||||
[{"id": 1, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
|
||||
"submitted_at": "2026-07-06T10:00:00Z", "dismissed": False}],
|
||||
"commit_id": "abc123", "submitted_at": "2026-07-06T10:00:00Z",
|
||||
"dismissed": False}],
|
||||
{}, {"merged_commit_sha": "c1"},
|
||||
]
|
||||
env = self._env(GITEA_PROFILE_NAME="gitea-merger",
|
||||
@@ -352,7 +369,9 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_submit_review_success_audited(self, _auth, mock_api):
|
||||
# mark_final_review_decision and submit each run eligibility (user + PR).
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 7, "state": "APPROVED"},
|
||||
[{"id": 7, "user": {"login": "reviewer-bot"}, "state": "APPROVED",
|
||||
@@ -361,24 +380,16 @@ class TestGatedToolAudit(_AuditWiringBase):
|
||||
env = self._env(GITEA_PROFILE_NAME="gitea-reviewer",
|
||||
GITEA_ALLOWED_OPERATIONS="read,review,approve")
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||
from tests.test_mcp_server import _install_owned_reviewer_lease
|
||||
import reviewer_pr_lease
|
||||
from mcp_server import gitea_mark_final_review_decision
|
||||
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
mcp_server.gitea_load_review_workflow()
|
||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
||||
lease_patch = _install_owned_reviewer_lease(8)
|
||||
lease_patch.start()
|
||||
try:
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve",
|
||||
body="LGTM", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
finally:
|
||||
lease_patch.stop()
|
||||
reviewer_pr_lease.clear_session_lease()
|
||||
gitea_mark_final_review_decision(
|
||||
8, "approve", expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve",
|
||||
body="LGTM", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
recs = self._records()
|
||||
self.assertEqual(len(recs), 1)
|
||||
|
||||
@@ -66,9 +66,12 @@ class TestCommitPayloads(unittest.TestCase):
|
||||
)
|
||||
self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name)
|
||||
|
||||
self.lock_file_path = "/tmp/gitea_issue_lock.json"
|
||||
import issue_lock_store
|
||||
import issue_lock_provenance
|
||||
|
||||
self._lock_dir = tempfile.TemporaryDirectory()
|
||||
os.environ["GITEA_ISSUE_LOCK_DIR"] = self._lock_dir.name
|
||||
|
||||
work_lease = {
|
||||
"operation_type": "author_issue_work",
|
||||
"issue_number": 263,
|
||||
@@ -89,8 +92,7 @@ class TestCommitPayloads(unittest.TestCase):
|
||||
claimant=work_lease.get("claimant"),
|
||||
),
|
||||
}
|
||||
with open(self.lock_file_path, "w", encoding="utf-8") as fh:
|
||||
fh.write(json.dumps(self.lock_data))
|
||||
self.lock_file_path = issue_lock_store.bind_session_lock(self.lock_data)
|
||||
|
||||
# Reset preflight status to bypass/pass verification in tests
|
||||
self.orig_whoami_called = mcp_server._preflight_whoami_called
|
||||
@@ -114,8 +116,7 @@ class TestCommitPayloads(unittest.TestCase):
|
||||
|
||||
self._dir.cleanup()
|
||||
self.locked_worktree_dir.cleanup()
|
||||
if os.path.exists(self.lock_file_path):
|
||||
os.remove(self.lock_file_path)
|
||||
self._lock_dir.cleanup()
|
||||
|
||||
def _env(self, profile: str) -> dict:
|
||||
return {
|
||||
@@ -124,6 +125,7 @@ class TestCommitPayloads(unittest.TestCase):
|
||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||
"GITEA_TEST_PORCELAIN": "",
|
||||
"GITEA_AUTHOR_WORKTREE": self.locked_worktree_path,
|
||||
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
|
||||
}
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
|
||||
@@ -106,20 +106,32 @@ class TestCreateIssueWorkspaceGuard(unittest.TestCase):
|
||||
@patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"})
|
||||
@patch("os.path.exists", return_value=True)
|
||||
@patch("os.path.isdir", return_value=True)
|
||||
@patch("author_mutation_worktree.subprocess.run")
|
||||
@patch("subprocess.run")
|
||||
def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth):
|
||||
# Mock subprocess.run for git --git-common-dir to return a different path
|
||||
mock_res = MagicMock()
|
||||
mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n"
|
||||
mock_run.return_value = mock_res
|
||||
|
||||
def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_amw_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth):
|
||||
wrong_repo_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1")
|
||||
|
||||
def _subprocess_side_effect(cmd, *args, **kwargs):
|
||||
mock_res = MagicMock(returncode=0)
|
||||
if "--git-common-dir" in cmd:
|
||||
cwd = cmd[cmd.index("-C") + 1] if "-C" in cmd else ""
|
||||
if cwd == wrong_repo_path:
|
||||
mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n"
|
||||
else:
|
||||
mock_res.stdout = f"{CONTROL_CHECKOUT_ROOT}/.git\n"
|
||||
else:
|
||||
mock_res.stdout = ""
|
||||
return mock_res
|
||||
|
||||
mock_run.side_effect = _subprocess_side_effect
|
||||
mock_amw_run.side_effect = _subprocess_side_effect
|
||||
|
||||
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
srv.gitea_create_issue(
|
||||
title="Test issue", body="body", worktree_path=wrong_repo_path
|
||||
)
|
||||
with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
srv.gitea_create_issue(
|
||||
title="Test issue", body="body", worktree_path=wrong_repo_path
|
||||
)
|
||||
self.assertIn("does not belong to the target repository", str(ctx.exception))
|
||||
|
||||
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
|
||||
|
||||
@@ -41,6 +41,10 @@ def _review_handoff(**overrides):
|
||||
"- Selected PR: #203",
|
||||
"- Reviewer eligibility: eligible",
|
||||
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Push occurred during validation: no",
|
||||
"- Worktree path: branches/review-203",
|
||||
"- Worktree dirty: clean",
|
||||
"- Scratch worktree used: yes (branches/review-203)",
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
"""Unit tests for own-branch lock adoption decision (#442 / #443)."""
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from issue_lock_adoption import ( # noqa: E402
|
||||
ADOPT,
|
||||
BLOCK_COMPETING,
|
||||
NO_MATCH,
|
||||
assess_own_branch_adoption,
|
||||
build_adoption_proof,
|
||||
)
|
||||
|
||||
REQ = "feat/issue-420-server-code-parity"
|
||||
|
||||
|
||||
class TestAssessOwnBranchAdoption(unittest.TestCase):
|
||||
def test_exact_own_branch_is_adopted(self):
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=420,
|
||||
requested_branch=REQ,
|
||||
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
|
||||
)
|
||||
self.assertEqual(result["outcome"], ADOPT)
|
||||
self.assertTrue(result["adopt"])
|
||||
self.assertFalse(result["block"])
|
||||
self.assertEqual(result["matched_branch"], REQ)
|
||||
self.assertEqual(result["matched_head_sha"], "934688a")
|
||||
|
||||
def test_exact_own_branch_adopted_when_sha_missing(self):
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=420, requested_branch=REQ, existing_branches=[REQ]
|
||||
)
|
||||
self.assertEqual(result["outcome"], ADOPT)
|
||||
self.assertIsNone(result["matched_head_sha"])
|
||||
|
||||
def test_different_branch_same_issue_blocks(self):
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=420,
|
||||
requested_branch=REQ,
|
||||
existing_branches=[{"name": "feat/issue-420-other-work"}],
|
||||
)
|
||||
self.assertEqual(result["outcome"], BLOCK_COMPETING)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertFalse(result["adopt"])
|
||||
self.assertIn("feat/issue-420-other-work", result["competing_branches"])
|
||||
self.assertIn("fail closed", result["reason"])
|
||||
|
||||
def test_own_branch_plus_competing_branch_blocks(self):
|
||||
# Ambiguous ownership: fail closed even though the exact branch exists.
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=420,
|
||||
requested_branch=REQ,
|
||||
existing_branches=[{"name": REQ}, {"name": "feat/issue-420-rogue"}],
|
||||
)
|
||||
self.assertEqual(result["outcome"], BLOCK_COMPETING)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertEqual(result["competing_branches"], ["feat/issue-420-rogue"])
|
||||
|
||||
def test_no_matching_branch_is_normal_path(self):
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=420,
|
||||
requested_branch=REQ,
|
||||
existing_branches=[{"name": "feat/issue-999-unrelated"}],
|
||||
)
|
||||
self.assertEqual(result["outcome"], NO_MATCH)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertFalse(result["adopt"])
|
||||
|
||||
def test_empty_branch_list_is_normal_path(self):
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=420, requested_branch=REQ, existing_branches=[]
|
||||
)
|
||||
self.assertEqual(result["outcome"], NO_MATCH)
|
||||
|
||||
def test_higher_issue_number_branch_does_not_block_lower_issue_adoption(self):
|
||||
# issue-420 must not be treated as competing work for issue #42.
|
||||
own_branch = "feat/issue-42-widget"
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=42,
|
||||
requested_branch=own_branch,
|
||||
existing_branches=[
|
||||
{"name": own_branch, "commit_sha": "abc1234"},
|
||||
{"name": "feat/issue-420-server-code-parity"},
|
||||
],
|
||||
)
|
||||
self.assertEqual(result["outcome"], ADOPT)
|
||||
self.assertTrue(result["adopt"])
|
||||
self.assertFalse(result["block"])
|
||||
self.assertEqual(result["matched_branch"], own_branch)
|
||||
|
||||
def test_unrelated_higher_number_branch_is_ignored_without_own_branch(self):
|
||||
result = assess_own_branch_adoption(
|
||||
issue_number=42,
|
||||
requested_branch="feat/issue-42-thing",
|
||||
existing_branches=[{"name": "feat/issue-420-server-code-parity"}],
|
||||
)
|
||||
self.assertEqual(result["outcome"], NO_MATCH)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertFalse(result["adopt"])
|
||||
|
||||
|
||||
class TestBuildAdoptionProof(unittest.TestCase):
|
||||
def test_proof_has_all_required_fields(self):
|
||||
assessment = assess_own_branch_adoption(
|
||||
issue_number=420,
|
||||
requested_branch=REQ,
|
||||
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
|
||||
)
|
||||
proof = build_adoption_proof(
|
||||
issue_number=420,
|
||||
branch_name=REQ,
|
||||
assessment=assessment,
|
||||
open_pr_checked=True,
|
||||
competing_lock_checked=True,
|
||||
lock_file_path="/tmp/example-lock.json",
|
||||
lock_file_status="written",
|
||||
)
|
||||
for key in (
|
||||
"issue_number",
|
||||
"branch_name",
|
||||
"branch_head_commit",
|
||||
"adoption_reason",
|
||||
"no_existing_pr_proof",
|
||||
"no_competing_live_lock_proof",
|
||||
"lock_file_path",
|
||||
"lock_file_status",
|
||||
):
|
||||
self.assertIn(key, proof)
|
||||
self.assertEqual(proof["branch_head_commit"], "934688a")
|
||||
self.assertTrue(proof["no_existing_pr_proof"])
|
||||
self.assertTrue(proof["no_competing_live_lock_proof"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,256 @@
|
||||
"""Unit tests for keyed issue-lock storage (#443) and flock hardening (#438)."""
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import threading
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from pathlib import Path
|
||||
from unittest import mock
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
import issue_lock_store as ils # noqa: E402
|
||||
|
||||
|
||||
def _lease(expires_at: str) -> dict:
|
||||
return {
|
||||
"operation_type": ils.AUTHOR_ISSUE_WORK_LEASE,
|
||||
"expires_at": expires_at,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"last_heartbeat_at": "2026-01-01T00:00:00Z",
|
||||
}
|
||||
|
||||
|
||||
def _lock_record(**overrides) -> dict:
|
||||
record = {
|
||||
"issue_number": 420,
|
||||
"branch_name": "feat/issue-420-server-code-parity",
|
||||
"remote": "prgs",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
"worktree_path": "/tmp/wt-420",
|
||||
"work_lease": _lease("2999-01-01T00:00:00Z"),
|
||||
}
|
||||
record.update(overrides)
|
||||
return record
|
||||
|
||||
|
||||
class TestIssueLockStore(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._dir = tempfile.TemporaryDirectory()
|
||||
self.lock_dir = self._dir.name
|
||||
self._env = mock.patch.dict(os.environ, {"GITEA_ISSUE_LOCK_DIR": self.lock_dir})
|
||||
self._env.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._env.stop()
|
||||
self._dir.cleanup()
|
||||
|
||||
def test_concurrent_repo_locks_do_not_overwrite(self):
|
||||
lock_a = _lock_record(
|
||||
issue_number=108,
|
||||
branch_name="feat/issue-108-root-menu",
|
||||
repo="mcp-control-plane",
|
||||
worktree_path="/tmp/wt-108",
|
||||
)
|
||||
lock_b = _lock_record(
|
||||
issue_number=420,
|
||||
branch_name="feat/issue-420-server-code-parity",
|
||||
repo="Gitea-Tools",
|
||||
worktree_path="/tmp/wt-420",
|
||||
)
|
||||
path_a = ils.bind_session_lock(lock_a)
|
||||
with mock.patch("os.getpid", return_value=9999):
|
||||
path_b = ils.bind_session_lock(lock_b)
|
||||
|
||||
self.assertNotEqual(path_a, path_b)
|
||||
self.assertTrue(os.path.exists(path_a))
|
||||
self.assertTrue(os.path.exists(path_b))
|
||||
stored_a = ils.read_lock_file(path_a)
|
||||
stored_b = ils.read_lock_file(path_b)
|
||||
self.assertEqual(stored_a["issue_number"], 108)
|
||||
self.assertEqual(stored_b["issue_number"], 420)
|
||||
|
||||
def test_concurrent_issue_locks_same_repo_do_not_overwrite(self):
|
||||
lock_a = _lock_record(issue_number=427, branch_name="feat/issue-427-a")
|
||||
lock_b = _lock_record(issue_number=428, branch_name="feat/issue-428-b")
|
||||
path_a = ils.bind_session_lock(lock_a)
|
||||
with mock.patch("os.getpid", return_value=4242):
|
||||
path_b = ils.bind_session_lock(lock_b)
|
||||
|
||||
self.assertNotEqual(path_a, path_b)
|
||||
self.assertEqual(ils.read_lock_file(path_a)["issue_number"], 427)
|
||||
self.assertEqual(ils.read_lock_file(path_b)["issue_number"], 428)
|
||||
|
||||
def test_foreign_live_lease_blocks_overwrite(self):
|
||||
existing = _lock_record(
|
||||
branch_name="feat/issue-420-other",
|
||||
worktree_path="/tmp/other",
|
||||
work_lease=_lease("2999-01-01T00:00:00Z"),
|
||||
)
|
||||
path = ils.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=420,
|
||||
)
|
||||
ils.save_lock_file(path, existing)
|
||||
|
||||
incoming = _lock_record(worktree_path="/tmp/mine")
|
||||
block = ils.assess_foreign_lock_overwrite(existing, incoming)
|
||||
self.assertIn("live foreign issue lock", block or "")
|
||||
|
||||
def test_expired_lease_allows_takeover_with_conflict_check(self):
|
||||
existing = _lock_record(
|
||||
branch_name="feat/issue-420-other",
|
||||
worktree_path="/tmp/other",
|
||||
work_lease=_lease("2000-01-01T00:00:00Z"),
|
||||
)
|
||||
incoming = _lock_record(worktree_path="/tmp/mine")
|
||||
self.assertIsNone(ils.assess_foreign_lock_overwrite(existing, incoming))
|
||||
block = ils.assess_same_issue_lease_conflict(
|
||||
existing,
|
||||
issue_number=420,
|
||||
branch_name="feat/issue-420-server-code-parity",
|
||||
worktree_path="/tmp/mine",
|
||||
)
|
||||
self.assertIn("Recovery review is required", block or "")
|
||||
|
||||
def test_same_owner_lease_conflict_allows_refresh(self):
|
||||
worktree = "/tmp/wt-420"
|
||||
existing = _lock_record(worktree_path=worktree)
|
||||
block = ils.assess_same_issue_lease_conflict(
|
||||
existing,
|
||||
issue_number=420,
|
||||
branch_name="feat/issue-420-server-code-parity",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIsNone(block)
|
||||
|
||||
def test_find_lock_for_branch_after_restart(self):
|
||||
record = _lock_record()
|
||||
path = ils.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=420,
|
||||
)
|
||||
ils.save_lock_file(path, record)
|
||||
|
||||
with mock.patch("os.getpid", return_value=5555):
|
||||
self.assertIsNone(ils.read_session_issue_lock())
|
||||
|
||||
found = ils.find_lock_for_branch(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
branch_name="feat/issue-420-server-code-parity",
|
||||
)
|
||||
self.assertEqual(found["issue_number"], 420)
|
||||
|
||||
def test_has_active_issue_lock_scans_keyed_store(self):
|
||||
ils.bind_session_lock(_lock_record())
|
||||
self.assertTrue(
|
||||
ils.has_active_issue_lock("feat/issue-420-server-code-parity")
|
||||
)
|
||||
self.assertFalse(ils.has_active_issue_lock("feat/issue-999-other"))
|
||||
|
||||
def test_atomic_write_preserves_unrelated_lock(self):
|
||||
path_a = ils.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=108,
|
||||
)
|
||||
ils.save_lock_file(path_a, _lock_record(issue_number=108, repo="mcp-control-plane"))
|
||||
path_b = ils.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=420,
|
||||
)
|
||||
ils.save_lock_file(path_b, _lock_record())
|
||||
|
||||
self.assertTrue(os.path.exists(path_a))
|
||||
self.assertTrue(os.path.exists(path_b))
|
||||
self.assertEqual(ils.read_lock_file(path_a)["issue_number"], 108)
|
||||
|
||||
def test_concurrent_bind_same_issue_only_one_wins(self):
|
||||
barrier = threading.Barrier(2)
|
||||
results: list[str | Exception] = []
|
||||
|
||||
def worker():
|
||||
barrier.wait()
|
||||
try:
|
||||
ils.bind_session_lock(
|
||||
_lock_record(worktree_path=f"/tmp/wt-{threading.get_ident()}")
|
||||
)
|
||||
results.append("ok")
|
||||
except Exception as exc: # noqa: BLE001
|
||||
results.append(exc)
|
||||
|
||||
threads = [threading.Thread(target=worker) for _ in range(2)]
|
||||
for thread in threads:
|
||||
thread.start()
|
||||
for thread in threads:
|
||||
thread.join()
|
||||
|
||||
successes = [item for item in results if item == "ok"]
|
||||
failures = [item for item in results if isinstance(item, Exception)]
|
||||
self.assertEqual(len(successes), 1)
|
||||
self.assertEqual(len(failures), 1)
|
||||
failure_text = str(failures[0]).lower()
|
||||
self.assertTrue(
|
||||
"active" in failure_text or "lock contention" in failure_text,
|
||||
failures[0],
|
||||
)
|
||||
|
||||
def test_verify_lock_for_mutation_blocks_stale_lock(self):
|
||||
record = _lock_record(
|
||||
work_lease=_lease("2000-01-01T00:00:00Z"),
|
||||
)
|
||||
record["pid"] = 999999
|
||||
record["session_pid"] = 999999
|
||||
result = ils.verify_lock_for_mutation(
|
||||
record,
|
||||
issue_number=420,
|
||||
branch_name="feat/issue-420-server-code-parity",
|
||||
worktree_path="/tmp/wt-420",
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertIn("not live", result["reasons"][0])
|
||||
|
||||
def test_list_live_locks_excludes_stale_records(self):
|
||||
live_path = ils.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=420,
|
||||
)
|
||||
ils.save_lock_file(
|
||||
live_path,
|
||||
_lock_record(worktree_path="/tmp/wt-420"),
|
||||
)
|
||||
stale_path = ils.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=440,
|
||||
)
|
||||
ils.save_lock_file(
|
||||
stale_path,
|
||||
_lock_record(
|
||||
issue_number=440,
|
||||
branch_name="feat/issue-440-recovery",
|
||||
work_lease=_lease("2000-01-01T00:00:00Z"),
|
||||
worktree_path="/tmp/wt-440",
|
||||
),
|
||||
)
|
||||
live = ils.list_live_locks(lock_dir=self.lock_dir)
|
||||
self.assertEqual([entry["issue_number"] for entry in live], [420])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -1,5 +1,4 @@
|
||||
"""Tests for early duplicate-work detection (#400)."""
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
@@ -10,6 +9,7 @@ from unittest.mock import patch
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
import issue_lock_provenance
|
||||
import issue_lock_store
|
||||
import issue_work_duplicate_gate as dup_gate
|
||||
import mcp_server
|
||||
from issue_work_duplicate_gate import (
|
||||
@@ -123,8 +123,9 @@ class TestDuplicateReportOutcome(unittest.TestCase):
|
||||
|
||||
|
||||
class TestInjectableDuplicateFetcher(unittest.TestCase):
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value="token x")
|
||||
def test_lock_issue_uses_injected_fetcher(self, _auth):
|
||||
def test_lock_issue_uses_injected_fetcher(self, _auth, _api):
|
||||
seen = {}
|
||||
|
||||
def fetcher(h, o, r, auth, issue_number):
|
||||
@@ -141,26 +142,29 @@ class TestInjectableDuplicateFetcher(unittest.TestCase):
|
||||
"porcelain_status": "",
|
||||
"base_equivalent": True,
|
||||
},
|
||||
), patch.dict(os.environ, {
|
||||
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment",
|
||||
}, clear=True):
|
||||
with patch.object(mcp_server, "ISSUE_LOCK_FILE", tempfile.mktemp()):
|
||||
mcp_server.gitea_lock_issue(
|
||||
issue_number=400,
|
||||
branch_name="feat/issue-400-duplicate-work-preflight",
|
||||
remote="prgs",
|
||||
)
|
||||
):
|
||||
with tempfile.TemporaryDirectory() as lock_dir:
|
||||
with patch.dict(os.environ, {
|
||||
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment",
|
||||
"GITEA_ISSUE_LOCK_DIR": lock_dir,
|
||||
}, clear=True):
|
||||
mcp_server.gitea_lock_issue(
|
||||
issue_number=400,
|
||||
branch_name="feat/issue-400-duplicate-work-preflight",
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertEqual(seen["issue_number"], 400)
|
||||
|
||||
|
||||
class TestMcpDuplicateRecheck(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._dir = tempfile.TemporaryDirectory()
|
||||
self.lock_path = os.path.join(self._dir.name, "gitea_issue_lock.json")
|
||||
self._lock_patch = patch.object(
|
||||
mcp_server, "ISSUE_LOCK_FILE", self.lock_path
|
||||
self._env_patch = patch.dict(
|
||||
os.environ,
|
||||
{"GITEA_ISSUE_LOCK_DIR": self._dir.name},
|
||||
clear=False,
|
||||
)
|
||||
self._lock_patch.start()
|
||||
self._env_patch.start()
|
||||
self._remotes = patch.dict(mcp_server.REMOTES, {
|
||||
"prgs": {"host": "gitea.example.com", "org": "Example-Org",
|
||||
"repo": "Example-Repo"},
|
||||
@@ -173,6 +177,7 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
|
||||
self._dir.cleanup()
|
||||
|
||||
def _write_lock(self, issue_number=400, branch="feat/issue-400-x"):
|
||||
worktree_path = os.path.realpath(os.getcwd())
|
||||
work_lease = {
|
||||
"operation_type": "author_issue_work",
|
||||
"issue_number": issue_number,
|
||||
@@ -180,17 +185,19 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
|
||||
"claimant": {"username": "test-user", "profile": "test-author"},
|
||||
"expires_at": "2999-01-01T00:00:00Z",
|
||||
}
|
||||
with open(self.lock_path, "w", encoding="utf-8") as fh:
|
||||
json.dump({
|
||||
"issue_number": issue_number,
|
||||
"branch_name": branch,
|
||||
"remote": "prgs",
|
||||
"work_lease": work_lease,
|
||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
||||
tool="gitea_lock_issue",
|
||||
claimant=work_lease.get("claimant"),
|
||||
),
|
||||
}, fh)
|
||||
issue_lock_store.bind_session_lock({
|
||||
"issue_number": issue_number,
|
||||
"branch_name": branch,
|
||||
"remote": "prgs",
|
||||
"org": "Example-Org",
|
||||
"repo": "Example-Repo",
|
||||
"worktree_path": worktree_path,
|
||||
"work_lease": work_lease,
|
||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
||||
tool="gitea_lock_issue",
|
||||
claimant=work_lease.get("claimant"),
|
||||
),
|
||||
})
|
||||
|
||||
@patch("mcp_server._assess_issue_duplicate_gate")
|
||||
@patch("mcp_server.get_profile", return_value={
|
||||
@@ -254,6 +261,7 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
|
||||
base="master",
|
||||
body="Closes #400",
|
||||
remote="prgs",
|
||||
worktree_path=os.path.realpath(os.getcwd()),
|
||||
)
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIsNone(result.get("number"))
|
||||
|
||||
@@ -123,16 +123,19 @@ class TestShaCannotBypassSelfReview(unittest.TestCase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_review_tool_refuses_self_approval_despite_sha(self, _auth, mock_api, mock_get_all):
|
||||
mock_get_all.return_value = [{"number": 9, "title": "PR 9", "state": "open", "head": {"ref": "branch9", "sha": "abc1234"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}]
|
||||
from mcp_server import init_review_decision_lock
|
||||
from tests.test_mcp_server import FULL_HEAD_SHA, _seed_ready_review_decision
|
||||
|
||||
head_sha = FULL_HEAD_SHA
|
||||
mock_get_all.return_value = [{"number": 9, "title": "PR 9", "state": "open", "head": {"ref": "branch9", "sha": head_sha}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}]
|
||||
mock_api.side_effect = [
|
||||
{"login": "jcwalker3"}, # /user (inventory)
|
||||
{"login": "jcwalker3"}, # /user (submit eligibility)
|
||||
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": "abc1234"}, "mergeable": True}, # /pulls/9
|
||||
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": head_sha}, "mergeable": True}, # /pulls/9
|
||||
]
|
||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
mcp_server.gitea_load_review_workflow()
|
||||
gitea_mark_final_review_decision(9, "approve", remote="prgs")
|
||||
with patch("mcp_server._list_pr_lease_comments", return_value=[]):
|
||||
_seed_ready_review_decision(9, "approve", sha=head_sha, remote="prgs")
|
||||
env = self._env(SHA_WOULD_BE_REVIEWER, "reviewer")
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_review_pr(
|
||||
|
||||
+398
-135
@@ -6,6 +6,7 @@ the MCP protocol) with mocked API responses.
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
@@ -45,8 +46,10 @@ from gitea_auth import get_profile # noqa: E402
|
||||
import gitea_config # noqa: E402
|
||||
|
||||
import mcp_server
|
||||
import issue_lock_store
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
FULL_HEAD_SHA = "a" * 40
|
||||
|
||||
_NO_BLOCKER_FEEDBACK = {
|
||||
"success": True,
|
||||
@@ -66,6 +69,7 @@ def _mark_request_changes_ready(pr_number=8, **kwargs):
|
||||
suppression feedback fetch stubbed to 'no existing blocker'."""
|
||||
with patch("mcp_server.gitea_get_pr_review_feedback",
|
||||
return_value=dict(_NO_BLOCKER_FEEDBACK)):
|
||||
kwargs.setdefault("expected_head_sha", "abc123")
|
||||
return gitea_mark_final_review_decision(
|
||||
pr_number, "request_changes", **kwargs)
|
||||
|
||||
@@ -86,6 +90,7 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
|
||||
|
||||
|
||||
_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
|
||||
_NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True}
|
||||
|
||||
|
||||
def _reviewer_lease_comment(
|
||||
@@ -143,6 +148,46 @@ def _install_owned_reviewer_lease(
|
||||
)
|
||||
|
||||
|
||||
def _seed_ready_review_decision(
|
||||
pr_number,
|
||||
action,
|
||||
*,
|
||||
sha=FULL_HEAD_SHA,
|
||||
remote="prgs",
|
||||
org=None,
|
||||
repo=None,
|
||||
):
|
||||
"""Mark the review-decision lock ready without mark_final API calls (#399)."""
|
||||
import mcp_server as _m
|
||||
|
||||
resolved_org, resolved_repo = org, repo
|
||||
if remote in _m.REMOTES:
|
||||
_, resolved_org, resolved_repo = _m._resolve(remote, None, org, repo)
|
||||
profile_name = (_m.get_profile().get("profile_name") or "").strip()
|
||||
session_lock = (
|
||||
(os.environ.get(_m.SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||
or profile_name
|
||||
)
|
||||
_m._save_review_decision_lock({
|
||||
"task": "review_pr",
|
||||
"remote": remote,
|
||||
"session_pid": os.getpid(),
|
||||
"session_profile": profile_name,
|
||||
"session_profile_lock": session_lock,
|
||||
"final_review_decision_ready": True,
|
||||
"ready_pr_number": pr_number,
|
||||
"ready_action": action,
|
||||
"ready_expected_head_sha": sha,
|
||||
"ready_remote": remote,
|
||||
"ready_org": resolved_org,
|
||||
"ready_repo": resolved_repo,
|
||||
"live_mutations": [],
|
||||
"correction_authorized": False,
|
||||
"correction_reason": None,
|
||||
})
|
||||
_m.gitea_load_review_workflow()
|
||||
|
||||
|
||||
# Issue-write tools are profile-gated (#69).
|
||||
ISSUE_WRITE_ENV = {
|
||||
"GITEA_ALLOWED_OPERATIONS": (
|
||||
@@ -180,6 +225,7 @@ def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
|
||||
"remote": "dadeschools",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
"worktree_path": "/tmp/test-worktree",
|
||||
"work_lease": work_lease,
|
||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
||||
tool="gitea_lock_issue",
|
||||
@@ -195,6 +241,17 @@ def _clear_duplicate_context_fetcher(*_args, **_kwargs):
|
||||
return [], [], {"status": "not_claimed"}
|
||||
|
||||
|
||||
def _bind_test_lock(**overrides) -> str:
|
||||
remote = overrides.get("remote", "dadeschools")
|
||||
record = _sample_issue_lock(**overrides)
|
||||
if remote in mcp_server.REMOTES:
|
||||
profile = mcp_server.REMOTES[remote]
|
||||
record.setdefault("org", profile["org"])
|
||||
record.setdefault("repo", profile["repo"])
|
||||
record["remote"] = remote
|
||||
return issue_lock_store.bind_session_lock(record)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Create Issue
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -257,18 +314,21 @@ class TestCreatePR(unittest.TestCase):
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("os.path.exists", return_value=True)
|
||||
@patch("builtins.open")
|
||||
def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher):
|
||||
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||
def test_creates_pr(self, _auth, mock_api, _role, _dup_fetcher):
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
result = gitea_create_pr(title="feat: X Closes #123", head="feat/x", base="main")
|
||||
with tempfile.TemporaryDirectory() as lock_dir:
|
||||
env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
_bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
|
||||
result = gitea_create_pr(
|
||||
title="feat: X Closes #123",
|
||||
head="feat/x",
|
||||
base="main",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertEqual(result["number"], 3)
|
||||
self.assertNotIn("url", result)
|
||||
mock_exists.assert_called_with(ISSUE_LOCK_FILE)
|
||||
mock_open.assert_called_with(ISSUE_LOCK_FILE, "r", encoding="utf-8")
|
||||
payload = mock_api.call_args[0][3]
|
||||
self.assertEqual(payload["head"], "feat/x")
|
||||
self.assertEqual(payload["base"], "main")
|
||||
@@ -282,30 +342,42 @@ class TestCreatePR(unittest.TestCase):
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("os.path.exists", return_value=True)
|
||||
@patch("builtins.open")
|
||||
def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher):
|
||||
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||
def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role, _dup_fetcher):
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||
env = {**CREATE_PR_ENV, "GITEA_MCP_REVEAL_ENDPOINTS": "1"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
result = gitea_create_pr(title="feat: X Closes #123", head="feat/x", base="main")
|
||||
with tempfile.TemporaryDirectory() as lock_dir:
|
||||
env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir, "GITEA_MCP_REVEAL_ENDPOINTS": "1"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
_bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
|
||||
result = gitea_create_pr(
|
||||
title="feat: X Closes #123",
|
||||
head="feat/x",
|
||||
base="main",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIn("pulls/3", result["url"])
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("os.path.exists", return_value=True)
|
||||
@patch("builtins.open")
|
||||
def test_create_pr_locked_issue_mismatch_fails(self, mock_open, mock_exists, _auth, _role):
|
||||
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(title="feat: X Closes #999", head="feat/x", base="main")
|
||||
def test_create_pr_locked_issue_mismatch_fails(self, _auth, _role):
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
with tempfile.TemporaryDirectory() as lock_dir:
|
||||
env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
_bind_test_lock(
|
||||
issue_number=123,
|
||||
branch_name="feat/x",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(
|
||||
title="feat: X Closes #999",
|
||||
head="feat/x",
|
||||
base="main",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIn("Closes #123", str(ctx.exception))
|
||||
mock_open.assert_called_with(ISSUE_LOCK_FILE, "r", encoding="utf-8")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -616,6 +688,17 @@ class TestMergePR(unittest.TestCase):
|
||||
self.addCleanup(self._auth_identity_patch.stop)
|
||||
self.addCleanup(self._lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
self._pr_lease_comments_patch = patch(
|
||||
"mcp_server._list_pr_lease_comments", return_value=[]
|
||||
)
|
||||
self._pr_lease_comments_patch.start()
|
||||
self.addCleanup(self._pr_lease_comments_patch.stop)
|
||||
self._pr_work_lease_patch = patch(
|
||||
"mcp_server._pr_work_lease_reviewer_block",
|
||||
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
|
||||
)
|
||||
self._pr_work_lease_patch.start()
|
||||
self.addCleanup(self._pr_work_lease_patch.stop)
|
||||
|
||||
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
||||
return {
|
||||
@@ -689,7 +772,8 @@ class TestMergePR(unittest.TestCase):
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_changed_files=["b.py", "a.py"], remote="prgs")
|
||||
expected_changed_files=["b.py", "a.py"],
|
||||
expected_head_sha="abc123", remote="prgs")
|
||||
self.assertTrue(r["performed"])
|
||||
|
||||
# -- read-back / cleanup surfacing (#98) -----------------------------------
|
||||
@@ -707,8 +791,10 @@ class TestMergePR(unittest.TestCase):
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(pr_number=8, confirmation=self._confirm(8),
|
||||
remote="prgs")
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
# The merge itself is still reported performed/successful.
|
||||
self.assertTrue(r["performed"])
|
||||
self.assertEqual(r["merge_result"], "PR #8 merged via 'merge'.")
|
||||
@@ -736,8 +822,10 @@ class TestMergePR(unittest.TestCase):
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(pr_number=8, confirmation=self._confirm(8),
|
||||
remote="prgs")
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
self.assertTrue(r["performed"])
|
||||
self.assertEqual(r["merge_commit"], "c9")
|
||||
self.assertTrue(r["cleanup_status"].startswith("skipped (cleanup error:"))
|
||||
@@ -751,7 +839,7 @@ class TestMergePR(unittest.TestCase):
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(pr_number=8, confirmation="", remote="prgs")
|
||||
r = gitea_merge_pr(pr_number=8, confirmation="", expected_head_sha="abc123", remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertTrue(any("explicit confirmation required" in x for x in r["reasons"]))
|
||||
mock_api.assert_not_called()
|
||||
@@ -762,7 +850,7 @@ class TestMergePR(unittest.TestCase):
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", remote="prgs")
|
||||
r = gitea_merge_pr(pr_number=8, confirmation="MERGE PR 9", expected_head_sha="abc123", remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
mock_api.assert_not_called()
|
||||
|
||||
@@ -896,7 +984,8 @@ class TestMergePR(unittest.TestCase):
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_changed_files=["a.py", "b.py"], remote="prgs")
|
||||
expected_changed_files=["a.py", "b.py"],
|
||||
expected_head_sha="abc123", remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertIn(
|
||||
"PR changed files do not match expected_changed_files (fail closed)",
|
||||
@@ -909,7 +998,7 @@ class TestMergePR(unittest.TestCase):
|
||||
def test_invalid_merge_method_rejected(self, mock_api):
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation="MERGE PR 8", do="octopus", remote="prgs")
|
||||
pr_number=8, confirmation="MERGE PR 8", do="octopus", expected_head_sha="abc123", remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertTrue(any("unknown merge method" in x for x in r["reasons"]))
|
||||
mock_api.assert_not_called()
|
||||
@@ -927,7 +1016,9 @@ class TestMergePR(unittest.TestCase):
|
||||
"GITEA_TOKEN": "super-secret-token"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
blob = repr(r).lower()
|
||||
for secret in ("super-secret-token", "authorization", "basic ", FAKE_AUTH.lower()):
|
||||
self.assertNotIn(secret, blob)
|
||||
@@ -944,12 +1035,37 @@ class TestMergePR(unittest.TestCase):
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
blob = repr(r)
|
||||
self.assertIn("[REDACTED]", blob)
|
||||
self.assertNotIn("abc-secret-xyz", blob)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_blocked_on_stale_approval_head(self, _auth, mock_api):
|
||||
old_sha = "8b61c4b41f1b49b271ed3b99657431cf06eeda3e"
|
||||
new_sha = "3e4b721d60e97147ba0704773cf57cd0d42cbe31"
|
||||
mock_api.side_effect = [
|
||||
{"login": "merger-bot"}, self._pr("author-bot", sha=new_sha),
|
||||
self._pr("author-bot", sha=new_sha),
|
||||
[_formal_review("reviewer-bot", "APPROVED", sha=old_sha)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-merger",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertTrue(r.get("approval_visible"))
|
||||
self.assertFalse(r.get("approval_at_current_head"))
|
||||
self.assertTrue(any("stale approval" in x for x in r["reasons"]))
|
||||
self.assertTrue(any(old_sha in x for x in r["reasons"]))
|
||||
self.assertTrue(any(new_sha in x for x in r["reasons"]))
|
||||
self._assert_no_merge_call(mock_api)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_merge_blocked_without_visible_approval(self, _auth, mock_api):
|
||||
@@ -962,7 +1078,9 @@ class TestMergePR(unittest.TestCase):
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertFalse(r.get("approval_visible"))
|
||||
self.assertTrue(any("no visible APPROVED review" in x for x in r["reasons"]))
|
||||
@@ -983,7 +1101,9 @@ class TestMergePR(unittest.TestCase):
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,merge"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_merge_pr(
|
||||
pr_number=8, confirmation=self._confirm(8), remote="prgs")
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertTrue(r.get("has_blocking_change_requests"))
|
||||
self.assertTrue(any("REQUEST_CHANGES" in x for x in r["reasons"]))
|
||||
@@ -1084,15 +1204,18 @@ class TestReviewPR(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = [{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1234"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}]
|
||||
head_sha = FULL_HEAD_SHA
|
||||
mock_get_all.return_value = [{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": head_sha}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}]
|
||||
# mock_api responses: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility)
|
||||
mock_api.side_effect = [
|
||||
{"login": "jcwalker3"}, # /api/v1/user (inventory)
|
||||
{"login": "jcwalker3"}, # /api/v1/user (submit eligibility)
|
||||
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": "abc1234"}, "mergeable": True}, # /pulls/1
|
||||
{"user": {"login": "jcwalker3"}, "state": "open", "head": {"sha": head_sha}, "mergeable": True}, # /pulls/1
|
||||
]
|
||||
_init_reviewer_session("prgs")
|
||||
gitea_mark_final_review_decision(1, "approve", remote="prgs")
|
||||
from mcp_server import init_review_decision_lock
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
with patch("mcp_server._list_pr_lease_comments", return_value=[]):
|
||||
_seed_ready_review_decision(1, "approve", sha=head_sha, remote="prgs")
|
||||
result = gitea_review_pr(
|
||||
pr_number=1,
|
||||
event="APPROVE",
|
||||
@@ -1748,7 +1871,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
|
||||
"""Block incidental live review mutations during validation."""
|
||||
|
||||
PR = 203
|
||||
SHA = "abc123"
|
||||
SHA = FULL_HEAD_SHA
|
||||
|
||||
def _pr(self, author, sha=SHA):
|
||||
return {
|
||||
@@ -1773,6 +1896,18 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
|
||||
self.addCleanup(self._auth_identity_patch.stop)
|
||||
self.addCleanup(self._lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
self._pr_lease_comments_patch = patch(
|
||||
"mcp_server._list_pr_lease_comments", return_value=[]
|
||||
)
|
||||
self._pr_lease_comments_patch.start()
|
||||
self._pr_work_lease_patch = patch(
|
||||
"mcp_server._pr_work_lease_reviewer_block",
|
||||
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
|
||||
)
|
||||
self._pr_work_lease_patch.start()
|
||||
self.addCleanup(self._pr_lease_comments_patch.stop)
|
||||
self.addCleanup(self._pr_work_lease_patch.stop)
|
||||
|
||||
|
||||
def _env(self):
|
||||
return patch.dict(os.environ, {
|
||||
@@ -1870,15 +2005,28 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
import reviewer_pr_lease
|
||||
|
||||
_init_reviewer_session("prgs")
|
||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
||||
self._lease_patch = _install_owned_reviewer_lease(8)
|
||||
self._lease_patch.start()
|
||||
self._auth_identity_patch = patch(
|
||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
||||
)
|
||||
self._auth_identity_patch.start()
|
||||
self._pr_lease_comments_patch = patch(
|
||||
"mcp_server._list_pr_lease_comments", return_value=[]
|
||||
)
|
||||
self._pr_lease_comments_patch.start()
|
||||
self._pr_work_lease_patch = patch(
|
||||
"mcp_server._pr_work_lease_reviewer_block",
|
||||
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
|
||||
)
|
||||
self._pr_work_lease_patch.start()
|
||||
gitea_mark_final_review_decision(
|
||||
8, "approve", remote="prgs", expected_head_sha="abc123",
|
||||
)
|
||||
self.addCleanup(self._auth_identity_patch.stop)
|
||||
self.addCleanup(self._lease_patch.stop)
|
||||
self.addCleanup(self._pr_lease_comments_patch.stop)
|
||||
self.addCleanup(self._pr_work_lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
|
||||
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
||||
@@ -2039,10 +2187,11 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_comment_succeeds_when_review_eligible(self, _auth, mock_api):
|
||||
gitea_mark_final_review_decision(8, "comment", remote="prgs")
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"), {"id": 3},
|
||||
]
|
||||
gitea_mark_final_review_decision(8, "comment", remote="prgs", expected_head_sha="abc123")
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
@@ -2058,12 +2207,15 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \
|
||||
patch("mcp_server.api_request") as mock_api:
|
||||
mock_api.side_effect = [
|
||||
{"login": "jcwalker3"}, self._pr("jcwalker3"),
|
||||
{"login": "jcwalker3"}, self._pr("jcwalker3"), {"id": 4},
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
gitea_mark_final_review_decision(8, "comment", remote="prgs")
|
||||
gitea_mark_final_review_decision(
|
||||
8, "comment", remote="prgs", expected_head_sha="abc123",
|
||||
)
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="comment", body="note", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
@@ -2106,14 +2258,14 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH), \
|
||||
patch("mcp_server.api_request") as mock_api:
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot", sha="abc123"),
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot", sha="deadbeef"),
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve",
|
||||
expected_head_sha="deadbeef", remote="prgs",
|
||||
expected_head_sha="abc123", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
@@ -2164,7 +2316,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve",
|
||||
"GITEA_TOKEN": "super-secret-token"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
gitea_mark_final_review_decision(5, "approve", remote="prgs")
|
||||
gitea_mark_final_review_decision(5, "approve", remote="prgs", expected_head_sha="abc123")
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=5, action="approve", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
@@ -2184,7 +2336,6 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
@@ -2274,7 +2425,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
|
||||
def test_mark_final_decision_rejects_remote_mismatch(self):
|
||||
_init_reviewer_session("prgs")
|
||||
r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools")
|
||||
r = gitea_mark_final_review_decision(8, "approve", remote="dadeschools", expected_head_sha="abc123")
|
||||
self.assertFalse(r["marked_ready"])
|
||||
self.assertTrue(any("does not match locked remote" in x for x in r["reasons"]))
|
||||
|
||||
@@ -2286,28 +2437,30 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
{"id": 42, "state": "APPROVED"},
|
||||
[_formal_review("reviewer-bot", "APPROVED", review_id=42)],
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 43, "state": "REQUEST_CHANGES"},
|
||||
[_formal_review("reviewer-bot", "REQUEST_CHANGES", review_id=43)],
|
||||
]
|
||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve,request_changes"}
|
||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
first = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
auth = gitea_authorize_review_correction(
|
||||
prior_review_id=42,
|
||||
prior_review_state="approve",
|
||||
reason="operator approved correcting mistaken approve",
|
||||
operator_authorized=True,
|
||||
)
|
||||
_mark_request_changes_ready(remote="prgs")
|
||||
second = gitea_submit_pr_review(
|
||||
pr_number=8, action="request_changes", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
with patch("mcp_server.gitea_get_pr_review_feedback",
|
||||
return_value=dict(_NO_BLOCKER_FEEDBACK)):
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
first = gitea_submit_pr_review(
|
||||
pr_number=8, action="approve", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
auth = gitea_authorize_review_correction(
|
||||
prior_review_id=42,
|
||||
prior_review_state="approve",
|
||||
reason="operator approved correcting mistaken approve",
|
||||
operator_authorized=True,
|
||||
)
|
||||
_mark_request_changes_ready(remote="prgs")
|
||||
second = gitea_submit_pr_review(
|
||||
pr_number=8, action="request_changes", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertTrue(first["performed"])
|
||||
self.assertTrue(auth["authorized"])
|
||||
self.assertTrue(second["performed"])
|
||||
@@ -2319,7 +2472,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
# Mark decision for specific remote, org, repo
|
||||
gitea_mark_final_review_decision(8, "approve", remote="prgs", org="MyOrg", repo="MyRepo")
|
||||
gitea_mark_final_review_decision(8, "approve", remote="prgs", expected_head_sha="abc123", org="MyOrg", repo="MyRepo")
|
||||
|
||||
# Mismatched remote
|
||||
r = gitea_submit_pr_review(
|
||||
@@ -2362,6 +2515,11 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
self.mock_audit = patch("gitea_audit.write_event").start()
|
||||
# gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216).
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["read", "merge", "edit", "close", "gitea.pr.close", "gitea.issue.close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
patch("mcp_server._list_pr_lease_comments", return_value=[]).start()
|
||||
patch(
|
||||
"mcp_server._pr_work_lease_reviewer_block",
|
||||
return_value=dict(_NO_PR_WORK_LEASE_BLOCK),
|
||||
).start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
@@ -2408,7 +2566,8 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
def test_merge_pr_with_closes_removes_label(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
|
||||
head_sha = FULL_HEAD_SHA
|
||||
lease_patch = _install_owned_reviewer_lease(1, head_sha=head_sha)
|
||||
lease_patch.start()
|
||||
self.addCleanup(lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
@@ -2417,12 +2576,12 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and url.endswith("/reviews"):
|
||||
return [_formal_review("reviewer", "APPROVED", sha="sha123")]
|
||||
return [_formal_review("reviewer", "APPROVED", sha=head_sha)]
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
"state": "open",
|
||||
"head": {"sha": "sha123", "ref": "feat/my-branch"},
|
||||
"head": {"sha": head_sha, "ref": "feat/my-branch"},
|
||||
"base": {"ref": "main"},
|
||||
"mergeable": True,
|
||||
"merged_commit_sha": "merge123",
|
||||
@@ -2442,14 +2601,18 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_merge_pr(pr_number=1, confirmation="MERGE PR 1", do="merge")
|
||||
res = gitea_merge_pr(
|
||||
pr_number=1, confirmation="MERGE PR 1", do="merge",
|
||||
expected_head_sha=head_sha,
|
||||
)
|
||||
self.assertTrue(res["performed"])
|
||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||
|
||||
def test_merge_pr_with_branch_name_removes_label(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
|
||||
head_sha = FULL_HEAD_SHA
|
||||
lease_patch = _install_owned_reviewer_lease(1, head_sha=head_sha)
|
||||
lease_patch.start()
|
||||
self.addCleanup(lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
@@ -2458,12 +2621,12 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and url.endswith("/reviews"):
|
||||
return [_formal_review("reviewer", "APPROVED", sha="sha123")]
|
||||
return [_formal_review("reviewer", "APPROVED", sha=head_sha)]
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
"state": "open",
|
||||
"head": {"sha": "sha123", "ref": "fix/issue-123-slug"},
|
||||
"head": {"sha": head_sha, "ref": "fix/issue-123-slug"},
|
||||
"base": {"ref": "main"},
|
||||
"mergeable": True,
|
||||
"merged_commit_sha": "merge123",
|
||||
@@ -2483,7 +2646,10 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_merge_pr(pr_number=1, confirmation="MERGE PR 1", do="merge")
|
||||
res = gitea_merge_pr(
|
||||
pr_number=1, confirmation="MERGE PR 1", do="merge",
|
||||
expected_head_sha=head_sha,
|
||||
)
|
||||
self.assertTrue(res["performed"])
|
||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||
|
||||
@@ -3191,7 +3357,12 @@ class TestIssueLocking(unittest.TestCase):
|
||||
"""Test issue locking and PR gating constraints."""
|
||||
|
||||
def setUp(self):
|
||||
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True)
|
||||
self._lock_dir = tempfile.TemporaryDirectory()
|
||||
env = {
|
||||
**ISSUE_WRITE_ENV,
|
||||
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
|
||||
}
|
||||
self._env_patcher = patch.dict(os.environ, env, clear=True)
|
||||
self._env_patcher.start()
|
||||
self._dup_fetcher_patcher = patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
@@ -3202,15 +3373,21 @@ class TestIssueLocking(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
self._dup_fetcher_patcher.stop()
|
||||
self._env_patcher.stop()
|
||||
if os.path.exists(ISSUE_LOCK_FILE):
|
||||
os.remove(ISSUE_LOCK_FILE)
|
||||
self._lock_dir.cleanup()
|
||||
|
||||
def _create_pr_env(self) -> dict:
|
||||
return {
|
||||
**CREATE_PR_ENV,
|
||||
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
|
||||
}
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_success(self, _auth, _git_state):
|
||||
def test_lock_issue_success(self, _auth, _api, _git_state):
|
||||
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work")
|
||||
@@ -3220,9 +3397,8 @@ class TestIssueLocking(unittest.TestCase):
|
||||
self.assertIn("expires_at", res["work_lease"])
|
||||
self.assertIn("last_heartbeat_at", res["work_lease"])
|
||||
self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default")
|
||||
self.assertTrue(os.path.exists(ISSUE_LOCK_FILE))
|
||||
with open(ISSUE_LOCK_FILE, encoding="utf-8") as f:
|
||||
lock = json.load(f)
|
||||
self.assertIn("lock_file_path", res)
|
||||
lock = issue_lock_store.read_lock_file(res["lock_file_path"])
|
||||
self.assertIn("worktree_path", lock)
|
||||
self.assertIn("work_lease", lock)
|
||||
|
||||
@@ -3278,32 +3454,81 @@ class TestIssueLocking(unittest.TestCase):
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception))
|
||||
|
||||
def test_lock_issue_blocks_active_same_operation_lease(self):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump({
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_adopts_exact_own_branch(self, _auth, mock_api, _git_state):
|
||||
branch = "feat/issue-196-mutations"
|
||||
self.mock_dup_fetcher.return_value = ([], [branch], {"status": "not_claimed"})
|
||||
mock_api.return_value = [{"name": branch, "commit": {"id": "abc123"}}]
|
||||
res = gitea_lock_issue(issue_number=196, branch_name=branch, remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertIn("adoption", res)
|
||||
self.assertEqual(res["adoption"]["branch_head_commit"], "abc123")
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_blocks_active_same_operation_lease(self, _auth, _api, _git_state):
|
||||
prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
|
||||
issue_lock_store.save_lock_file(
|
||||
issue_lock_store.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo=prgs_repo,
|
||||
issue_number=196,
|
||||
),
|
||||
{
|
||||
"issue_number": 196,
|
||||
"branch_name": "feat/issue-196-other-work",
|
||||
"remote": "prgs",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": prgs_repo,
|
||||
"worktree_path": "/tmp/other-worktree",
|
||||
"work_lease": {
|
||||
"operation_type": "author_issue_work",
|
||||
"expires_at": "2999-01-01T00:00:00Z",
|
||||
},
|
||||
}, f)
|
||||
},
|
||||
)
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already has an active author_issue_work lease", str(ctx.exception))
|
||||
|
||||
def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump({
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self, _auth, _api, _git_state):
|
||||
prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
|
||||
issue_lock_store.save_lock_file(
|
||||
issue_lock_store.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo=prgs_repo,
|
||||
issue_number=196,
|
||||
),
|
||||
{
|
||||
"issue_number": 196,
|
||||
"branch_name": "feat/issue-196-other-work",
|
||||
"remote": "prgs",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": prgs_repo,
|
||||
"worktree_path": "/tmp/other-worktree",
|
||||
"work_lease": {
|
||||
"operation_type": "author_issue_work",
|
||||
"expires_at": "2000-01-01T00:00:00Z",
|
||||
},
|
||||
}, f)
|
||||
},
|
||||
)
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("Recovery review is required before takeover", str(ctx.exception))
|
||||
@@ -3370,9 +3595,7 @@ class TestIssueLocking(unittest.TestCase):
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_missing_lock_fails(self, _auth, _role):
|
||||
if os.path.exists(ISSUE_LOCK_FILE):
|
||||
os.remove(ISSUE_LOCK_FILE)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("Issue lock is missing", str(ctx.exception))
|
||||
@@ -3381,37 +3604,64 @@ class TestIssueLocking(unittest.TestCase):
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_branch_mismatch_fails(self, _auth, _role):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=196, branch_name="feat/issue-196-mutations"), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
_bind_test_lock(
|
||||
issue_number=196,
|
||||
branch_name="feat/issue-196-mutations",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-different", remote="prgs")
|
||||
gitea_create_pr(
|
||||
title="feat: X Closes #196",
|
||||
head="feat/issue-196-different",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIn("does not match locked branch", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_forbidden_terms_fails(self, _auth, _role):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=196, branch_name="feat/issue-196-mutations"), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
_bind_test_lock(
|
||||
issue_number=196,
|
||||
branch_name="feat/issue-196-mutations",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
||||
for term in ("equivalent to #196", "related to #196", "same as #196"):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(title=f"feat: X {term}", head="feat/issue-196-mutations", remote="prgs")
|
||||
gitea_create_pr(
|
||||
title=f"feat: X {term}",
|
||||
head="feat/issue-196-mutations",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIn("contains forbidden term", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_missing_closes_ref_fails(self, _auth, _role):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=196, branch_name="feat/issue-196-mutations"), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
_bind_test_lock(
|
||||
issue_number=196,
|
||||
branch_name="feat/issue-196-mutations",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs")
|
||||
gitea_create_pr(
|
||||
title="feat: X refs #196",
|
||||
head="feat/issue-196-mutations",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
@@ -3419,13 +3669,13 @@ class TestIssueLocking(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_worktree_mismatch_fails(self, _auth, _role):
|
||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr")
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
_bind_test_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
remote="prgs",
|
||||
)
|
||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(
|
||||
title="feat: lock scratch worktree Closes #249",
|
||||
@@ -3439,22 +3689,35 @@ class TestIssueLocking(unittest.TestCase):
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_manual_lock_seed_blocked(self, _auth, _role):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(
|
||||
worktree = os.path.realpath(os.getcwd())
|
||||
with tempfile.TemporaryDirectory() as lock_dir:
|
||||
env = {**self._create_pr_env(), "GITEA_ISSUE_LOCK_DIR": lock_dir}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
issue_lock_store.save_lock_file(
|
||||
issue_lock_store.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo=mcp_server.REMOTES["prgs"]["repo"],
|
||||
issue_number=447,
|
||||
lock_dir=lock_dir,
|
||||
),
|
||||
_sample_issue_lock(
|
||||
issue_number=447,
|
||||
branch_name="feat/issue-447-lock-provenance",
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo=mcp_server.REMOTES["prgs"]["repo"],
|
||||
worktree_path=worktree,
|
||||
lock_provenance=None,
|
||||
),
|
||||
f,
|
||||
)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_create_pr(
|
||||
title="feat: lock provenance Closes #447",
|
||||
head="feat/issue-447-lock-provenance",
|
||||
remote="prgs",
|
||||
)
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_create_pr(
|
||||
title="feat: lock provenance Closes #447",
|
||||
head="feat/issue-447-lock-provenance",
|
||||
remote="prgs",
|
||||
worktree_path=worktree,
|
||||
)
|
||||
self.assertIn("lock provenance", str(ctx.exception).lower())
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@@ -3464,13 +3727,13 @@ class TestIssueLocking(unittest.TestCase):
|
||||
def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
|
||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
|
||||
mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
_bind_test_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
remote="prgs",
|
||||
)
|
||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
||||
res = gitea_create_pr(
|
||||
title="feat: issue-lock scratch worktree Closes #249",
|
||||
head="feat/issue-249-issue-lock-scratch-worktree",
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
"""Hermetic tests for merge approval head pinning (#471)."""
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from merge_approval_gate import assess_merge_approval_head # noqa: E402
|
||||
|
||||
HEAD_OLD = "8b61c4b41f1b49b271ed3b99657431cf06eeda3e"
|
||||
HEAD_NEW = "3e4b721d60e97147ba0704773cf57cd0d42cbe31"
|
||||
|
||||
|
||||
class TestMergeApprovalGate(unittest.TestCase):
|
||||
def test_fresh_approval_at_current_head(self):
|
||||
result = assess_merge_approval_head(
|
||||
current_head_sha=HEAD_NEW,
|
||||
latest_by_reviewer={
|
||||
"reviewer1": {
|
||||
"verdict": "APPROVED",
|
||||
"dismissed": False,
|
||||
"reviewed_head_sha": HEAD_NEW,
|
||||
"submitted_at": "2026-07-06T12:00:00Z",
|
||||
}
|
||||
},
|
||||
)
|
||||
self.assertTrue(result["approval_at_current_head"])
|
||||
self.assertIsNone(result["stale_approval_block_reason"])
|
||||
|
||||
def test_stale_approval_after_rebase(self):
|
||||
result = assess_merge_approval_head(
|
||||
current_head_sha=HEAD_NEW,
|
||||
latest_by_reviewer={
|
||||
"reviewer1": {
|
||||
"verdict": "APPROVED",
|
||||
"dismissed": False,
|
||||
"reviewed_head_sha": HEAD_OLD,
|
||||
"submitted_at": "2026-07-06T10:00:00Z",
|
||||
}
|
||||
},
|
||||
)
|
||||
self.assertFalse(result["approval_at_current_head"])
|
||||
self.assertEqual(result["latest_approved_head_sha"], HEAD_OLD)
|
||||
self.assertIn("stale approval", result["stale_approval_block_reason"])
|
||||
self.assertIn(HEAD_OLD, result["stale_approval_block_reason"])
|
||||
self.assertIn(HEAD_NEW, result["stale_approval_block_reason"])
|
||||
self.assertIn("re-review PR at current head", result["stale_approval_block_reason"])
|
||||
|
||||
def test_no_approval_entries(self):
|
||||
result = assess_merge_approval_head(
|
||||
current_head_sha=HEAD_NEW,
|
||||
latest_by_reviewer={},
|
||||
)
|
||||
self.assertFalse(result["approval_at_current_head"])
|
||||
self.assertIsNone(result["latest_approved_head_sha"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -231,8 +231,9 @@ class TestEligibilityDenialReport(PermissionReportBase):
|
||||
return PR_PAYLOAD
|
||||
mock_api.side_effect = fake_api
|
||||
mcp_server.init_review_decision_lock("prgs", "review_pr")
|
||||
mcp_server.gitea_load_review_workflow()
|
||||
mcp_server.gitea_mark_final_review_decision(42, "approve", remote="prgs")
|
||||
from tests.test_mcp_server import _seed_ready_review_decision
|
||||
|
||||
_seed_ready_review_decision(42, "approve", remote="prgs")
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
res = mcp_server.gitea_submit_pr_review(
|
||||
pr_number=42, action="approve", body="lgtm", remote="prgs",
|
||||
@@ -276,8 +277,9 @@ class TestReviewCommentPathUsesCanonicalOp(PermissionReportBase):
|
||||
return PR_PAYLOAD
|
||||
mock_api.side_effect = fake_api
|
||||
mcp_server.init_review_decision_lock("prgs", "review_pr")
|
||||
mcp_server.gitea_load_review_workflow()
|
||||
mcp_server.gitea_mark_final_review_decision(42, "comment", remote="prgs")
|
||||
from tests.test_mcp_server import _seed_ready_review_decision
|
||||
|
||||
_seed_ready_review_decision(42, "comment", remote="prgs")
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
res = mcp_server.gitea_submit_pr_review(
|
||||
pr_number=42, action="comment", body="finding", remote="prgs",
|
||||
|
||||
@@ -129,18 +129,29 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
|
||||
from mcp_server import init_review_decision_lock
|
||||
from tests.test_mcp_server import (
|
||||
FULL_HEAD_SHA,
|
||||
_install_owned_reviewer_lease,
|
||||
_seed_ready_review_decision,
|
||||
)
|
||||
import reviewer_pr_lease
|
||||
|
||||
head_sha = FULL_HEAD_SHA
|
||||
mock_fetch.return_value = _final_page_fetch([
|
||||
{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "other_user"}}
|
||||
{"number": 1, "title": "PR 1", "state": "open",
|
||||
"head": {"ref": "branch1", "sha": head_sha},
|
||||
"base": {"ref": "master"}, "mergeable": True,
|
||||
"user": {"login": "other_user"}}
|
||||
])
|
||||
# mock_api: inventory whoami, eligibility whoami, eligibility PR,
|
||||
# POST review (#244: state + visible-verdict GET reviews).
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer1"},
|
||||
{"login": "reviewer1"},
|
||||
{
|
||||
"user": {"login": "other_user"},
|
||||
"state": "open",
|
||||
"head": {"sha": "abc1"},
|
||||
"head": {"sha": head_sha},
|
||||
"mergeable": True,
|
||||
},
|
||||
{"id": 100, "state": "APPROVED"},
|
||||
@@ -149,23 +160,19 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"id": 100,
|
||||
"user": {"login": "reviewer1"},
|
||||
"state": "APPROVED",
|
||||
"commit_id": "abc1",
|
||||
"commit_id": head_sha,
|
||||
"submitted_at": "2026-07-06T10:00:00Z",
|
||||
"dismissed": False,
|
||||
}
|
||||
],
|
||||
]
|
||||
|
||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||
from tests.test_mcp_server import _install_owned_reviewer_lease
|
||||
import reviewer_pr_lease
|
||||
|
||||
with patch("mcp_server._authenticated_username", return_value="reviewer1"):
|
||||
with patch("mcp_server._authenticated_username", return_value="reviewer1"), \
|
||||
patch("mcp_server._list_pr_lease_comments", return_value=[]):
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
gitea_load_review_workflow()
|
||||
gitea_mark_final_review_decision(1, "approve", remote="prgs")
|
||||
_seed_ready_review_decision(1, "approve", sha=head_sha, remote="prgs")
|
||||
lease_patch = _install_owned_reviewer_lease(
|
||||
1, head_sha="abc1", session_id="inventory-review-lease",
|
||||
1, head_sha=head_sha, session_id="inventory-review-lease",
|
||||
)
|
||||
lease_patch.start()
|
||||
self.addCleanup(lease_patch.stop)
|
||||
|
||||
@@ -0,0 +1,207 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Regression tests for conflict-fix and reviewer PR work leases (#399)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
from pr_work_lease import ( # noqa: E402
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
REVIEWER_LEASE_MARKER,
|
||||
assess_conflict_fix_final_report,
|
||||
assess_conflict_fix_push,
|
||||
assess_head_sha_equality,
|
||||
assess_reviewer_mutation_blocked,
|
||||
assess_reviewer_stale_head_final_report,
|
||||
format_conflict_fix_lease_body,
|
||||
parse_conflict_fix_lease_comment,
|
||||
parse_reviewer_lease_comment,
|
||||
)
|
||||
|
||||
HEAD_A = "a" * 40
|
||||
HEAD_B = "b" * 40
|
||||
NOW = datetime(2026, 7, 7, 15, 0, tzinfo=timezone.utc)
|
||||
|
||||
|
||||
def _reviewer_lease_body(*, phase: str = "validating", expires_minutes: int = 60) -> str:
|
||||
expires = (NOW + timedelta(minutes=expires_minutes)).isoformat().replace("+00:00", "Z")
|
||||
return "\n".join([
|
||||
REVIEWER_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"phase: " + phase,
|
||||
f"candidate_head: {HEAD_A}",
|
||||
f"expires_at: {expires}",
|
||||
"profile: prgs-reviewer",
|
||||
])
|
||||
|
||||
|
||||
def _conflict_fix_body(*, phase: str = "claimed", worktree: str = "branches/fix-376") -> str:
|
||||
expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z")
|
||||
return "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
f"phase: {phase}",
|
||||
f"worktree: {worktree}",
|
||||
f"head_before: {HEAD_A}",
|
||||
f"expires_at: {expires}",
|
||||
"profile: prgs-author",
|
||||
])
|
||||
|
||||
|
||||
class TestLeaseParsing(unittest.TestCase):
|
||||
def test_parse_reviewer_lease(self):
|
||||
parsed = parse_reviewer_lease_comment(_reviewer_lease_body())
|
||||
self.assertEqual(parsed["pr_number"], 376)
|
||||
self.assertEqual(parsed["phase"], "validating")
|
||||
self.assertEqual(parsed["candidate_head"], HEAD_A)
|
||||
|
||||
def test_parse_conflict_fix_lease(self):
|
||||
parsed = parse_conflict_fix_lease_comment(_conflict_fix_body())
|
||||
self.assertEqual(parsed["pr_number"], 376)
|
||||
self.assertEqual(parsed["phase"], "claimed")
|
||||
|
||||
|
||||
class TestConflictFixPushGate(unittest.TestCase):
|
||||
def test_blocks_push_during_active_reviewer_lease(self):
|
||||
comments = [{"body": _reviewer_lease_body()}]
|
||||
result = assess_conflict_fix_push(
|
||||
pr_number=376,
|
||||
comments=comments,
|
||||
branch_head_before=HEAD_A,
|
||||
branch_head_after=HEAD_B,
|
||||
worktree_path="branches/fix-376",
|
||||
push_cwd="/proj/branches/fix-376",
|
||||
is_fast_forward=True,
|
||||
now=NOW,
|
||||
)
|
||||
self.assertFalse(result["push_allowed"])
|
||||
self.assertTrue(any("reviewer lease" in r for r in result["reasons"]))
|
||||
|
||||
def test_rejects_non_fast_forward(self):
|
||||
result = assess_conflict_fix_push(
|
||||
pr_number=376,
|
||||
comments=[],
|
||||
branch_head_before=HEAD_A,
|
||||
branch_head_after=HEAD_B,
|
||||
worktree_path="branches/fix-376",
|
||||
push_cwd="/proj/branches/fix-376",
|
||||
is_fast_forward=False,
|
||||
now=NOW,
|
||||
)
|
||||
self.assertFalse(result["push_allowed"])
|
||||
self.assertTrue(any("non-fast-forward" in r for r in result["reasons"]))
|
||||
|
||||
def test_wrong_cwd_push_attempt(self):
|
||||
result = assess_conflict_fix_push(
|
||||
pr_number=376,
|
||||
comments=[],
|
||||
branch_head_before=HEAD_A,
|
||||
branch_head_after=HEAD_B,
|
||||
worktree_path="branches/fix-376",
|
||||
push_cwd="/proj/master",
|
||||
is_fast_forward=True,
|
||||
now=NOW,
|
||||
)
|
||||
self.assertFalse(result["push_allowed"])
|
||||
self.assertTrue(any("cwd" in r.lower() for r in result["reasons"]))
|
||||
|
||||
def test_sibling_conflict_fix_collision(self):
|
||||
comments = [{"body": _conflict_fix_body(phase="pushing", worktree="branches/other")}]
|
||||
result = assess_conflict_fix_push(
|
||||
pr_number=376,
|
||||
comments=comments,
|
||||
branch_head_before=HEAD_A,
|
||||
branch_head_after=HEAD_B,
|
||||
worktree_path="branches/fix-376",
|
||||
push_cwd="/proj/branches/fix-376",
|
||||
is_fast_forward=True,
|
||||
now=NOW,
|
||||
)
|
||||
self.assertFalse(result["push_allowed"])
|
||||
self.assertTrue(any("sibling conflict-fix" in r for r in result["reasons"]))
|
||||
|
||||
|
||||
class TestReviewerMutationGate(unittest.TestCase):
|
||||
def test_blocks_review_during_conflict_fix(self):
|
||||
comments = [{"body": _conflict_fix_body(phase="pushing")}]
|
||||
result = assess_reviewer_mutation_blocked(
|
||||
pr_number=376,
|
||||
comments=comments,
|
||||
reviewed_head_sha=HEAD_A,
|
||||
live_head_sha=HEAD_A,
|
||||
mutation="approve",
|
||||
now=NOW,
|
||||
)
|
||||
self.assertFalse(result["mutation_allowed"])
|
||||
self.assertTrue(any("conflict-fix lease" in r for r in result["reasons"]))
|
||||
|
||||
def test_stale_head_blocks_approval(self):
|
||||
result = assess_reviewer_mutation_blocked(
|
||||
pr_number=376,
|
||||
comments=[],
|
||||
reviewed_head_sha=HEAD_A,
|
||||
live_head_sha=HEAD_B,
|
||||
mutation="merge",
|
||||
now=NOW,
|
||||
)
|
||||
self.assertFalse(result["mutation_allowed"])
|
||||
self.assertTrue(result["head_check"]["head_changed"])
|
||||
|
||||
def test_head_equality_required_fields(self):
|
||||
result = assess_head_sha_equality(HEAD_A, HEAD_B)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["head_changed"])
|
||||
|
||||
|
||||
class TestFinalReportProof(unittest.TestCase):
|
||||
def test_reviewer_stale_head_report_requires_fields(self):
|
||||
result = assess_reviewer_stale_head_final_report("no head proof here")
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_reviewer_stale_head_report_passes(self):
|
||||
report = "\n".join([
|
||||
f"Reviewed head SHA: {HEAD_A}",
|
||||
f"Final live head SHA before approval: {HEAD_A}",
|
||||
f"Final live head SHA before merge: {HEAD_A}",
|
||||
"Push occurred during validation: no",
|
||||
])
|
||||
result = assess_reviewer_stale_head_final_report(report)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_conflict_fix_report_requires_fields(self):
|
||||
result = assess_conflict_fix_final_report("incomplete")
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_conflict_fix_report_passes(self):
|
||||
report = "\n".join([
|
||||
f"Branch head before push: {HEAD_A}",
|
||||
f"Branch head after push: {HEAD_B}",
|
||||
"Active reviewer lease status: none",
|
||||
"Whether push was fast-forward: yes",
|
||||
"Whether any reviewer was active: no",
|
||||
])
|
||||
result = assess_conflict_fix_final_report(report)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
|
||||
class TestFormatLease(unittest.TestCase):
|
||||
def test_format_conflict_fix_lease_includes_marker(self):
|
||||
body = format_conflict_fix_lease_body(
|
||||
pr_number=376,
|
||||
branch="feat/x",
|
||||
worktree="branches/fix-376",
|
||||
profile="prgs-author",
|
||||
head_before=HEAD_A,
|
||||
)
|
||||
self.assertIn(CONFLICT_FIX_LEASE_MARKER, body)
|
||||
parsed = parse_conflict_fix_lease_comment(body)
|
||||
self.assertEqual(parsed["pr_number"], 376)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -115,6 +115,22 @@ class TestPRReviewFeedbackDiscovery(unittest.TestCase):
|
||||
result["latest_review_state_by_reviewer"], {"reviewer1": "APPROVED"})
|
||||
self.assertFalse(result["has_blocking_change_requests"])
|
||||
self.assertTrue(result["approval_visible"])
|
||||
self.assertTrue(result["approval_at_current_head"])
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_stale_approval_not_at_current_head(self, mock_get_profile, _auth, mock_api):
|
||||
mock_get_profile.return_value = self._profile()
|
||||
mock_api.side_effect = [
|
||||
_pr_details(head_sha="newhead3"),
|
||||
[_review("reviewer1", "APPROVED", commit_id="oldhead1")],
|
||||
]
|
||||
result = gitea_get_pr_review_feedback(pr_number=5, remote="prgs")
|
||||
self.assertTrue(result["approval_visible"])
|
||||
self.assertFalse(result["approval_at_current_head"])
|
||||
self.assertEqual(result["latest_approved_head_sha"], "oldhead1")
|
||||
self.assertIn("stale approval", result["stale_approval_block_reason"])
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
|
||||
@@ -19,7 +19,11 @@ def _minimal_review_report(**overrides):
|
||||
"- Issue/PR: #182 / PR #203",
|
||||
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Files changed: review_proofs.py",
|
||||
"- Validation: pytest tests/test_review_proofs.py -q in branches/review-203",
|
||||
"- Validation: pass: pytest tests/test_review_proofs.py -q in branches/review-203",
|
||||
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Push occurred during validation: no",
|
||||
"- Mutations: review only",
|
||||
"- File edits by reviewer: none",
|
||||
"- Worktree/index mutations: none",
|
||||
@@ -80,7 +84,7 @@ class TestReviewFinalReportSchema(unittest.TestCase):
|
||||
|
||||
def test_reviewed_head_without_validation_blocks(self):
|
||||
report = _minimal_review_report().replace(
|
||||
"- Validation: pytest tests/test_review_proofs.py -q in branches/review-203",
|
||||
"- Validation: pass: pytest tests/test_review_proofs.py -q in branches/review-203",
|
||||
"- Validation: not run",
|
||||
)
|
||||
report += "\n- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9"
|
||||
|
||||
@@ -130,7 +130,9 @@ class TestReviewWorkflowLoadGates(unittest.TestCase):
|
||||
self.assertTrue(blockers)
|
||||
|
||||
def test_dry_run_allowed_without_load(self):
|
||||
with patch("mcp_server.gitea_check_pr_eligibility") as elig:
|
||||
with patch("mcp_server.get_auth_header", return_value="Basic dGVzdA=="), \
|
||||
patch("mcp_server._list_pr_lease_comments", return_value=[]), \
|
||||
patch("mcp_server.gitea_check_pr_eligibility") as elig:
|
||||
elig.return_value = {
|
||||
"eligible": True,
|
||||
"authenticated_user": "rev",
|
||||
|
||||
@@ -12,6 +12,8 @@ from unittest.mock import patch
|
||||
|
||||
import mcp_server
|
||||
|
||||
HEAD_SHA = "a" * 40
|
||||
|
||||
|
||||
def _lock(mutations=None, correction=False):
|
||||
return {
|
||||
@@ -131,10 +133,20 @@ def _feedback(blocking, stale=False, success=True):
|
||||
"success": success,
|
||||
"has_blocking_change_requests": blocking,
|
||||
"review_feedback_stale": stale,
|
||||
"current_head_sha": "abc123",
|
||||
"current_head_sha": HEAD_SHA,
|
||||
}
|
||||
|
||||
|
||||
def _mark(action, pr_number=6, **kwargs):
|
||||
kwargs.setdefault("expected_head_sha", HEAD_SHA)
|
||||
no_lease_block = {"block": False, "reasons": [], "mutation_allowed": True}
|
||||
with patch("mcp_server._list_pr_lease_comments", return_value=[]), \
|
||||
patch("mcp_server._pr_work_lease_reviewer_block", return_value=no_lease_block):
|
||||
return mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=pr_number, action=action, remote="prgs", **kwargs
|
||||
)
|
||||
|
||||
|
||||
class TestDuplicateRequestChangesSuppression(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
mcp_server._save_review_decision_lock(None)
|
||||
@@ -144,8 +156,7 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=True, stale=False)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
result = _mark("request_changes")
|
||||
self.assertFalse(result["marked_ready"])
|
||||
self.assertTrue(
|
||||
any("duplicate" in r for r in result["reasons"]),
|
||||
@@ -155,16 +166,14 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=True, stale=True)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
result = _mark("request_changes")
|
||||
self.assertTrue(result["marked_ready"], result.get("reasons"))
|
||||
|
||||
def test_request_changes_allowed_when_no_blocker(self):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=False)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
result = _mark("request_changes")
|
||||
self.assertTrue(result["marked_ready"], result.get("reasons"))
|
||||
|
||||
def test_request_changes_fails_closed_when_feedback_unavailable(self):
|
||||
@@ -172,8 +181,7 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=False,
|
||||
success=False)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
result = _mark("request_changes")
|
||||
self.assertFalse(result["marked_ready"])
|
||||
self.assertTrue(
|
||||
any("could not verify" in r for r in result["reasons"]),
|
||||
@@ -183,8 +191,7 @@ class TestDuplicateRequestChangesSuppression(unittest.TestCase):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
side_effect=AssertionError("must not be called")):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="approve", remote="prgs")
|
||||
result = _mark("approve")
|
||||
self.assertTrue(result["marked_ready"], result.get("reasons"))
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
"""Tests for validation status vocabulary (#406)."""
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from final_report_validator import assess_final_report_validator # noqa: E402
|
||||
from validation_status_vocabulary import ( # noqa: E402
|
||||
STATUS_BASELINE_EQUIVALENT,
|
||||
STATUS_FAILED,
|
||||
STATUS_MERGE_SIM_RESOLVED,
|
||||
STATUS_PASSED,
|
||||
STATUS_TRANSIENT_PASS,
|
||||
assess_validation_status_vocabulary,
|
||||
)
|
||||
|
||||
|
||||
def _handoff(**extra):
|
||||
fields = {
|
||||
"Task": "review PR #386",
|
||||
"Validation status": STATUS_PASSED,
|
||||
"Raw PR-head validation result": "passed",
|
||||
"Merge simulation result": "not run",
|
||||
"Baseline worktree used": "none",
|
||||
}
|
||||
fields.update(extra)
|
||||
lines = ["## Controller Handoff", ""]
|
||||
lines.extend(f"- {key}: {value}" for key, value in fields.items())
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
class TestValidationStatusVocabulary(unittest.TestCase):
|
||||
def test_raw_head_pass_status(self):
|
||||
report = _handoff()
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertEqual(result["status_claimed"], STATUS_PASSED)
|
||||
|
||||
def test_raw_head_failure_with_baseline_match(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_BASELINE_EQUIVALENT,
|
||||
"Raw PR-head validation result": "failed",
|
||||
"Baseline worktree used": "branches/baseline-master-pr386",
|
||||
"Baseline target SHA": "a" * 40,
|
||||
"Baseline failures": "test_foo failed",
|
||||
"PR failures": "test_foo failed",
|
||||
"Failure signatures match": "true",
|
||||
}
|
||||
)
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertTrue(result["baseline_proof_complete"])
|
||||
|
||||
def test_baseline_equivalent_without_baseline_proof_blocked(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_BASELINE_EQUIVALENT,
|
||||
"Raw PR-head validation result": "failed",
|
||||
}
|
||||
)
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertIn("baseline-equivalent", result["reasons"][0])
|
||||
|
||||
def test_merge_simulation_resolution_passes(self):
|
||||
report = "\n".join([
|
||||
_handoff(
|
||||
**{
|
||||
"Validation status": STATUS_MERGE_SIM_RESOLVED,
|
||||
"Raw PR-head validation result": "failed",
|
||||
"Merge simulation result": "passed",
|
||||
}
|
||||
),
|
||||
"Worktree/index mutations: merge simulation in branches/review-pr386",
|
||||
"Worktree path: branches/review-pr386",
|
||||
"Pre-simulation clean status: clean",
|
||||
"Merge result: clean merge",
|
||||
"Abort command: git merge --abort",
|
||||
"Post-abort clean status: clean",
|
||||
])
|
||||
command_log = [
|
||||
{"command": "git merge --no-commit prgs/master"},
|
||||
{"command": "git merge --abort"},
|
||||
]
|
||||
result = assess_validation_status_vocabulary(
|
||||
report, command_log=command_log
|
||||
)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertTrue(result["merge_simulation_passed"])
|
||||
|
||||
def test_merge_simulation_failure_stays_failed(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_FAILED,
|
||||
"Raw PR-head validation result": "failed",
|
||||
"Merge simulation result": "failed",
|
||||
}
|
||||
)
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
def test_failed_status_with_passing_merge_sim_blocked(self):
|
||||
report = "\n".join([
|
||||
_handoff(
|
||||
**{
|
||||
"Validation status": STATUS_FAILED,
|
||||
"Raw PR-head validation result": "failed",
|
||||
"Merge simulation result": "passed",
|
||||
}
|
||||
),
|
||||
"Worktree/index mutations: merge simulation",
|
||||
"Worktree path: branches/review-pr386",
|
||||
"Pre-simulation clean status: clean",
|
||||
"Merge result: clean",
|
||||
"Abort command: git merge --abort",
|
||||
"Post-abort clean status: clean",
|
||||
])
|
||||
result = assess_validation_status_vocabulary(
|
||||
report,
|
||||
command_log=[{"command": "git merge --no-commit prgs/master"}],
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_transient_failure_then_pass(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_TRANSIENT_PASS,
|
||||
"Raw PR-head validation result": "passed",
|
||||
"Transient validation failure history": (
|
||||
"first run failed with infra flake; rerun passed"
|
||||
),
|
||||
}
|
||||
)
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
def test_transient_pass_without_history_blocked(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_TRANSIENT_PASS,
|
||||
"Raw PR-head validation result": "passed",
|
||||
}
|
||||
)
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_bare_passed_after_raw_failure_blocked(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_PASSED,
|
||||
"Raw PR-head validation result": "failed",
|
||||
}
|
||||
)
|
||||
result = assess_validation_status_vocabulary(report)
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_wrong_baseline_label_when_merge_sim_used_blocked(self):
|
||||
report = "\n".join([
|
||||
_handoff(
|
||||
**{
|
||||
"Validation status": STATUS_BASELINE_EQUIVALENT,
|
||||
"Raw PR-head validation result": "failed",
|
||||
"Merge simulation result": "passed",
|
||||
}
|
||||
),
|
||||
"Worktree/index mutations: merge simulation",
|
||||
"Worktree path: branches/review-pr386",
|
||||
"Pre-simulation clean status: clean",
|
||||
"Merge result: clean",
|
||||
"Abort command: git merge --abort",
|
||||
"Post-abort clean status: clean",
|
||||
])
|
||||
result = assess_validation_status_vocabulary(
|
||||
report,
|
||||
command_log=[{"command": "git merge --no-commit prgs/master"}],
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
joined = " ".join(result["reasons"]).lower()
|
||||
self.assertTrue(
|
||||
"misleading" in joined or "baseline-equivalent" in joined
|
||||
)
|
||||
|
||||
def test_final_report_validator_integration_blocks_misleading_label(self):
|
||||
report = _handoff(
|
||||
**{
|
||||
"Validation status": STATUS_BASELINE_EQUIVALENT,
|
||||
"Raw PR-head validation result": "failed",
|
||||
}
|
||||
)
|
||||
result = assess_final_report_validator(report, "review_pr")
|
||||
blocked_ids = {f["rule_id"] for f in result["findings"]}
|
||||
self.assertIn("reviewer.validation_status_vocabulary", blocked_ids)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,153 @@
|
||||
"""Tests for runtime_context / mutation-guard workspace alignment (#460)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest import mock
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
import author_mutation_worktree as amw # noqa: E402
|
||||
import gitea_mcp_server as srv # noqa: E402
|
||||
|
||||
CONTROL_ROOT = str(Path(__file__).resolve().parents[3])
|
||||
BRANCHES_WORKTREE = str(Path(__file__).resolve().parents[1])
|
||||
MCP_PROCESS_ROOT = BRANCHES_WORKTREE
|
||||
|
||||
|
||||
class TestCanonicalRepoRoot(unittest.TestCase):
|
||||
@mock.patch("subprocess.run")
|
||||
def test_resolves_main_repo_from_branches_worktree(self, mock_run):
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
||||
)
|
||||
root = amw.resolve_canonical_repo_root(BRANCHES_WORKTREE, MCP_PROCESS_ROOT)
|
||||
self.assertEqual(root, CONTROL_ROOT)
|
||||
|
||||
def test_falls_back_when_git_unavailable(self):
|
||||
root = amw.resolve_canonical_repo_root("/missing/path", MCP_PROCESS_ROOT)
|
||||
self.assertEqual(root, os.path.realpath(MCP_PROCESS_ROOT))
|
||||
|
||||
|
||||
class TestWorkspaceRepoMembership(unittest.TestCase):
|
||||
@mock.patch("os.path.isdir", return_value=True)
|
||||
@mock.patch("os.path.exists", return_value=True)
|
||||
@mock.patch("subprocess.run")
|
||||
def test_valid_branches_worktree_accepted(self, mock_run, *_exists):
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
||||
)
|
||||
result = amw.assess_workspace_repo_membership(
|
||||
workspace_path=BRANCHES_WORKTREE,
|
||||
canonical_repo_root=CONTROL_ROOT,
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
@mock.patch("os.path.isdir", return_value=True)
|
||||
@mock.patch("os.path.exists", return_value=True)
|
||||
@mock.patch("subprocess.run")
|
||||
def test_wrong_repo_rejected(self, mock_run, *_exists):
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout="/other/repo/.git\n",
|
||||
)
|
||||
result = amw.assess_workspace_repo_membership(
|
||||
workspace_path=BRANCHES_WORKTREE,
|
||||
canonical_repo_root=CONTROL_ROOT,
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertIn("does not belong", result["reasons"][0])
|
||||
|
||||
@mock.patch("os.path.exists", return_value=False)
|
||||
def test_missing_worktree_rejected(self, *_exists):
|
||||
result = amw.assess_workspace_repo_membership(
|
||||
workspace_path=f"{CONTROL_ROOT}/branches/missing-worktree",
|
||||
canonical_repo_root=CONTROL_ROOT,
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertIn("does not exist", result["reasons"][0])
|
||||
|
||||
|
||||
class TestRuntimeContextGuardAlignment(unittest.TestCase):
|
||||
def setUp(self):
|
||||
srv._preflight_whoami_called = True
|
||||
srv._preflight_capability_called = True
|
||||
srv._preflight_resolved_role = "author"
|
||||
self._orig_in_test = srv._preflight_in_test_mode
|
||||
srv._preflight_in_test_mode = lambda: False
|
||||
self._env_patch = mock.patch.dict(
|
||||
os.environ,
|
||||
{},
|
||||
clear=False,
|
||||
)
|
||||
self._env_patch.start()
|
||||
os.environ.pop("GITEA_ACTIVE_WORKTREE", None)
|
||||
os.environ.pop("GITEA_AUTHOR_WORKTREE", None)
|
||||
|
||||
def tearDown(self):
|
||||
srv._preflight_in_test_mode = self._orig_in_test
|
||||
self._env_patch.stop()
|
||||
|
||||
def test_runtime_context_and_guard_share_resolved_workspace(self):
|
||||
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
|
||||
ctx = srv._resolve_author_mutation_context(BRANCHES_WORKTREE)
|
||||
status = srv.assess_preflight_status(worktree_path=BRANCHES_WORKTREE)
|
||||
self.assertEqual(ctx["workspace_path"], os.path.realpath(BRANCHES_WORKTREE))
|
||||
self.assertEqual(ctx["canonical_repo_root"], CONTROL_ROOT)
|
||||
self.assertFalse(ctx["roots_aligned"])
|
||||
self.assertEqual(
|
||||
status["preflight_workspace"]["active_task_workspace_root"],
|
||||
os.path.realpath(BRANCHES_WORKTREE),
|
||||
)
|
||||
self.assertEqual(
|
||||
status["preflight_workspace"]["canonical_repository_root"],
|
||||
CONTROL_ROOT,
|
||||
)
|
||||
self.assertIn("workspace_root_mismatch", status["preflight_workspace"])
|
||||
|
||||
@mock.patch("subprocess.run")
|
||||
@mock.patch("os.path.isdir", return_value=True)
|
||||
@mock.patch("os.path.exists", return_value=True)
|
||||
def test_declared_branches_worktree_passes_when_mcp_root_differs(
|
||||
self, _exists, _isdir, mock_run
|
||||
):
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
||||
)
|
||||
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
|
||||
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
|
||||
srv.verify_preflight_purity(worktree_path=BRANCHES_WORKTREE)
|
||||
|
||||
def test_stable_checkout_still_rejected(self):
|
||||
with mock.patch.object(srv, "PROJECT_ROOT", CONTROL_ROOT):
|
||||
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
srv.verify_preflight_purity()
|
||||
self.assertIn("stable control checkout", str(ctx.exception))
|
||||
|
||||
@mock.patch("os.path.isdir", return_value=True)
|
||||
@mock.patch("os.path.exists", return_value=True)
|
||||
@mock.patch("subprocess.run")
|
||||
def test_non_branches_worktree_rejected(self, mock_run, *_exists):
|
||||
outside = "/tmp/outside-repo-checkout"
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
||||
)
|
||||
with mock.patch.object(srv, "PROJECT_ROOT", CONTROL_ROOT):
|
||||
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
srv.verify_preflight_purity(worktree_path=outside)
|
||||
self.assertIn("not under", str(ctx.exception))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
+26
-9
@@ -20,33 +20,50 @@ def run(script, *args):
|
||||
branch = arg
|
||||
break
|
||||
|
||||
lock_file = Path("/tmp/gitea_issue_lock.json")
|
||||
created_lock = False
|
||||
lock_dir_ctx = None
|
||||
extra_env = os.environ.copy()
|
||||
if script == "worktree-start" and branch:
|
||||
import re
|
||||
import json
|
||||
import tempfile
|
||||
import issue_lock_store
|
||||
|
||||
m = re.search(r"issue-(\d+)", branch)
|
||||
if not m:
|
||||
m = re.search(r"pr-(\d+)", branch)
|
||||
issue_num = int(m.group(1)) if m else 999
|
||||
lock_file.write_text(json.dumps({
|
||||
lock_dir_ctx = tempfile.TemporaryDirectory()
|
||||
extra_env["GITEA_ISSUE_LOCK_DIR"] = lock_dir_ctx.name
|
||||
record = {
|
||||
"issue_number": issue_num,
|
||||
"branch_name": branch,
|
||||
"remote": "prgs",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools"
|
||||
}), encoding="utf-8")
|
||||
created_lock = True
|
||||
"repo": "Gitea-Tools",
|
||||
"worktree_path": "/tmp/test-worktree",
|
||||
"work_lease": {
|
||||
"operation_type": "author_issue_work",
|
||||
"expires_at": "2999-01-01T00:00:00Z",
|
||||
},
|
||||
}
|
||||
path = issue_lock_store.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=issue_num,
|
||||
lock_dir=lock_dir_ctx.name,
|
||||
)
|
||||
issue_lock_store.save_lock_file(path, record)
|
||||
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
["bash", str(SCRIPTS / script), *args],
|
||||
capture_output=True, text=True, cwd=str(REPO),
|
||||
env=extra_env,
|
||||
)
|
||||
return proc.returncode, proc.stdout, proc.stderr
|
||||
finally:
|
||||
if created_lock and lock_file.exists():
|
||||
lock_file.unlink()
|
||||
if lock_dir_ctx is not None:
|
||||
lock_dir_ctx.cleanup()
|
||||
|
||||
|
||||
class TestWorktreeStart(unittest.TestCase):
|
||||
|
||||
Reference in New Issue
Block a user