Closes #442. gitea_lock_issue treated ANY remote branch containing issue-<n> as duplicate competing work (#400) and failed closed — including the issue's own already-pushed branch. If the in-memory lock was lost (e.g. MCP server restart) after the branch was pushed but before the PR, there was no non-destructive way to reacquire the lock, deadlocking PR creation. This surfaced during recovery of #420 (feat/issue-420-server-code-parity). Adds a safe own-branch adoption path. Changes: - issue_lock_adoption.py — assess_own_branch_adoption() distinguishes the issue's exact requested branch (adopt) from a different same-issue branch (competing, still fail-closed); ambiguous (own + other) fails closed. build_adoption_proof() emits the required proof block. - gitea_mcp_server.py — gitea_lock_issue consults the adoption assessment in place of the blanket branch-match block; on adoption the result carries an `adoption` proof (issue, branch, head commit, reason, no-existing-PR proof, no-competing-live-lock proof, lock file path/status). Open-PR and active- lease checks already run first, so adoption implies neither exists. Adds _branch_entry_commit_sha helper. - Tests: tests/test_issue_lock_adoption.py (8 cases) + 3 MCP-level cases in test_mcp_server.py (adopt own branch, open PR blocks adoption, create_pr proceeds after adopted lock). Existing competing-branch block preserved (still raises "already has matching branch"). Safety: - Different same-issue branch, competing branch, existing open PR, ambiguous ownership all remain fail-closed. #400 duplicate-work protection intact. - #420's feature branch was only read (git ls-remote); not modified/deleted. Validation: venv/bin/python -m pytest tests/ -q -> 1611 passed, 6 skipped Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
125 lines
4.6 KiB
Python
125 lines
4.6 KiB
Python
"""Unit tests for own-branch lock adoption decision (#442)."""
|
|
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,
|
|
)
|
|
|
|
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_substring_issue_number_not_falsely_matched_as_exact(self):
|
|
# A different issue's branch must not be adopted as ours.
|
|
result = assess_own_branch_adoption(
|
|
issue_number=42,
|
|
requested_branch="feat/issue-42-thing",
|
|
existing_branches=[{"name": "feat/issue-420-server-code-parity"}],
|
|
)
|
|
# 'issue-42' is a substring of 'issue-420', so marker matches but the
|
|
# name differs -> competing block, never adoption of the wrong branch.
|
|
self.assertEqual(result["outcome"], BLOCK_COMPETING)
|
|
self.assertNotEqual(result["matched_branch"], "feat/issue-420-server-code-parity")
|
|
|
|
|
|
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/gitea_issue_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"])
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|