Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 74 Pull Requests 9 Actions Packages Projects Releases Wiki Activity

Merge pull request 'feat: move duplicate-work detection before author mutations (Closes #400)' (#413) from feat/issue-400-duplicate-work-preflight into master

Browse Source
This commit was merged in pull request #413.
This commit is contained in:
sysadmin
2026-07-07 15:40:23 -05:00
parent 89a7d4dbfc e4adccd82a
commit d95ea323d7
10 changed files with 711 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_agent_temp_artifacts.py
+4 -1
View File
@@ -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())
tests/test_commit_files_capability.py
+7
View File
@@ -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 = (
tests/test_commit_payloads.py
+7
View File
@@ -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()
tests/test_issue_work_duplicate_gate.py
+251
View File
@@ -0,0 +1,251 @@
"""Tests for early duplicate-work detection (#400)."""
import json
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_work_duplicate_gate as dup_gate
import mcp_server
from issue_work_duplicate_gate import (
OUTCOME_DUPLICATE_BRANCH_PREVENTED,
OUTCOME_DUPLICATE_COMMIT_PREVENTED,
OUTCOME_DUPLICATE_PR_PREVENTED,
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED,
PHASE_COMMIT,
PHASE_CREATE_PR,
PHASE_LOCK,
assess_work_issue_duplicate_gate,
assess_work_issue_duplicate_report,
)
class TestDuplicateGateAssessment(unittest.TestCase):
def test_clear_issue_passes(self):
result = assess_work_issue_duplicate_gate(
400,
open_prs=[],
branch_names=["feat/other-issue-99"],
claim_entry={"status": "not_claimed"},
locked_branch="feat/issue-400-duplicate-work-preflight",
phase=PHASE_LOCK,
)
self.assertFalse(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
def test_open_pr_blocks(self):
prs = [{
"number": 397,
"title": "feat: handoff",
"body": "Closes #395",
"head": {"ref": "feat/issue-395-proof-backed-review-handoff"},
}]
result = assess_work_issue_duplicate_gate(
395,
open_prs=prs,
branch_names=[],
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_PR_PREVENTED)
def test_conflicting_remote_branch_blocks(self):
result = assess_work_issue_duplicate_gate(
395,
open_prs=[],
branch_names=["feat/issue-395-proof-backed-handoff-claims"],
locked_branch="feat/issue-395-new-attempt",
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_BRANCH_PREVENTED)
def test_active_claim_on_other_branch_blocks(self):
result = assess_work_issue_duplicate_gate(
398,
open_prs=[],
branch_names=[],
claim_entry={
"status": "active",
"latest_heartbeat": {
"branch": "feat/issue-398-validation-cwd-proof",
},
},
locked_branch="feat/issue-398-other-branch",
phase=PHASE_LOCK,
)
self.assertTrue(result["block"])
def test_commit_phase_maps_to_commit_outcome(self):
prs = [{
"number": 411,
"title": "x",
"body": "Closes #398",
"head": {"ref": "feat/issue-398-validation-cwd-proof"},
}]
result = assess_work_issue_duplicate_gate(
398,
open_prs=prs,
branch_names=[],
locked_branch="feat/issue-398-alt",
phase=PHASE_COMMIT,
)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_COMMIT_PREVENTED)
def test_stale_claim_does_not_block_by_status_alone(self):
result = assess_work_issue_duplicate_gate(
400,
open_prs=[],
branch_names=[],
claim_entry={"status": "reclaimable", "reasons": ["stale"]},
locked_branch="feat/issue-400-duplicate-work-preflight",
phase=PHASE_LOCK,
)
self.assertFalse(result["block"])
class TestDuplicateReportOutcome(unittest.TestCase):
def test_requires_exactly_one_outcome(self):
bad = assess_work_issue_duplicate_report("work finished")
self.assertFalse(bad["complete"])
good = assess_work_issue_duplicate_report(
"Duplicate work not prevented for issue #400."
)
self.assertTrue(good["complete"])
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()
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._lock_patch.start()
self._remotes = patch.dict(mcp_server.REMOTES, {
"prgs": {"host": "gitea.example.com", "org": "Example-Org",
"repo": "Example-Repo"},
})
self._remotes.start()
mcp_server._IDENTITY_CACHE.clear()
def tearDown(self):
patch.stopall()
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)
@patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={
"profile_name": "test-author",
"allowed_operations": ["gitea.read", "gitea.repo.commit"],
"forbidden_operations": [],
"audit_label": "test-author",
})
@patch("mcp_server.get_auth_header", return_value="token x")
def test_commit_files_blocked_on_recheck(self, _auth, _profile, mock_gate):
self._write_lock()
mock_gate.return_value = {
"block": True,
"reasons": ["open PR #412 already covers issue #400"],
"outcome": OUTCOME_DUPLICATE_COMMIT_PREVENTED,
"safe_next_action": "stop",
}
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author")
with patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
):
result = mcp_server.gitea_commit_files(
files=[{
"operation": "create",
"path": "a.txt",
"content_plain": "hi",
}],
message="test",
remote="prgs",
)
self.assertFalse(result["success"])
self.assertIn("duplicate_gate", result)
@patch("mcp_server._assess_issue_duplicate_gate")
@patch("mcp_server.get_profile", return_value={
"profile_name": "test-author",
"allowed_operations": ["gitea.read", "gitea.pr.create"],
"forbidden_operations": [],
"audit_label": "test-author",
})
@patch("mcp_server.get_auth_header", return_value="token x")
def test_create_pr_returns_handoff_on_duplicate(self, _auth, _profile, mock_gate):
self._write_lock()
mock_gate.return_value = {
"block": True,
"reasons": ["open PR #412 already covers issue #400"],
"outcome": OUTCOME_DUPLICATE_PR_PREVENTED,
"safe_next_action": "reconciliation handoff",
}
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author")
with patch(
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
return_value=(True, []),
):
result = mcp_server.gitea_create_pr(
title="feat: x (Closes #400)",
head="feat/issue-400-x",
base="master",
body="Closes #400",
remote="prgs",
)
self.assertFalse(result["success"])
self.assertIsNone(result.get("number"))
self.assertIn("duplicate_gate", result)
if __name__ == "__main__":
unittest.main()
tests/test_mcp_server.py
+38 -23
View File
@@ -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:
tests/test_review_proofs.py
+1
View File
@@ -2346,6 +2346,7 @@ class TestWorkIssueFinalReport(unittest.TestCase):
"- Safe next action: open PR",
"- Next: open PR",
"- Safety statement: no review/merge",
"- Duplicate work outcome: duplicate work not prevented",
])
def test_complete_work_issue_report_earns_a(self):