fix: inject duplicate-work context fetcher for testable lock/create_pr paths (#400)
Expose issue_duplicate_context_fetcher on the MCP server so lock_issue, commit_files, and create_pr duplicate rechecks avoid live Gitea calls in unit tests. Update affected test suites to patch the fetcher without weakening duplicate-work gate assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
@@ -80,7 +80,10 @@ class TestIssueLockArtifactWarning(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
self._env_patcher.stop()
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
@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())
|
||||
|
||||
@@ -76,7 +76,14 @@ class CommitFilesCapabilityBase(unittest.TestCase):
|
||||
with open(self.config_path, "w", encoding="utf-8") as fh:
|
||||
fh.write(json.dumps(CONFIG))
|
||||
|
||||
self._dup_fetcher_patcher = patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
self._dup_fetcher_patcher.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._dup_fetcher_patcher.stop()
|
||||
self._remotes.stop()
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = (
|
||||
|
||||
@@ -84,7 +84,14 @@ class TestCommitPayloads(unittest.TestCase):
|
||||
mcp_server._preflight_whoami_called = True
|
||||
mcp_server._preflight_capability_called = True
|
||||
|
||||
self._dup_fetcher_patcher = patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
self._dup_fetcher_patcher.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._dup_fetcher_patcher.stop()
|
||||
self._remotes.stop()
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
|
||||
|
||||
@@ -121,6 +121,37 @@ class TestDuplicateReportOutcome(unittest.TestCase):
|
||||
self.assertEqual(good["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
|
||||
|
||||
|
||||
class TestInjectableDuplicateFetcher(unittest.TestCase):
|
||||
@patch("mcp_server.get_auth_header", return_value="token x")
|
||||
def test_lock_issue_uses_injected_fetcher(self, _auth):
|
||||
seen = {}
|
||||
|
||||
def fetcher(h, o, r, auth, issue_number):
|
||||
seen["issue_number"] = issue_number
|
||||
return [], [], {"status": "not_claimed"}
|
||||
|
||||
with patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
side_effect=fetcher,
|
||||
), patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value={
|
||||
"current_branch": "master",
|
||||
"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",
|
||||
)
|
||||
self.assertEqual(seen["issue_number"], 400)
|
||||
|
||||
|
||||
class TestMcpDuplicateRecheck(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._dir = tempfile.TemporaryDirectory()
|
||||
|
||||
+38
-23
@@ -112,6 +112,11 @@ def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
|
||||
return record
|
||||
|
||||
|
||||
def _clear_duplicate_context_fetcher(*_args, **_kwargs):
|
||||
"""Default injectable duplicate-work context for lock/create_pr tests."""
|
||||
return [], [], {"status": "not_claimed"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Create Issue
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -166,13 +171,17 @@ class TestCreateIssue(unittest.TestCase):
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestCreatePR(unittest.TestCase):
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("os.path.exists", return_value=True)
|
||||
@patch("builtins.open")
|
||||
def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role):
|
||||
def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher):
|
||||
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||
@@ -187,13 +196,17 @@ class TestCreatePR(unittest.TestCase):
|
||||
self.assertEqual(payload["base"], "main")
|
||||
self.assertIn("Closes #123", payload["title"])
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []))
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("os.path.exists", return_value=True)
|
||||
@patch("builtins.open")
|
||||
def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role):
|
||||
def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role, _dup_fetcher):
|
||||
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||
@@ -3044,8 +3057,14 @@ class TestIssueLocking(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True)
|
||||
self._env_patcher.start()
|
||||
self._dup_fetcher_patcher = patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
return_value=([], [], {"status": "not_claimed"}),
|
||||
)
|
||||
self.mock_dup_fetcher = self._dup_fetcher_patcher.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._dup_fetcher_patcher.stop()
|
||||
self._env_patcher.stop()
|
||||
if os.path.exists(ISSUE_LOCK_FILE):
|
||||
os.remove(ISSUE_LOCK_FILE)
|
||||
@@ -3054,10 +3073,8 @@ class TestIssueLocking(unittest.TestCase):
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_success(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [] # no open PRs
|
||||
def test_lock_issue_success(self, _auth, _git_state):
|
||||
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work")
|
||||
@@ -3082,50 +3099,48 @@ class TestIssueLocking(unittest.TestCase):
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_reused_by_open_pr_branch(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [{
|
||||
def test_lock_issue_reused_by_open_pr_branch(self, _auth, _git_state):
|
||||
self.mock_dup_fetcher.return_value = ([{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/issue-196-boundary"},
|
||||
"title": "Some PR",
|
||||
"body": "No closes ref"
|
||||
}]
|
||||
"body": "No closes ref",
|
||||
}], [], {"status": "not_claimed"})
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already tied to an open PR", str(ctx.exception))
|
||||
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state):
|
||||
mock_api.return_value = [{
|
||||
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, _git_state):
|
||||
self.mock_dup_fetcher.return_value = ([{
|
||||
"number": 200,
|
||||
"head": {"ref": "feat/other-branch"},
|
||||
"title": "Some PR",
|
||||
"body": "fixes #196"
|
||||
}]
|
||||
"body": "fixes #196",
|
||||
}], [], {"status": "not_claimed"})
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already tied to an open PR", str(ctx.exception))
|
||||
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
|
||||
|
||||
@patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=_clean_master_git_state_for_lock(),
|
||||
)
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_lock_issue_reused_by_remote_branch(self, _auth, mock_api, _git_state):
|
||||
mock_api.side_effect = [
|
||||
def test_lock_issue_reused_by_remote_branch(self, _auth, _git_state):
|
||||
self.mock_dup_fetcher.return_value = (
|
||||
[],
|
||||
[{"name": "feat/issue-196-existing-work"}],
|
||||
]
|
||||
["feat/issue-196-existing-work"],
|
||||
{"status": "not_claimed"},
|
||||
)
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||
self.assertIn("already has matching branch", str(ctx.exception))
|
||||
self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception))
|
||||
|
||||
def test_lock_issue_blocks_active_same_operation_lease(self):
|
||||
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||
|
||||
Reference in New Issue
Block a user