feat(mcp): bind issue lock to author scratch worktree (#249)
gitea_lock_issue and gitea_create_pr accept worktree_path so lock preconditions validate the caller's scratch clone instead of the shared MCP server CWD. Lock records store the validated path; PR creation fails closed when the declared path does not match. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Claude Opus 4.8
@@ -0,0 +1,99 @@
|
||||
"""Tests for issue-lock worktree validation (#249)."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import issue_lock_worktree # noqa: E402
|
||||
|
||||
|
||||
class TestIssueLockWorktreeAssessment(unittest.TestCase):
|
||||
def test_clean_base_branch_passes(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="master",
|
||||
porcelain_status="",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
def test_dirty_tracked_files_fail(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="master",
|
||||
porcelain_status=" M gitea_mcp_server.py\n",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("tracked file edits exist before issue lock", result["reasons"][0])
|
||||
|
||||
def test_feature_branch_fails(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="feat/issue-243-forbidden-git-gaps",
|
||||
porcelain_status="",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("issue lock must be taken from base branch", result["reasons"][0])
|
||||
|
||||
def test_untracked_files_do_not_block(self):
|
||||
result = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path="/scratch/wt",
|
||||
current_branch="main",
|
||||
porcelain_status="?? notes.txt\n",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
|
||||
class TestIssueLockWorktreeResolution(unittest.TestCase):
|
||||
def test_explicit_path_wins(self):
|
||||
resolved = issue_lock_worktree.resolve_author_worktree_path(
|
||||
"/tmp/scratch/wt", "/shared/dev"
|
||||
)
|
||||
self.assertEqual(resolved, os.path.realpath("/tmp/scratch/wt"))
|
||||
|
||||
def test_env_var_when_explicit_missing(self):
|
||||
with patch.dict(os.environ, {"GITEA_AUTHOR_WORKTREE": "/tmp/from-env"}):
|
||||
resolved = issue_lock_worktree.resolve_author_worktree_path(
|
||||
None, "/shared/dev"
|
||||
)
|
||||
self.assertEqual(resolved, os.path.realpath("/tmp/from-env"))
|
||||
|
||||
def test_project_root_fallback(self):
|
||||
resolved = issue_lock_worktree.resolve_author_worktree_path(
|
||||
None, "/shared/dev"
|
||||
)
|
||||
self.assertEqual(resolved, os.path.realpath("/shared/dev"))
|
||||
|
||||
|
||||
class TestPrWorktreeMatch(unittest.TestCase):
|
||||
def test_matching_paths_pass(self):
|
||||
result = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
"/tmp/scratch/wt",
|
||||
"/tmp/scratch/wt",
|
||||
"/shared/dev",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_mismatch_fails(self):
|
||||
result = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
"/tmp/scratch/wt",
|
||||
"/shared/dev",
|
||||
"/shared/dev",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("does not match locked worktree", result["reasons"][0])
|
||||
|
||||
def test_missing_locked_path_skips_check(self):
|
||||
result = issue_lock_worktree.verify_pr_worktree_matches_lock(
|
||||
None,
|
||||
"/any/path",
|
||||
"/shared/dev",
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
+121
-3
@@ -2802,26 +2802,41 @@ class TestVerifyMutationAuthority(unittest.TestCase):
|
||||
class TestIssueLocking(unittest.TestCase):
|
||||
"""Test issue locking and PR gating constraints."""
|
||||
|
||||
@staticmethod
|
||||
def _clean_master_git_state():
|
||||
return {"current_branch": "master", "porcelain_status": ""}
|
||||
|
||||
def tearDown(self):
|
||||
if os.path.exists(ISSUE_LOCK_FILE):
|
||||
os.remove(ISSUE_LOCK_FILE)
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
)
|
||||
@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):
|
||||
def test_lock_issue_success(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [] # no open PRs
|
||||
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertTrue(os.path.exists(ISSUE_LOCK_FILE))
|
||||
with open(ISSUE_LOCK_FILE, encoding="utf-8") as f:
|
||||
lock = json.load(f)
|
||||
self.assertIn("worktree_path", lock)
|
||||
|
||||
def test_lock_issue_mismatch_branch_fails(self):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-195-mutations", remote="prgs")
|
||||
self.assertIn("must contain locked issue pattern", str(ctx.exception))
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
)
|
||||
@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):
|
||||
def test_lock_issue_reused_by_open_pr_branch(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/issue-196-boundary"},
|
||||
@@ -2832,9 +2847,13 @@ class TestIssueLocking(unittest.TestCase):
|
||||
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))
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
)
|
||||
@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):
|
||||
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/other-branch"},
|
||||
@@ -2845,6 +2864,62 @@ class TestIssueLocking(unittest.TestCase):
|
||||
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))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_from_clean_scratch_worktree(self, _auth, _api):
|
||||
scratch = "/tmp/gitea-tools-author-scratch/issue-249-clean"
|
||||
with patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={"current_branch": "master", "porcelain_status": ""},
|
||||
) as mock_git:
|
||||
res = gitea_lock_issue(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path=scratch,
|
||||
)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["worktree_path"], os.path.realpath(scratch))
|
||||
mock_git.assert_called_once_with(os.path.realpath(scratch))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_fails_when_declared_worktree_dirty(self, _auth, _api):
|
||||
with patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={
|
||||
"current_branch": "master",
|
||||
"porcelain_status": " M gitea_mcp_server.py\n",
|
||||
},
|
||||
):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_lock_issue(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path="/tmp/scratch/wt",
|
||||
)
|
||||
self.assertIn("tracked file edits exist before issue lock", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_fails_when_declared_worktree_not_on_base(self, _auth, _api):
|
||||
with patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={
|
||||
"current_branch": "feat/issue-243-forbidden-git-gaps",
|
||||
"porcelain_status": "",
|
||||
},
|
||||
):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_lock_issue(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path="/tmp/scratch/wt",
|
||||
)
|
||||
self.assertIn("issue lock must be taken from base branch", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@@ -2893,6 +2968,49 @@ class TestIssueLocking(unittest.TestCase):
|
||||
gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_create_pr_worktree_mismatch_fails(self, _auth, _role):
|
||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr")
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_create_pr(
|
||||
title="feat: lock scratch worktree Closes #249",
|
||||
head="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path="/tmp/other-scratch",
|
||||
)
|
||||
self.assertIn("does not match locked worktree", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@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_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
|
||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
|
||||
mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
json.dump(_sample_issue_lock(
|
||||
issue_number=249,
|
||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||
worktree_path=scratch,
|
||||
), f)
|
||||
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||
res = gitea_create_pr(
|
||||
title="feat: issue-lock scratch worktree Closes #249",
|
||||
head="feat/issue-249-issue-lock-scratch-worktree",
|
||||
remote="prgs",
|
||||
worktree_path=scratch,
|
||||
)
|
||||
self.assertEqual(res["number"], 250)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pre-flight ordering and workspace edit block (#210)
|
||||
|
||||
Reference in New Issue
Block a user