fix: resolve conflicts for PR #416
Merge current prgs/master into feat/issue-399-conflict-fix-leases. Preserve reviewer PR lease gate (#407) before conflict-fix stale-head gate (#399); document both as §26B and §26C. Align MCP and validator tests with mandatory expected_head_sha and stale-head proof fields.
This commit is contained in:
+291
-74
@@ -60,6 +60,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)
|
||||
|
||||
@@ -79,6 +80,65 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
|
||||
return [_formal_review(reviewer, "APPROVED", sha=sha)]
|
||||
|
||||
|
||||
_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
|
||||
_NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True}
|
||||
|
||||
|
||||
def _reviewer_lease_comment(
|
||||
pr_number,
|
||||
*,
|
||||
session_id=_DEFAULT_LEASE_SESSION,
|
||||
head_sha="abc123",
|
||||
reviewer="reviewer-bot",
|
||||
):
|
||||
from datetime import datetime, timezone
|
||||
|
||||
import reviewer_pr_lease
|
||||
|
||||
body = reviewer_pr_lease.format_lease_body(
|
||||
repo="Scaled-Tech-Consulting/Gitea-Tools",
|
||||
pr_number=pr_number,
|
||||
issue_number=407,
|
||||
reviewer_identity=reviewer,
|
||||
profile="gitea-reviewer",
|
||||
session_id=session_id,
|
||||
worktree="branches/review-test",
|
||||
phase="claimed",
|
||||
candidate_head=head_sha,
|
||||
target_branch="master",
|
||||
target_branch_sha="b" * 40,
|
||||
last_activity=datetime.now(timezone.utc),
|
||||
)
|
||||
return {"id": 9001, "body": body, "user": {"login": reviewer}}
|
||||
|
||||
|
||||
def _install_owned_reviewer_lease(
|
||||
pr_number,
|
||||
*,
|
||||
session_id=_DEFAULT_LEASE_SESSION,
|
||||
head_sha="abc123",
|
||||
):
|
||||
import reviewer_pr_lease
|
||||
|
||||
reviewer_pr_lease.clear_session_lease()
|
||||
reviewer_pr_lease.record_session_lease({
|
||||
"pr_number": pr_number,
|
||||
"session_id": session_id,
|
||||
"candidate_head": head_sha,
|
||||
"target_branch": "master",
|
||||
})
|
||||
return patch(
|
||||
"mcp_server._fetch_pr_comments",
|
||||
return_value=[
|
||||
_reviewer_lease_comment(
|
||||
pr_number,
|
||||
session_id=session_id,
|
||||
head_sha=head_sha,
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
# Issue-write tools are profile-gated (#69).
|
||||
ISSUE_WRITE_ENV = {
|
||||
"GITEA_ALLOWED_OPERATIONS": (
|
||||
@@ -101,17 +161,36 @@ ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
|
||||
|
||||
|
||||
def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
|
||||
import issue_lock_provenance
|
||||
|
||||
work_lease = {
|
||||
"operation_type": "author_issue_work",
|
||||
"issue_number": issue_number,
|
||||
"branch": branch_name,
|
||||
"claimant": {"username": "test-user", "profile": "test-author"},
|
||||
"expires_at": "2999-01-01T00:00:00Z",
|
||||
}
|
||||
record = {
|
||||
"issue_number": issue_number,
|
||||
"branch_name": branch_name,
|
||||
"remote": "dadeschools",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
"work_lease": work_lease,
|
||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
||||
tool="gitea_lock_issue",
|
||||
claimant=work_lease.get("claimant"),
|
||||
),
|
||||
}
|
||||
record.update(overrides)
|
||||
return record
|
||||
|
||||
|
||||
def _clear_duplicate_context_fetcher(*_args, **_kwargs):
|
||||
"""Default injectable duplicate-work context for lock/create_pr tests."""
|
||||
return [], [], {"status": "not_claimed"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Create Issue
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -166,13 +245,17 @@ class TestCreateIssue(unittest.TestCase):
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestCreatePR(unittest.TestCase):
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
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):
|
||||
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
|
||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||
@@ -187,13 +270,17 @@ class TestCreatePR(unittest.TestCase):
|
||||
self.assertEqual(payload["base"], "main")
|
||||
self.assertIn("Closes #123", payload["title"])
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
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):
|
||||
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
|
||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||
@@ -512,6 +599,30 @@ class TestViewPR(unittest.TestCase):
|
||||
class TestMergePR(unittest.TestCase):
|
||||
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path."""
|
||||
|
||||
def setUp(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
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.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 {
|
||||
"user": {"login": author},
|
||||
@@ -584,7 +695,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) -----------------------------------
|
||||
@@ -602,8 +714,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'.")
|
||||
@@ -631,8 +745,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:"))
|
||||
@@ -646,7 +762,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()
|
||||
@@ -657,7 +773,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()
|
||||
|
||||
@@ -772,9 +888,11 @@ class TestMergePR(unittest.TestCase):
|
||||
pr_number=8, confirmation=self._confirm(8),
|
||||
expected_head_sha="deadbeef", remote="prgs")
|
||||
self.assertFalse(r["performed"])
|
||||
self.assertIn(
|
||||
"expected head SHA does not match current PR head (fail closed)",
|
||||
r["reasons"])
|
||||
self.assertTrue(any(
|
||||
"expected head SHA does not match current PR head (fail closed)" in reason
|
||||
or "PR head changed during lease" in reason
|
||||
for reason in r["reasons"]
|
||||
))
|
||||
self._assert_no_merge_call(mock_api)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@@ -789,7 +907,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)",
|
||||
@@ -802,7 +921,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()
|
||||
@@ -820,7 +939,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)
|
||||
@@ -837,7 +958,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"])
|
||||
blob = repr(r)
|
||||
self.assertIn("[REDACTED]", blob)
|
||||
@@ -855,7 +978,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"]))
|
||||
@@ -876,7 +1001,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"]))
|
||||
@@ -986,7 +1113,7 @@ class TestReviewPR(unittest.TestCase):
|
||||
]
|
||||
from mcp_server import init_review_decision_lock
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
gitea_mark_final_review_decision(1, "approve", remote="prgs")
|
||||
gitea_mark_final_review_decision(1, "approve", remote="prgs", expected_head_sha="abc123")
|
||||
result = gitea_review_pr(
|
||||
pr_number=1,
|
||||
event="APPROVE",
|
||||
@@ -1653,7 +1780,26 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
|
||||
}
|
||||
|
||||
def setUp(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
self._lease_patch = _install_owned_reviewer_lease(
|
||||
self.PR, head_sha=self.SHA,
|
||||
)
|
||||
self._lease_patch.start()
|
||||
self._auth_identity_patch = patch(
|
||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
||||
)
|
||||
self._auth_identity_patch.start()
|
||||
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)
|
||||
|
||||
|
||||
def _env(self):
|
||||
return patch.dict(os.environ, {
|
||||
@@ -1748,8 +1894,32 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
"""Gated review-mutation tool (#15)."""
|
||||
|
||||
def setUp(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
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):
|
||||
return {
|
||||
@@ -1909,10 +2079,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):
|
||||
@@ -1928,12 +2099,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,
|
||||
@@ -1976,20 +2150,22 @@ 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"])
|
||||
self.assertIn(
|
||||
"expected head SHA does not match current PR head (fail closed)",
|
||||
r["reasons"])
|
||||
self.assertTrue(any(
|
||||
"expected head SHA does not match current PR head (fail closed)" in reason
|
||||
or "PR head changed during lease" in reason
|
||||
for reason in r["reasons"]
|
||||
))
|
||||
self._assert_no_mutation(mock_api)
|
||||
|
||||
def test_head_sha_match_allows(self):
|
||||
@@ -2032,7 +2208,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,
|
||||
@@ -2052,9 +2228,8 @@ 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(5, "approve", remote="prgs")
|
||||
r = gitea_submit_pr_review(
|
||||
pr_number=5, action="approve", remote="prgs",
|
||||
pr_number=8, action="approve", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
)
|
||||
self.assertFalse(r["performed"])
|
||||
@@ -2142,7 +2317,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
|
||||
def test_mark_final_decision_rejects_remote_mismatch(self):
|
||||
init_review_decision_lock("prgs", "review_pr")
|
||||
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"]))
|
||||
|
||||
@@ -2154,28 +2329,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"])
|
||||
@@ -2187,7 +2364,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(
|
||||
@@ -2273,6 +2450,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
||||
|
||||
def test_merge_pr_with_closes_removes_label(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
|
||||
lease_patch.start()
|
||||
self.addCleanup(lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
@@ -2307,6 +2491,13 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
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")
|
||||
lease_patch.start()
|
||||
self.addCleanup(lease_patch.stop)
|
||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
||||
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
@@ -2988,10 +3179,12 @@ class TestVerifyMutationAuthority(unittest.TestCase):
|
||||
# profile; the active profile resolves as reviewer — side-channel
|
||||
# override rejected even with a matching in-process authority.
|
||||
self._authority()
|
||||
with patch.dict(os.environ,
|
||||
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
with patch("mcp_server.gitea_config.is_runtime_switching_enabled",
|
||||
return_value=False):
|
||||
with patch.dict(os.environ,
|
||||
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
self.assertIn("side-channel override rejected", str(ctx.exception))
|
||||
|
||||
def test_foreign_pid_authority_is_not_trusted(self):
|
||||
@@ -3044,8 +3237,14 @@ class TestIssueLocking(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True)
|
||||
self._env_patcher.start()
|
||||
self._dup_fetcher_patcher = patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
self.mock_dup_fetcher = self._dup_fetcher_patcher.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._dup_fetcher_patcher.stop()
|
||||
self._env_patcher.stop()
|
||||
if os.path.exists(ISSUE_LOCK_FILE):
|
||||
os.remove(ISSUE_LOCK_FILE)
|
||||
@@ -3054,10 +3253,8 @@ class TestIssueLocking(unittest.TestCase):
|
||||
"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_success(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [] # no open PRs
|
||||
def test_lock_issue_success(self, _auth, _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")
|
||||
@@ -3082,50 +3279,48 @@ class TestIssueLocking(unittest.TestCase):
|
||||
"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_reused_by_open_pr_branch(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [{
|
||||
def test_lock_issue_reused_by_open_pr_branch(self, _auth, _git_state):
|
||||
self.mock_dup_fetcher.return_value = ([{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/issue-196-boundary"},
|
||||
"title": "Some PR",
|
||||
"body": "No closes ref"
|
||||
}]
|
||||
"body": "No closes ref",
|
||||
}], [], {"status": "not_claimed"})
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already tied to an open PR", str(ctx.exception))
|
||||
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
|
||||
|
||||
@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_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [{
|
||||
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, _git_state):
|
||||
self.mock_dup_fetcher.return_value = ([{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/other-branch"},
|
||||
"title": "Some PR",
|
||||
"body": "fixes #196"
|
||||
}]
|
||||
"body": "fixes #196",
|
||||
}], [], {"status": "not_claimed"})
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already tied to an open PR", str(ctx.exception))
|
||||
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
|
||||
|
||||
@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_reused_by_remote_branch(self, _auth, mock_api, _git_state):
|
||||
mock_api.side_effect = [
|
||||
def test_lock_issue_reused_by_remote_branch(self, _auth, _git_state):
|
||||
self.mock_dup_fetcher.return_value = (
|
||||
[],
|
||||
[{"name": "feat/issue-196-existing-work"}],
|
||||
]
|
||||
["feat/issue-196-existing-work"],
|
||||
{"status": "not_claimed"},
|
||||
)
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already has matching branch", str(ctx.exception))
|
||||
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:
|
||||
@@ -3284,6 +3479,28 @@ class TestIssueLocking(unittest.TestCase):
|
||||
)
|
||||
self.assertIn("does not match locked worktree", 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_manual_lock_seed_blocked(self, _auth, _role):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(
|
||||
_sample_issue_lock(
|
||||
issue_number=447,
|
||||
branch_name="feat/issue-447-lock-provenance",
|
||||
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",
|
||||
)
|
||||
self.assertIn("lock provenance", str(ctx.exception).lower())
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
|
||||
Reference in New Issue
Block a user