fix: resolve conflicts for PR #421

Merge prgs/master; keep audit-readonly gate (#419) and explicit delete_branch
preflight task binding from master.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
2026-07-08 04:08:07 -04:00
co-authored by Claude Opus 4.8
46 changed files with 6588 additions and 375 deletions
+6 -3
View File
@@ -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",
+46 -7
View File
@@ -284,8 +284,40 @@ 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 mcp_server import init_review_decision_lock
from tests.test_mcp_server import _install_owned_reviewer_lease
import reviewer_pr_lease
# init_review_decision_lock clears any prior session lease (#407).
init_review_decision_lock("prgs", "review_pr")
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):
return {"user": {"login": author}, "state": state,
"head": {"sha": sha}, "mergeable": mergeable}
@@ -298,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",
@@ -334,7 +367,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",
@@ -343,12 +378,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
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(8, "approve", remote="prgs")
r = gitea_submit_pr_review(pr_number=8, action="approve",
body="LGTM", remote="prgs",
final_review_decision_ready=True)
from mcp_server import gitea_mark_final_review_decision
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)
+7 -5
View File
@@ -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")
+22 -10
View File
@@ -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)
+4
View File
@@ -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)",
+227
View File
@@ -0,0 +1,227 @@
"""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,
build_non_adoption_lock_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"])
class TestExplicitAdoptionProofFields(unittest.TestCase):
"""#477: explicit, citable adoption-proof fields for all outcomes."""
def _proof(self, assessment, branch):
return build_adoption_proof(
issue_number=420,
branch_name=branch,
assessment=assessment,
open_pr_checked=True,
competing_lock_checked=True,
lock_file_path="/tmp/example-lock.json",
lock_file_status="written",
)
def test_adopt_proof_exposes_explicit_fields(self):
assessment = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
)
proof = self._proof(assessment, REQ)
self.assertEqual(proof["adoption_decision"], "ADOPT")
self.assertTrue(proof["adopted"])
self.assertEqual(proof["adopted_branch"], REQ)
self.assertEqual(proof["adopted_branch_head"], "934688a")
self.assertEqual(proof["competing_branch_check"]["result"], "clear")
self.assertEqual(proof["competing_branch_check"]["competing_branches"], [])
self.assertIn("gitea_create_pr", proof["safe_next_action"])
self.assertIn("exactly matches", proof["matcher_summary"])
def test_block_proof_reports_competing_and_does_not_claim_adoption(self):
assessment = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": "feat/issue-420-rogue"}],
)
proof = self._proof(assessment, REQ)
self.assertEqual(proof["adoption_decision"], "BLOCK_COMPETING")
self.assertFalse(proof["adopted"])
self.assertIsNone(proof["adopted_branch"])
self.assertIsNone(proof["adopted_branch_head"])
self.assertEqual(proof["competing_branch_check"]["result"], "blocked")
self.assertIn(
"feat/issue-420-rogue",
proof["competing_branch_check"]["competing_branches"],
)
self.assertIn("fail closed", proof["safe_next_action"])
def test_no_match_proof_does_not_claim_adoption(self):
assessment = assess_own_branch_adoption(
issue_number=420,
requested_branch=REQ,
existing_branches=[{"name": "feat/issue-999-unrelated"}],
)
proof = self._proof(assessment, REQ)
self.assertEqual(proof["adoption_decision"], "NO_MATCH")
self.assertFalse(proof["adopted"])
self.assertIsNone(proof["adopted_branch"])
self.assertEqual(proof["competing_branch_check"]["result"], "clear")
def test_substring_collision_stays_boundary_safe(self):
# issue-42 must not adopt/claim against an issue-420 branch (#440/#477).
own = "feat/issue-42-widget"
assessment = assess_own_branch_adoption(
issue_number=42,
requested_branch=own,
existing_branches=[
{"name": own, "commit_sha": "abc1234"},
{"name": "feat/issue-420-server-code-parity"},
],
)
proof = self._proof(assessment, own)
self.assertEqual(proof["adoption_decision"], "ADOPT")
self.assertEqual(proof["adopted_branch"], own)
self.assertEqual(proof["competing_branch_check"]["competing_branches"], [])
def test_non_adoption_lock_proof_is_adoption_free(self):
proof = build_non_adoption_lock_proof(
issue_number=196, branch_name="feat/issue-196-mutations"
)
self.assertEqual(proof["adoption_decision"], "NO_MATCH")
self.assertFalse(proof["adopted"])
self.assertIsNone(proof["adopted_branch"])
self.assertIsNone(proof["adopted_branch_head"])
self.assertEqual(proof["competing_branch_check"]["result"], "clear")
self.assertIn("no adoption", proof["safe_next_action"].lower())
if __name__ == "__main__":
unittest.main()
+280
View File
@@ -0,0 +1,280 @@
"""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_approved_stacked_base_survives_round_trip(self):
# #484: the approved stacked base recorded on the lock must persist so
# gitea_create_pr can validate the non-master base at PR time.
record = _lock_record(
issue_number=482,
branch_name="feat/issue-482-skip-stale-request-changes-pr",
approved_stacked_base={
"branch": "feat/issue-478-mcp-menu-shell",
"pr_number": 479,
"verified_open": True,
},
)
path = ils.lock_file_path(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
issue_number=482,
)
ils.save_lock_file(path, record)
stored = ils.read_lock_file(path)
self.assertEqual(stored["approved_stacked_base"]["branch"], "feat/issue-478-mcp-menu-shell")
self.assertEqual(stored["approved_stacked_base"]["pr_number"], 479)
self.assertTrue(stored["approved_stacked_base"]["verified_open"])
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()
+53
View File
@@ -72,6 +72,59 @@ class TestIssueLockWorktreeAssessment(unittest.TestCase):
self.assertTrue(result["proven"])
class TestStackedBaseEquivalence(unittest.TestCase):
"""extra_bases (an approved stacked base) can anchor base-equivalence (#484)."""
def _git(self, *args):
import subprocess
subprocess.run(
["git", "-C", self.repo, *args],
check=True,
capture_output=True,
text=True,
)
def setUp(self):
import subprocess
import tempfile
self.tmp = tempfile.TemporaryDirectory()
self.repo = self.tmp.name
subprocess.run(["git", "init", "-q", self.repo], check=True, capture_output=True)
self._git("config", "user.email", "t@t")
self._git("config", "user.name", "t")
self._git("commit", "--allow-empty", "-q", "-m", "base")
# Rename the default branch away from master/main/dev so no *base* branch
# exists at HEAD — otherwise HEAD would be base-equivalent for free.
self._git("branch", "-m", "trunk")
# Create a non-master "dependency" branch at the same commit, then a
# feature branch off it — mirrors a stacked worktree.
self._git("branch", "feat/issue-100-dep")
self._git("checkout", "-q", "-b", "feat/issue-101-stacked")
def tearDown(self):
self.tmp.cleanup()
def test_stacked_base_not_equivalent_without_extra_bases(self):
state = issue_lock_worktree.read_worktree_git_state(self.repo)
# HEAD does not match master/main/dev, so base-equivalence is False.
self.assertFalse(state["base_equivalent"])
def test_stacked_base_equivalent_with_extra_bases(self):
state = issue_lock_worktree.read_worktree_git_state(
self.repo, extra_bases=("feat/issue-100-dep",)
)
self.assertTrue(state["base_equivalent"])
self.assertEqual(state["base_branch"], "feat/issue-100-dep")
def test_unrelated_extra_base_does_not_anchor(self):
state = issue_lock_worktree.read_worktree_git_state(
self.repo, extra_bases=("feat/does-not-exist",)
)
self.assertFalse(state["base_equivalent"])
class TestIssueLockWorktreeResolution(unittest.TestCase):
def test_explicit_path_wins(self):
resolved = issue_lock_worktree.resolve_author_worktree_path(
+42 -21
View File
@@ -1,5 +1,4 @@
"""Tests for early duplicate-work detection (#400)."""
import json
import os
import sys
import tempfile
@@ -9,6 +8,8 @@ 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 (
@@ -122,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):
@@ -140,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"},
@@ -172,12 +177,27 @@ class TestMcpDuplicateRecheck(unittest.TestCase):
self._dir.cleanup()
def _write_lock(self, issue_number=400, branch="feat/issue-400-x"):
with open(self.lock_path, "w", encoding="utf-8") as fh:
json.dump({
"issue_number": issue_number,
"branch_name": branch,
"remote": "prgs",
}, fh)
worktree_path = os.path.realpath(os.getcwd())
work_lease = {
"operation_type": "author_issue_work",
"issue_number": issue_number,
"branch": branch,
"claimant": {"username": "test-user", "profile": "test-author"},
"expires_at": "2999-01-01T00:00:00Z",
}
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={
@@ -241,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"))
+8 -4
View File
@@ -122,15 +122,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")
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(
+6
View File
@@ -221,6 +221,12 @@ def test_validation_failure_history_verifier_exported():
assert callable(assess_validation_failure_history_report)
def test_validation_cwd_proof_verifier_exported():
from review_proofs import assess_validation_cwd_proof_report
assert callable(assess_validation_cwd_proof_report)
def test_prior_blocker_skip_verifier_exported():
from review_proofs import assess_prior_blocker_skip_proof
+558 -143
View File
File diff suppressed because it is too large Load Diff
+59
View File
@@ -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()
+6 -2
View File
@@ -230,7 +230,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_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",
@@ -272,7 +274,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_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",
@@ -0,0 +1,76 @@
"""Regression tests for non-list API payloads on PR/issue comment listing (#485)."""
import os
import sys
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
from mcp_server import ( # noqa: E402
_list_pr_lease_comments,
gitea_list_issue_comments,
)
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
AUTHOR_ENV = {
"GITEA_PROFILE_NAME": "gitea-author",
"GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment",
}
class TestPrLeaseCommentsNonListGuard(unittest.TestCase):
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_list_pr_lease_comments_non_list_payload_returns_empty(self, _auth, mock_api):
mock_api.return_value = {"message": "Unauthorized"}
result = _list_pr_lease_comments(
12, remote="prgs", host=None, org=None, repo=None)
self.assertEqual(result, [])
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_list_pr_lease_comments_none_returns_empty(self, _auth, mock_api):
mock_api.return_value = None
result = _list_pr_lease_comments(
12, remote="prgs", host=None, org=None, repo=None)
self.assertEqual(result, [])
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_list_pr_lease_comments_list_payload_unchanged(self, _auth, mock_api):
comment = {"id": 7, "body": "<!-- mcp-review-lease:v1 -->"}
mock_api.return_value = [comment]
result = _list_pr_lease_comments(
12, remote="prgs", host=None, org=None, repo=None, limit=5)
self.assertEqual(result, [comment])
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_list_issue_comments_non_list_payload_returns_empty(self, _auth, mock_api):
mock_api.return_value = {"message": "Unauthorized"}
with patch.dict(os.environ, AUTHOR_ENV, clear=True):
result = gitea_list_issue_comments(issue_number=9, remote="prgs")
self.assertTrue(result["success"])
self.assertEqual(result["comments"], [])
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_list_issue_comments_list_payload_unchanged(self, _auth, mock_api):
mock_api.return_value = [
{
"id": 101,
"user": {"login": "alice"},
"body": "hello",
"created_at": "2026-07-03T00:00:00Z",
"updated_at": "2026-07-03T01:00:00Z",
}
]
with patch.dict(os.environ, AUTHOR_ENV, clear=True):
result = gitea_list_issue_comments(issue_number=9, remote="prgs")
self.assertTrue(result["success"])
self.assertEqual(len(result["comments"]), 1)
self.assertEqual(result["comments"][0]["author"], "alice")
if __name__ == "__main__":
unittest.main()
+30 -9
View File
@@ -128,18 +128,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"},
@@ -148,17 +159,27 @@ 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
init_review_decision_lock("prgs", "review_pr")
gitea_mark_final_review_decision(1, "approve", remote="prgs")
result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs", final_review_decision_ready=True)
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")
_seed_ready_review_decision(1, "approve", sha=head_sha, remote="prgs")
lease_patch = _install_owned_reviewer_lease(
1, head_sha=head_sha, session_id="inventory-review-lease",
)
lease_patch.start()
self.addCleanup(lease_patch.stop)
self.addCleanup(reviewer_pr_lease.clear_session_lease)
result = gitea_review_pr(
pr_number=1, event="APPROVE", remote="prgs",
final_review_decision_ready=True,
)
self.assertTrue(result["success"])
self.assertIn("=== PR Queue Inventory ===", result["message"])
self.assertIn("Repository:", result["message"])
+207
View File
@@ -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()
+106
View File
@@ -0,0 +1,106 @@
"""#469: capability preflight survives interleaved read-only whoami calls."""
import os
import unittest
import gitea_mcp_server as mcp_server
class TestPreflightReadSurvival(unittest.TestCase):
def setUp(self):
self.orig_whoami = mcp_server._preflight_whoami_called
self.orig_capability = mcp_server._preflight_capability_called
self.orig_whoami_violation = mcp_server._preflight_whoami_violation
self.orig_capability_violation = mcp_server._preflight_capability_violation
self.orig_resolved_role = mcp_server._preflight_resolved_role
self.orig_resolved_task = mcp_server._preflight_resolved_task
self.orig_process_start = mcp_server._process_start_porcelain
self.orig_whoami_baseline = mcp_server._preflight_whoami_baseline_porcelain
self.orig_capability_baseline = mcp_server._preflight_capability_baseline_porcelain
for key in ("GITEA_TEST_FORCE_DIRTY", "GITEA_TEST_PORCELAIN"):
if key in os.environ:
del os.environ[key]
os.environ["GITEA_TEST_PORCELAIN"] = ""
mcp_server._preflight_whoami_called = False
mcp_server._preflight_capability_called = False
mcp_server._preflight_whoami_violation = False
mcp_server._preflight_capability_violation = False
mcp_server._preflight_resolved_role = None
mcp_server._preflight_resolved_task = None
mcp_server._process_start_porcelain = ""
mcp_server._preflight_whoami_baseline_porcelain = None
mcp_server._preflight_capability_baseline_porcelain = None
def tearDown(self):
mcp_server._preflight_whoami_called = self.orig_whoami
mcp_server._preflight_capability_called = self.orig_capability
mcp_server._preflight_whoami_violation = self.orig_whoami_violation
mcp_server._preflight_capability_violation = self.orig_capability_violation
mcp_server._preflight_resolved_role = self.orig_resolved_role
mcp_server._preflight_resolved_task = self.orig_resolved_task
mcp_server._process_start_porcelain = self.orig_process_start
mcp_server._preflight_whoami_baseline_porcelain = self.orig_whoami_baseline
mcp_server._preflight_capability_baseline_porcelain = self.orig_capability_baseline
for key in ("GITEA_TEST_FORCE_DIRTY", "GITEA_TEST_PORCELAIN"):
if key in os.environ:
del os.environ[key]
def test_interleaved_whoami_preserves_capability(self):
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check(
"capability", resolved_role="reconciler", resolved_task="close_pr"
)
self.assertTrue(mcp_server._preflight_capability_called)
self.assertEqual(mcp_server._preflight_resolved_task, "close_pr")
mcp_server.record_preflight_check("whoami")
self.assertTrue(mcp_server._preflight_capability_called)
self.assertEqual(mcp_server._preflight_resolved_task, "close_pr")
mcp_server.verify_preflight_purity(task="close_pr")
self.assertFalse(mcp_server._preflight_capability_called)
def test_missing_capability_still_fails_closed(self):
mcp_server.record_preflight_check("whoami")
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity(task="close_pr")
self.assertIn("has not been resolved", str(ctx.exception))
def test_task_mismatch_fails_closed(self):
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check(
"capability", resolved_role="author", resolved_task="create_issue"
)
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity(task="close_pr")
self.assertIn("task mismatch", str(ctx.exception))
def test_capability_consumed_after_mutation_gate(self):
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check(
"capability", resolved_role="author", resolved_task="create_issue"
)
mcp_server.verify_preflight_purity(task="create_issue")
with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity(task="create_issue")
self.assertIn("has not been resolved", str(ctx.exception))
def test_whoami_recovery_after_violation_clears_capability(self):
os.environ["GITEA_TEST_FORCE_DIRTY"] = "1"
mcp_server.record_preflight_check("whoami")
self.assertTrue(mcp_server._preflight_whoami_violation)
del os.environ["GITEA_TEST_FORCE_DIRTY"]
os.environ["GITEA_TEST_PORCELAIN"] = ""
mcp_server.record_preflight_check("whoami")
self.assertFalse(mcp_server._preflight_whoami_violation)
self.assertFalse(mcp_server._preflight_capability_called)
mcp_server.record_preflight_check(
"capability", resolved_role="reviewer", resolved_task="review_pr"
)
mcp_server.verify_preflight_purity(task="review_pr")
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,89 @@
"""Reconciler close_pr must not require author branches/ worktree (#468)."""
import os
import sys
import unittest
from pathlib import Path
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import gitea_mcp_server as srv
FAKE_AUTH = "token test"
CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3])
RECONCILER_PROFILE = {
"profile_name": "prgs-reconciler",
"allowed_operations": ["gitea.read", "gitea.pr.close", "gitea.pr.comment"],
"forbidden_operations": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit",
],
"audit_label": "prgs-reconciler",
}
class TestReconcilerCloseWorkspaceGuard(unittest.TestCase):
def setUp(self):
srv._preflight_whoami_called = True
srv._preflight_capability_called = True
srv._preflight_whoami_violation = False
srv._preflight_capability_violation = False
self._orig_in_test = srv._preflight_in_test_mode
srv._preflight_in_test_mode = lambda: False
def tearDown(self):
srv._preflight_in_test_mode = self._orig_in_test
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
@patch("gitea_mcp_server._namespace_mutation_block", return_value=None)
@patch("gitea_mcp_server.get_profile", return_value=RECONCILER_PROFILE)
@patch("gitea_mcp_server.api_request")
def test_reconciler_close_pr_from_control_checkout_succeeds(
self, mock_api, _profile, _ns, _auth
):
srv._preflight_resolved_role = "reconciler"
mock_api.return_value = {
"number": 414,
"title": "old",
"body": "",
"state": "closed",
"html_url": "https://gitea.example.com/pulls/414",
}
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with patch.dict(os.environ, {}, clear=False):
os.environ.pop("GITEA_AUTHOR_WORKTREE", None)
os.environ.pop("GITEA_ACTIVE_WORKTREE", None)
result = srv.gitea_edit_pr(414, state="closed", remote="prgs")
self.assertTrue(result["success"])
self.assertEqual(result["state"], "closed")
mock_api.assert_called_once()
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
@patch("gitea_mcp_server._profile_permission_block", return_value=None)
@patch("gitea_mcp_server._namespace_mutation_block", return_value=None)
@patch(
"gitea_mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
)
@patch("gitea_mcp_server.api_get_all", return_value=[])
@patch(
"gitea_mcp_server.issue_lock_worktree.read_worktree_git_state",
return_value={"current_branch": "master"},
)
def test_author_create_issue_still_blocked_on_control_checkout(
self, _git, _get_all, _role, _ns, _prof, _auth
):
srv._preflight_resolved_role = "author"
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with self.assertRaises(RuntimeError) as ctx:
srv.gitea_create_issue(title="Test", body="body")
self.assertIn("stable control checkout", str(ctx.exception))
if __name__ == "__main__":
unittest.main()
+16
View File
@@ -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)
+6 -2
View File
@@ -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"
+193
View File
@@ -0,0 +1,193 @@
"""Tests for per-PR reviewer leases (#407)."""
import sys
import unittest
from datetime import datetime, timedelta, timezone
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import reviewer_pr_lease as leases
def _lease_comment(
pr_number: int,
session_id: str,
*,
phase: str = "claimed",
minutes_ago: int = 0,
candidate_head: str = "a" * 40,
) -> dict:
now = datetime.now(timezone.utc) - timedelta(minutes=minutes_ago)
body = leases.format_lease_body(
repo="Scaled-Tech-Consulting/Gitea-Tools",
pr_number=pr_number,
issue_number=295,
reviewer_identity="rev1",
profile="prgs-reviewer",
session_id=session_id,
worktree="branches/review-pr382",
phase=phase,
candidate_head=candidate_head,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=now,
)
return {"id": 1, "body": body, "user": {"login": "rev1"}}
class TestReviewerLeaseAcquire(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_two_reviewers_cannot_lease_same_pr(self):
comments = [_lease_comment(382, "session-a")]
result = leases.assess_acquire_lease(
comments,
pr_number=382,
reviewer_identity="rev2",
profile="prgs-reviewer",
session_id="session-b",
repo="Scaled-Tech-Consulting/Gitea-Tools",
issue_number=295,
worktree="branches/review-pr382-b",
candidate_head="c" * 40,
target_branch="master",
target_branch_sha="d" * 40,
)
self.assertFalse(result["acquire_allowed"])
self.assertTrue(any("already has active" in r for r in result["reasons"]))
def test_two_reviewers_can_lease_different_prs(self):
comments = [_lease_comment(382, "session-a")]
result = leases.assess_acquire_lease(
comments,
pr_number=383,
reviewer_identity="rev2",
profile="prgs-reviewer",
session_id="session-b",
repo="Scaled-Tech-Consulting/Gitea-Tools",
issue_number=296,
worktree="branches/review-pr383",
candidate_head="c" * 40,
target_branch="master",
target_branch_sha="d" * 40,
)
self.assertTrue(result["acquire_allowed"])
self.assertIsNotNone(result["lease_body"])
class TestReviewerLeaseFreshness(unittest.TestCase):
def test_stale_warning_after_30_minutes(self):
lease = leases.parse_lease_comment(
_lease_comment(382, "session-a", minutes_ago=35)["body"]
)
self.assertEqual(
leases.classify_lease_freshness(lease),
"stale_warning",
)
def test_reclaimable_after_60_minutes(self):
lease = leases.parse_lease_comment(
_lease_comment(382, "session-a", minutes_ago=65)["body"]
)
self.assertEqual(
leases.classify_lease_freshness(lease),
"reclaimable",
)
class TestReviewerLeaseMutationGate(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_reviewer_without_lease_cannot_mutate(self):
head = "f" * 40
comments = [_lease_comment(382, "other-session", candidate_head=head)]
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertTrue(result["block"])
def test_owned_lease_allows_mutation(self):
head = "f" * 40
comments = [_lease_comment(382, "my-session", candidate_head=head)]
leases.record_session_lease({
"pr_number": 382,
"session_id": "my-session",
"candidate_head": head,
"target_branch": "master",
})
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertFalse(result["block"])
def test_head_change_invalidates_lease(self):
reviewed = "f" * 40
live = "e" * 40
comments = [_lease_comment(382, "my-session", candidate_head=reviewed)]
leases.record_session_lease({
"pr_number": 382,
"session_id": "my-session",
"candidate_head": reviewed,
})
result = leases.assess_mutation_lease_gate(
pr_number=382,
comments=comments,
reviewer_identity="rev1",
session_id="my-session",
mutation="merge",
live_head_sha=live,
pinned_head_sha=reviewed,
)
self.assertTrue(result["block"])
self.assertTrue(any("head" in r.lower() for r in result["reasons"]))
class TestReviewerLeaseMcpGate(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
patch("mcp_server.verify_preflight_purity").start()
patch("gitea_audit.audit_enabled", return_value=False).start()
mcp_server = __import__("mcp_server")
mcp_server._IDENTITY_CACHE.clear()
mcp_server.init_review_decision_lock("prgs", "review_pr")
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", "reviewer")
def tearDown(self):
patch.stopall()
leases.clear_session_lease()
def test_reviewer_pr_lease_gate_helper_blocks_without_session(self):
import mcp_server
head = "a" * 40
with patch("mcp_server._fetch_pr_comments", return_value=[]):
reasons = mcp_server._reviewer_pr_lease_gate(
pr_number=382,
remote="prgs",
host=None,
org=None,
repo=None,
mutation="approve",
live_head_sha=head,
pinned_head_sha=head,
)
self.assertTrue(any("lease" in r.lower() for r in reasons))
if __name__ == "__main__":
unittest.main()
+135
View File
@@ -0,0 +1,135 @@
"""Tests for validation cwd/HEAD proof verifier (#398)."""
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 reviewer_validation_cwd_proof import assess_validation_cwd_proof_report # noqa: E402
ROOT = "/Users/jasonwalker/Development/Gitea-Tools"
WORKTREE = f"{ROOT}/branches/review-feat-issue-398"
HEAD = "f5953549aad5e822f14f52d3ea3c6d7990109384"
def _proof_backed_report() -> str:
return "\n".join([
f"Candidate head SHA: {HEAD}",
f"pwd: {WORKTREE}",
f"git rev-parse HEAD: {HEAD}",
"git status --short --branch: ## feat/issue-398...prgs/master",
f"Validation command: cd {WORKTREE} && venv/bin/python -m pytest tests/ -q",
"Result: 1497 passed, 6 skipped",
])
class TestValidationCwdProof(unittest.TestCase):
def test_no_validation_claim_passes(self):
result = assess_validation_cwd_proof_report("Review decision: approve")
self.assertTrue(result["proven"])
def test_missing_cwd_proof_fails(self):
result = assess_validation_cwd_proof_report(
f"Validation command: pytest tests/\nCandidate head SHA: {HEAD}",
validation_session={"validation_ran": True, "expected_head_sha": HEAD},
)
self.assertFalse(result["proven"])
self.assertTrue(result["block"])
def test_main_checkout_cwd_blocks(self):
result = assess_validation_cwd_proof_report(
"\n".join([
f"Candidate head SHA: {HEAD}",
f"pwd: {ROOT}",
f"git rev-parse HEAD: {HEAD}",
"git status --short --branch: ## master",
"Validation command: pytest tests/ -q",
]),
validation_session={"validation_ran": True, "expected_head_sha": HEAD},
project_root=ROOT,
)
self.assertFalse(result["proven"])
self.assertTrue(result["violations"])
def test_wrong_head_blocks(self):
wrong = "a" * 40
result = assess_validation_cwd_proof_report(
"\n".join([
f"Candidate head SHA: {HEAD}",
f"pwd: {WORKTREE}",
f"git rev-parse HEAD: {wrong}",
"git status --short --branch: clean",
f"Validation command: cd {WORKTREE} && pytest -q",
]),
validation_session={"validation_ran": True, "expected_head_sha": HEAD},
project_root=ROOT,
)
self.assertFalse(result["proven"])
self.assertTrue(result["violations"])
def test_fully_proof_backed_passes(self):
result = assess_validation_cwd_proof_report(
_proof_backed_report(),
validation_session={"validation_ran": True, "expected_head_sha": HEAD},
project_root=ROOT,
)
self.assertTrue(result["proven"], result["reasons"])
def test_baseline_without_cwd_fails(self):
result = assess_validation_cwd_proof_report(
"\n".join([
_proof_backed_report(),
"Baseline validation command: pytest tests/ -q",
]),
validation_session={
"validation_ran": True,
"expected_head_sha": HEAD,
"baseline_validation_ran": True,
},
project_root=ROOT,
)
self.assertFalse(result["proven"])
self.assertTrue(
any("baseline" in r.lower() for r in result["reasons"])
)
def test_baseline_with_full_proof_passes(self):
result = assess_validation_cwd_proof_report(
"\n".join([
_proof_backed_report(),
f"Baseline worktree: {ROOT}/branches/baseline-master-pr376",
f"Baseline target SHA: {HEAD}",
f"Baseline validation command: cd {ROOT}/branches/baseline-master-pr376 && pytest -q",
]),
validation_session={
"validation_ran": True,
"expected_head_sha": HEAD,
"baseline_validation_ran": True,
},
project_root=ROOT,
)
self.assertTrue(result["proven"], result["reasons"])
def test_final_report_validator_integration(self):
result = assess_final_report_validator(
"Validation command: pytest tests/ -q",
"review_pr",
validation_session={"validation_ran": True},
)
self.assertTrue(result["blocked"] or result["downgraded"])
self.assertTrue(
any(
f.get("rule_id") == "reviewer.validation_cwd_proof"
for f in result.get("findings") or []
)
)
def test_exported_from_review_proofs(self):
from review_proofs import assess_validation_cwd_proof_report as exported
self.assertTrue(callable(exported))
if __name__ == "__main__":
unittest.main()
+185
View File
@@ -0,0 +1,185 @@
"""Unit tests for stacked-PR base policy (#484)."""
import unittest
import stacked_pr_support as sps
def _pr(number, branch, state="open"):
return {"number": number, "state": state, "head": {"ref": branch}}
OPEN_PRS = [
_pr(479, "feat/issue-478-mcp-menu-shell"),
_pr(481, "feat/issue-477-lock-adoption-proof"),
]
# Motivating case (#482 stacked on #479 / #478).
STACKED_BODY = (
"Closes #482.\n\n"
"Stacked on PR #479 / issue #478.\n"
"Base branch: feat/issue-478-mcp-menu-shell\n"
"Head branch: feat/issue-482-skip-stale-request-changes-pr\n"
"Do not merge before PR #479 lands."
)
class TestIsBaseBranch(unittest.TestCase):
def test_master_main_dev_are_base(self):
for b in ("master", "main", "dev"):
self.assertTrue(sps.is_base_branch(b))
def test_feature_branch_is_not_base(self):
self.assertFalse(sps.is_base_branch("feat/issue-478-mcp-menu-shell"))
class TestStackedBaseDeclaration(unittest.TestCase):
def test_no_declaration_is_normal_path(self):
out = sps.assess_stacked_base_declaration(
stacked_base_branch=None, stacked_base_pr=None, open_prs=OPEN_PRS
)
self.assertFalse(out["block"])
self.assertFalse(out["declared"])
self.assertIsNone(out["approved"])
def test_valid_open_pr_base_is_approved(self):
out = sps.assess_stacked_base_declaration(
stacked_base_branch="feat/issue-478-mcp-menu-shell",
stacked_base_pr=479,
open_prs=OPEN_PRS,
)
self.assertFalse(out["block"])
self.assertEqual(out["approved"]["branch"], "feat/issue-478-mcp-menu-shell")
self.assertEqual(out["approved"]["pr_number"], 479)
self.assertTrue(out["approved"]["verified_open"])
def test_missing_pr_number_blocks(self):
out = sps.assess_stacked_base_declaration(
stacked_base_branch="feat/issue-478-mcp-menu-shell",
stacked_base_pr=None,
open_prs=OPEN_PRS,
)
self.assertTrue(out["block"])
self.assertIn("without stacked_base_pr", out["reasons"][0])
def test_arbitrary_branch_with_no_open_pr_blocks(self):
out = sps.assess_stacked_base_declaration(
stacked_base_branch="feat/random-unrelated-branch",
stacked_base_pr=999,
open_prs=OPEN_PRS,
)
self.assertTrue(out["block"])
self.assertIn("does not correspond to any OPEN pull request", out["reasons"][0])
def test_stale_merged_base_blocks(self):
merged = [_pr(479, "feat/issue-478-mcp-menu-shell", state="closed")]
out = sps.assess_stacked_base_declaration(
stacked_base_branch="feat/issue-478-mcp-menu-shell",
stacked_base_pr=479,
open_prs=merged,
)
self.assertTrue(out["block"])
self.assertIsNone(out["approved"])
def test_pr_number_mismatch_blocks(self):
out = sps.assess_stacked_base_declaration(
stacked_base_branch="feat/issue-478-mcp-menu-shell",
stacked_base_pr=481, # wrong PR for this branch
open_prs=OPEN_PRS,
)
self.assertTrue(out["block"])
self.assertIn("does not match the open", out["reasons"][0])
def test_declaring_a_base_branch_blocks(self):
out = sps.assess_stacked_base_declaration(
stacked_base_branch="master", stacked_base_pr=1, open_prs=OPEN_PRS
)
self.assertTrue(out["block"])
self.assertIn("already a normal base branch", out["reasons"][0])
class TestStackedPrBody(unittest.TestCase):
def test_complete_body_has_no_missing_fields(self):
missing = sps.assess_stacked_pr_body(
STACKED_BODY, base_branch="feat/issue-478-mcp-menu-shell", pr_number=479
)
self.assertEqual(missing, [])
def test_missing_all_fields(self):
missing = sps.assess_stacked_pr_body(
"just some text", base_branch="feat/issue-478-mcp-menu-shell", pr_number=479
)
self.assertEqual(len(missing), 3)
def test_missing_merge_ordering_only(self):
body = "Base branch feat/issue-478-mcp-menu-shell for PR #479"
missing = sps.assess_stacked_pr_body(
body, base_branch="feat/issue-478-mcp-menu-shell", pr_number=479
)
self.assertEqual(len(missing), 1)
self.assertIn("merge-ordering", missing[0])
class TestCreatePrBase(unittest.TestCase):
APPROVED = {"branch": "feat/issue-478-mcp-menu-shell", "pr_number": 479, "verified_open": True}
def test_master_base_passes_without_stacked_metadata(self):
out = sps.assess_create_pr_base(
base="master", approved_stacked_base=None, body="Closes #1", open_prs=[]
)
self.assertFalse(out["block"])
self.assertFalse(out["stacked"])
def test_non_base_without_approval_blocks(self):
out = sps.assess_create_pr_base(
base="feat/issue-478-mcp-menu-shell",
approved_stacked_base=None,
body=STACKED_BODY,
open_prs=OPEN_PRS,
)
self.assertTrue(out["block"])
self.assertIn("no approved stacked base", out["reasons"][0])
def test_non_base_mismatched_approval_blocks(self):
out = sps.assess_create_pr_base(
base="feat/some-other-branch",
approved_stacked_base=self.APPROVED,
body=STACKED_BODY,
open_prs=OPEN_PRS,
)
self.assertTrue(out["block"])
self.assertIn("does not match the issue lock's approved stacked base", out["reasons"][0])
def test_approved_base_with_good_body_passes(self):
out = sps.assess_create_pr_base(
base="feat/issue-478-mcp-menu-shell",
approved_stacked_base=self.APPROVED,
body=STACKED_BODY,
open_prs=OPEN_PRS,
)
self.assertFalse(out["block"])
self.assertTrue(out["stacked"])
self.assertEqual(out["stacked_base_pr"], 479)
def test_approved_base_now_stale_blocks(self):
out = sps.assess_create_pr_base(
base="feat/issue-478-mcp-menu-shell",
approved_stacked_base=self.APPROVED,
body=STACKED_BODY,
open_prs=[_pr(479, "feat/issue-478-mcp-menu-shell", state="merged")],
)
self.assertTrue(out["block"])
self.assertIn("no longer corresponds to an OPEN", out["reasons"][0])
def test_approved_base_with_incomplete_body_blocks(self):
out = sps.assess_create_pr_base(
base="feat/issue-478-mcp-menu-shell",
approved_stacked_base=self.APPROVED,
body="Closes #482 only",
open_prs=OPEN_PRS,
)
self.assertTrue(out["block"])
self.assertIn("must document the stack", out["reasons"][0])
if __name__ == "__main__":
unittest.main()
+18 -11
View File
@@ -12,6 +12,8 @@ from unittest.mock import patch
import mcp_server
HEAD_SHA = "a" * 40
def _lock(mutations=None, correction=False):
return {
@@ -127,10 +129,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)
@@ -139,8 +151,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"]),
@@ -150,16 +161,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):
@@ -167,8 +176,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"]),
@@ -178,8 +186,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"))
+198
View File
@@ -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()
+153
View File
@@ -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
View File
@@ -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):