Merge remote-tracking branch 'prgs/master' into feat/issue-636-inventory-api
This commit is contained in:
@@ -1639,6 +1639,358 @@ class TestSecondRemediationIntegration(unittest.TestCase):
|
||||
self.assertTrue(ownership_calls)
|
||||
|
||||
|
||||
class TestIssue855ExactPrSelector(unittest.TestCase):
|
||||
"""#855: exact pr_number pin for reconcile_merged_cleanups (#851 lifecycle)."""
|
||||
|
||||
def setUp(self):
|
||||
self._remotes = patch.dict(
|
||||
mcp_server.REMOTES,
|
||||
{
|
||||
"prgs": {
|
||||
"host": "gitea.example.com",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
}
|
||||
},
|
||||
)
|
||||
self._remotes.start()
|
||||
patch("gitea_audit.audit_enabled", return_value=False).start()
|
||||
self.mock_api = patch("mcp_server.api_request").start()
|
||||
self.mock_all = patch("mcp_server.api_get_all", return_value=[]).start()
|
||||
patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start()
|
||||
patch(
|
||||
"mcp_server.merged_cleanup_reconcile.is_head_ancestor_of_ref",
|
||||
return_value=True,
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server.get_profile",
|
||||
return_value=dict(RECONCILER_WITH_DELETE),
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server._profile_operation_gate",
|
||||
return_value=[],
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server._collect_branch_ownership_records",
|
||||
return_value={"records": [], "inventory_error": False},
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server.merged_cleanup_reconcile.discover_reviewer_scratch_worktrees",
|
||||
return_value=[],
|
||||
).start()
|
||||
patch("mcp_server.verify_preflight_purity", return_value=None).start()
|
||||
patch(
|
||||
"mcp_server.audit_reconciliation_mode.check_cleanup_execution_allowed",
|
||||
return_value=(True, []),
|
||||
).start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
|
||||
def _merged_pr(self, number, branch, sha="c" * 40):
|
||||
return {
|
||||
"number": number,
|
||||
"title": f"PR {number}",
|
||||
"body": f"Closes #{number - 4}",
|
||||
"merged": True,
|
||||
"merged_at": "2026-07-23T12:00:00Z",
|
||||
"merge_commit_sha": "f" * 40,
|
||||
"state": "closed",
|
||||
"head": {"ref": branch, "sha": sha},
|
||||
"base": {"ref": "master"},
|
||||
}
|
||||
|
||||
def test_exact_pr_848_ignores_newer_852_in_batch_queue(self):
|
||||
"""pr_number=848 selects only #848 even when #852 is newer/first."""
|
||||
from mcp_server import gitea_reconcile_merged_cleanups
|
||||
|
||||
pr_848 = self._merged_pr(
|
||||
848, "fix/issue-844-exclude-epic-containers", sha="c3f282ba" + "0" * 32
|
||||
)
|
||||
# Closed list would rank #852 first in batch mode; exact pin must ignore it.
|
||||
closed_batch = [
|
||||
self._merged_pr(852, "fix/issue-851-cleanup-worktree-before-remote-delete"),
|
||||
pr_848,
|
||||
self._merged_pr(849, "fix/issue-849-other"),
|
||||
self._merged_pr(846, "fix/issue-846-other"),
|
||||
self._merged_pr(845, "fix/issue-845-other"),
|
||||
]
|
||||
batch_fetch_calls = []
|
||||
|
||||
def fake_api(method, url, *args, **kwargs):
|
||||
if method == "GET" and url.rstrip("/").endswith("/pulls/848"):
|
||||
return dict(pr_848)
|
||||
if method == "GET" and "/pulls/" in url:
|
||||
raise AssertionError(f"unexpected PR fetch: {url}")
|
||||
if method == "GET" and "/branches/" in url:
|
||||
return {"name": "present"}
|
||||
return {}
|
||||
|
||||
def fake_all(url, auth, limit=None):
|
||||
batch_fetch_calls.append((url, limit))
|
||||
if "state=open" in url:
|
||||
return []
|
||||
if "state=closed" in url:
|
||||
# Exact mode must not use the closed batch list.
|
||||
raise AssertionError(
|
||||
"exact pr_number mode must not page closed PRs: " + url
|
||||
)
|
||||
return []
|
||||
|
||||
self.mock_api.side_effect = fake_api
|
||||
self.mock_all.side_effect = fake_all
|
||||
patch(
|
||||
"mcp_server._remote_branch_exists",
|
||||
return_value=True,
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server.merged_cleanup_reconcile.build_reconciliation_report",
|
||||
side_effect=lambda **kwargs: {
|
||||
"entries": [
|
||||
{
|
||||
"pr_number": int(pr["number"]),
|
||||
"head_branch": (pr.get("head") or {}).get("ref"),
|
||||
"issue_number": 844,
|
||||
"remote_branch": {
|
||||
"safe_to_delete_remote": True,
|
||||
"head_branch": (pr.get("head") or {}).get("ref"),
|
||||
},
|
||||
"local_worktree": {
|
||||
"safe_to_remove_worktree": True,
|
||||
"worktree_path": (
|
||||
"/tmp/branches/fix-issue-844-exclude-epic-containers"
|
||||
),
|
||||
},
|
||||
"planned_execution_order": (
|
||||
mcp_server.merged_cleanup_reconcile.plan_cleanup_execution_order(
|
||||
remote_assessment={"safe_to_delete_remote": True},
|
||||
local_assessment={"safe_to_remove_worktree": True},
|
||||
)
|
||||
),
|
||||
}
|
||||
for pr in kwargs.get("closed_prs") or []
|
||||
if pr.get("merged_at") or pr.get("merged")
|
||||
],
|
||||
"reviewer_scratch_entries": [],
|
||||
"merged_pr_count": len(kwargs.get("closed_prs") or []),
|
||||
},
|
||||
).start()
|
||||
|
||||
res = gitea_reconcile_merged_cleanups(
|
||||
dry_run=True,
|
||||
pr_number=848,
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
)
|
||||
self.assertTrue(res.get("success"))
|
||||
self.assertFalse(res.get("performed"))
|
||||
self.assertEqual(res.get("selection_mode"), "exact_pr")
|
||||
self.assertEqual(res.get("selected_pr_number"), 848)
|
||||
entries = res.get("entries") or []
|
||||
self.assertEqual(len(entries), 1, entries)
|
||||
self.assertEqual(entries[0].get("pr_number"), 848)
|
||||
self.assertEqual(
|
||||
entries[0].get("head_branch"),
|
||||
"fix/issue-844-exclude-epic-containers",
|
||||
)
|
||||
# No other PR appears in plan.
|
||||
self.assertEqual(list((res.get("planned_execution_orders") or {}).keys()), ["848"])
|
||||
plan = (res.get("planned_execution_orders") or {}).get("848") or []
|
||||
actions = [s.get("action") for s in plan]
|
||||
self.assertEqual(
|
||||
actions,
|
||||
[
|
||||
"remove_local_worktree",
|
||||
"reassess_branch_ownership",
|
||||
"delete_remote_branch",
|
||||
],
|
||||
)
|
||||
# Prove we never scanned the multi-PR closed batch.
|
||||
self.assertFalse(any("state=closed" in (u or "") for u, _ in batch_fetch_calls))
|
||||
# closed_batch fixture must remain unused (sanity).
|
||||
self.assertEqual(closed_batch[0]["number"], 852)
|
||||
|
||||
def test_exact_pr_execute_only_mutates_selected_pr(self):
|
||||
"""Execute with pr_number must never touch #845/#846/#849/#852."""
|
||||
from mcp_server import gitea_reconcile_merged_cleanups
|
||||
|
||||
pr_848 = self._merged_pr(848, "fix/issue-844-exclude-epic-containers")
|
||||
worktree_path = "/tmp/branches/fix-issue-844-exclude-epic-containers"
|
||||
remove_calls = []
|
||||
delete_api_calls = []
|
||||
ownership_branches = []
|
||||
|
||||
def fake_api(method, url, *args, **kwargs):
|
||||
if method == "GET" and url.rstrip("/").endswith("/pulls/848"):
|
||||
return dict(pr_848)
|
||||
if method == "DELETE":
|
||||
delete_api_calls.append(url)
|
||||
# Forbid foreign PR branch deletion by URL content.
|
||||
for forbidden in ("845", "846", "849", "852"):
|
||||
self.assertNotIn(forbidden, url)
|
||||
return {}
|
||||
|
||||
def fake_remove(project_root, branch, worktree_path=None):
|
||||
remove_calls.append({"branch": branch, "worktree_path": worktree_path})
|
||||
return {
|
||||
"success": True,
|
||||
"performed": True,
|
||||
"message": f"removed {worktree_path}",
|
||||
"worktree_path": worktree_path,
|
||||
}
|
||||
|
||||
def fake_collect(**kwargs):
|
||||
ownership_branches.append(kwargs.get("branch"))
|
||||
return {"records": [], "inventory_error": False}
|
||||
|
||||
def fake_probe(h, o, r, auth, br):
|
||||
return guard.classify_branch_readback_http_status(
|
||||
404, not_found_scope=guard.NOT_FOUND_SCOPE_BRANCH
|
||||
)
|
||||
|
||||
self.mock_api.side_effect = fake_api
|
||||
self.mock_all.side_effect = lambda url, auth, limit=None: []
|
||||
patch("mcp_server._remote_branch_exists", return_value=True).start()
|
||||
patch(
|
||||
"mcp_server.merged_cleanup_reconcile.build_reconciliation_report",
|
||||
return_value={
|
||||
"entries": [
|
||||
{
|
||||
"pr_number": 848,
|
||||
"head_branch": "fix/issue-844-exclude-epic-containers",
|
||||
"remote_branch": {"safe_to_delete_remote": True},
|
||||
"local_worktree": {
|
||||
"safe_to_remove_worktree": True,
|
||||
"worktree_path": worktree_path,
|
||||
},
|
||||
"planned_execution_order": [
|
||||
{"action": "remove_local_worktree", "phase": 1},
|
||||
{"action": "reassess_branch_ownership", "phase": 2},
|
||||
{"action": "delete_remote_branch", "phase": 3},
|
||||
],
|
||||
}
|
||||
],
|
||||
"reviewer_scratch_entries": [
|
||||
# Foreign scratch must be filtered before report execute loop;
|
||||
# if present here it would still be a test failure if acted on.
|
||||
],
|
||||
"merged_pr_count": 1,
|
||||
},
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server.merged_cleanup_reconcile.remove_local_worktree",
|
||||
side_effect=fake_remove,
|
||||
).start()
|
||||
patch(
|
||||
"mcp_server._collect_branch_ownership_records",
|
||||
side_effect=fake_collect,
|
||||
).start()
|
||||
patch("mcp_server._probe_remote_branch", side_effect=fake_probe).start()
|
||||
|
||||
res = gitea_reconcile_merged_cleanups(
|
||||
dry_run=False,
|
||||
execute_confirmed=True,
|
||||
pr_number=848,
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
)
|
||||
self.assertTrue(res.get("performed") or res.get("executed"))
|
||||
self.assertEqual(res.get("selection_mode"), "exact_pr")
|
||||
self.assertEqual(res.get("selected_pr_number"), 848)
|
||||
actions = res.get("actions") or []
|
||||
pr_numbers_touched = {
|
||||
a.get("pr_number") for a in actions if a.get("pr_number") is not None
|
||||
}
|
||||
self.assertTrue(pr_numbers_touched.issubset({None, 848}) or not pr_numbers_touched)
|
||||
removes = [a for a in actions if a.get("action") == "remove_local_worktree"]
|
||||
deletes = [a for a in actions if a.get("action") == "delete_remote_branch"]
|
||||
self.assertEqual(len(removes), 1)
|
||||
self.assertEqual(remove_calls[0]["branch"], "fix/issue-844-exclude-epic-containers")
|
||||
self.assertEqual(len(deletes), 1)
|
||||
self.assertTrue(deletes[0].get("success"))
|
||||
self.assertTrue(deletes[0].get("after_worktree_removal"))
|
||||
self.assertEqual(len(delete_api_calls), 1)
|
||||
self.assertEqual(
|
||||
ownership_branches, ["fix/issue-844-exclude-epic-containers"]
|
||||
)
|
||||
|
||||
def test_exact_pr_unknown_fails_closed_without_mutation(self):
|
||||
from mcp_server import gitea_reconcile_merged_cleanups
|
||||
|
||||
def fake_api(method, url, *args, **kwargs):
|
||||
if method == "GET" and "/pulls/99999" in url:
|
||||
raise RuntimeError("HTTP 404 Not Found")
|
||||
raise AssertionError(f"unexpected API call {method} {url}")
|
||||
|
||||
self.mock_api.side_effect = fake_api
|
||||
res = gitea_reconcile_merged_cleanups(
|
||||
dry_run=True,
|
||||
pr_number=99999,
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertFalse(res.get("success"))
|
||||
self.assertFalse(res.get("performed"))
|
||||
self.assertEqual(res.get("blocker_kind"), "pr_unresolvable")
|
||||
self.assertIn("99999", " ".join(res.get("reasons") or []))
|
||||
|
||||
def test_exact_pr_not_merged_fails_closed(self):
|
||||
from mcp_server import gitea_reconcile_merged_cleanups
|
||||
|
||||
def fake_api(method, url, *args, **kwargs):
|
||||
if method == "GET" and url.rstrip("/").endswith("/pulls/900"):
|
||||
return {
|
||||
"number": 900,
|
||||
"merged": False,
|
||||
"merged_at": None,
|
||||
"state": "open",
|
||||
"head": {"ref": "feat/x", "sha": "a" * 40},
|
||||
}
|
||||
raise AssertionError(f"unexpected {method} {url}")
|
||||
|
||||
self.mock_api.side_effect = fake_api
|
||||
res = gitea_reconcile_merged_cleanups(
|
||||
dry_run=False,
|
||||
execute_confirmed=True,
|
||||
pr_number=900,
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertFalse(res.get("success"))
|
||||
self.assertFalse(res.get("performed"))
|
||||
self.assertEqual(res.get("blocker_kind"), "pr_not_merged")
|
||||
|
||||
def test_exact_pr_invalid_number_fails_closed(self):
|
||||
from mcp_server import gitea_reconcile_merged_cleanups
|
||||
|
||||
res = gitea_reconcile_merged_cleanups(
|
||||
dry_run=True,
|
||||
pr_number=0,
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertFalse(res.get("success"))
|
||||
self.assertEqual(res.get("blocker_kind"), "invalid_pr_number")
|
||||
self.mock_api.assert_not_called()
|
||||
|
||||
def test_batch_mode_still_works_without_pr_number(self):
|
||||
"""Unfiltered batch path remains backward compatible."""
|
||||
from mcp_server import gitea_reconcile_merged_cleanups
|
||||
|
||||
self.mock_all.side_effect = lambda url, auth, limit=None: []
|
||||
self.mock_api.side_effect = lambda *a, **k: {}
|
||||
patch(
|
||||
"mcp_server.merged_cleanup_reconcile.build_reconciliation_report",
|
||||
return_value={
|
||||
"entries": [],
|
||||
"reviewer_scratch_entries": [],
|
||||
"merged_pr_count": 0,
|
||||
},
|
||||
).start()
|
||||
res = gitea_reconcile_merged_cleanups(dry_run=True, remote="prgs", limit=10)
|
||||
self.assertTrue(res.get("success"))
|
||||
self.assertEqual(res.get("selection_mode"), "batch")
|
||||
self.assertIsNone(res.get("selected_pr_number"))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -0,0 +1,483 @@
|
||||
"""Synthetic regression coverage for dirty orphaned worktree recovery (#860).
|
||||
|
||||
Modeled on the #850 / #855 shape without mutating their real state.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest import mock
|
||||
|
||||
import dirty_orphan_worktree_recovery as dorec
|
||||
import issue_lock_store
|
||||
|
||||
|
||||
DEAD_PID = 999_999_999
|
||||
LIVE_PID = os.getpid()
|
||||
BRANCH = "fix/issue-901-dirty-orphan"
|
||||
SOURCE_WT = "/repo/branches/issue-901-dirty-orphan"
|
||||
RECOVERY_WT_NAME = "recovery-issue-901-dirty-orphan"
|
||||
LOCAL_HEAD = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
|
||||
REMOTE_HEAD = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
|
||||
OTHER_HEAD = "cccccccccccccccccccccccccccccccccccccccc"
|
||||
FP_A = dorec.sha256_bytes(b"dirty-a")
|
||||
FP_B = dorec.sha256_bytes(b"dirty-b")
|
||||
FP_C = dorec.sha256_bytes(b"dirty-c-conflict")
|
||||
|
||||
|
||||
def durable_lock(**overrides):
|
||||
"""#850-shaped PID-less malformed same-claimant lock."""
|
||||
lock = {
|
||||
"issue_number": 901,
|
||||
"branch_name": BRANCH,
|
||||
"worktree_path": SOURCE_WT,
|
||||
"remote": "prgs",
|
||||
"org": "Example-Org",
|
||||
"repo": "Example-Repo",
|
||||
# intentionally no pid / session_pid / work_lease expiry
|
||||
"claimant": {"username": "author-user", "profile": "prgs-author"},
|
||||
}
|
||||
lock.update(overrides)
|
||||
return lock
|
||||
|
||||
|
||||
def base_kwargs(**overrides):
|
||||
kwargs = {
|
||||
"issue_number": 901,
|
||||
"branch_name": BRANCH,
|
||||
"source_worktree_path": SOURCE_WT,
|
||||
"remote": "prgs",
|
||||
"org": "Example-Org",
|
||||
"repo": "Example-Repo",
|
||||
"identity": "author-user",
|
||||
"profile": "prgs-author",
|
||||
"expected_local_head": LOCAL_HEAD,
|
||||
"expected_remote_head": REMOTE_HEAD,
|
||||
"expected_dirty_fingerprints": {"a.py": FP_A, "b.py": FP_B},
|
||||
"current_branch": BRANCH,
|
||||
"porcelain_status": " M a.py\n M b.py\n",
|
||||
"observed_local_head": LOCAL_HEAD,
|
||||
"observed_remote_head": REMOTE_HEAD,
|
||||
"observed_dirty_fingerprints": {"a.py": FP_A, "b.py": FP_B},
|
||||
"competing_live_locks": [],
|
||||
"competing_live_sessions": [],
|
||||
"workflow_lease_active": False,
|
||||
"workflow_lease_expired": True,
|
||||
"canonical_repo_root": "/repo",
|
||||
"worktree_registered": True,
|
||||
"current_pid": LIVE_PID,
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return kwargs
|
||||
|
||||
|
||||
def assess(lock=None, **overrides):
|
||||
return dorec.assess_dirty_orphan_recovery(
|
||||
durable_lock() if lock is None else lock, **base_kwargs(**overrides)
|
||||
)
|
||||
|
||||
|
||||
class FreshnessPidLess(unittest.TestCase):
|
||||
def test_pid_less_lock_is_not_live(self):
|
||||
freshness = issue_lock_store.assess_lock_freshness(durable_lock())
|
||||
self.assertFalse(freshness["live"])
|
||||
self.assertTrue(freshness.get("pid_missing"))
|
||||
self.assertEqual(freshness["status"], "malformed")
|
||||
|
||||
def test_pid_less_with_far_future_expiry_still_not_live(self):
|
||||
lock = durable_lock(
|
||||
work_lease={
|
||||
"operation_type": "author_issue_work",
|
||||
"expires_at": "2999-01-01T00:00:00Z",
|
||||
"last_heartbeat_at": "2999-01-01T00:00:00Z",
|
||||
}
|
||||
)
|
||||
freshness = issue_lock_store.assess_lock_freshness(lock)
|
||||
self.assertFalse(freshness["live"])
|
||||
self.assertTrue(freshness.get("pid_missing"))
|
||||
|
||||
|
||||
class EligibilityGranted(unittest.TestCase):
|
||||
def test_dead_same_claimant_pid_less_dirty(self):
|
||||
result = assess()
|
||||
self.assertEqual(result["outcome"], dorec.ELIGIBLE)
|
||||
self.assertTrue(result["eligible"])
|
||||
|
||||
def test_expired_workflow_lease_corroboration(self):
|
||||
result = assess(workflow_lease_active=False, workflow_lease_expired=True)
|
||||
self.assertTrue(result["eligible"])
|
||||
|
||||
def test_older_local_newer_remote_heads(self):
|
||||
result = assess()
|
||||
self.assertTrue(result["evidence"].get("heads_diverged"))
|
||||
self.assertTrue(result["eligible"])
|
||||
|
||||
|
||||
class EligibilityRefused(unittest.TestCase):
|
||||
def test_active_owner_with_pid(self):
|
||||
lock = durable_lock(pid=LIVE_PID, session_pid=LIVE_PID)
|
||||
result = assess(lock=lock, owner_process_alive_override=True)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
self.assertFalse(result["eligible"])
|
||||
self.assertTrue(any("alive" in r for r in result["reasons"]))
|
||||
|
||||
def test_foreign_claimant(self):
|
||||
result = assess(identity="other-user")
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
self.assertTrue(any("foreign claimant identity" in r for r in result["reasons"]))
|
||||
|
||||
def test_foreign_profile(self):
|
||||
result = assess(profile="prgs-reviewer")
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
|
||||
def test_fingerprint_mismatch(self):
|
||||
result = assess(observed_dirty_fingerprints={"a.py": "0" * 64, "b.py": FP_B})
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
self.assertTrue(any("fingerprint mismatch" in r for r in result["reasons"]))
|
||||
|
||||
def test_head_mismatch(self):
|
||||
result = assess(observed_local_head=OTHER_HEAD)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
|
||||
def test_remote_head_mismatch(self):
|
||||
result = assess(observed_remote_head=OTHER_HEAD)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
|
||||
def test_path_not_under_branches(self):
|
||||
result = assess(
|
||||
source_worktree_path="/tmp/branches/evil",
|
||||
# lock path also changed so worktree agreement holds
|
||||
lock=durable_lock(worktree_path="/tmp/branches/evil"),
|
||||
)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
self.assertTrue(any("canonical branches" in r for r in result["reasons"]))
|
||||
|
||||
def test_unregistered_worktree(self):
|
||||
result = assess(worktree_registered=False)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
|
||||
def test_active_workflow_lease(self):
|
||||
result = assess(workflow_lease_active=True, workflow_lease_expired=False)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
|
||||
def test_unsafe_dirty_path_pin(self):
|
||||
result = assess(
|
||||
expected_dirty_fingerprints={"../etc/passwd": FP_A},
|
||||
observed_dirty_fingerprints={"../etc/passwd": FP_A},
|
||||
)
|
||||
self.assertEqual(result["outcome"], dorec.REFUSED)
|
||||
|
||||
def test_symlink_escape_rejected_by_ancestry(self):
|
||||
ok, reasons = dorec.is_path_under_canonical_branches(
|
||||
"/tmp/branches/evil", canonical_repo_root="/repo"
|
||||
)
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(reasons)
|
||||
|
||||
|
||||
class ConflictDetection(unittest.TestCase):
|
||||
def test_overlapping_upstream_change(self):
|
||||
conflicts = dorec.detect_path_conflicts(
|
||||
dirty_paths=["c.py"],
|
||||
local_head_contents={"c.py": b"local-base"},
|
||||
remote_head_contents={"c.py": b"remote-changed"},
|
||||
dirty_contents={"c.py": b"dirty-c-conflict"},
|
||||
)
|
||||
self.assertEqual(len(conflicts), 1)
|
||||
self.assertEqual(conflicts[0]["path"], "c.py")
|
||||
|
||||
def test_unchanged_upstream_no_conflict(self):
|
||||
conflicts = dorec.detect_path_conflicts(
|
||||
dirty_paths=["a.py"],
|
||||
local_head_contents={"a.py": b"same"},
|
||||
remote_head_contents={"a.py": b"same"},
|
||||
dirty_contents={"a.py": b"dirty-a"},
|
||||
)
|
||||
self.assertEqual(conflicts, [])
|
||||
|
||||
|
||||
class CrashSafeRecovery(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.mkdtemp(prefix="dirty-orphan-")
|
||||
self.repo = os.path.join(self.tmp, "repo")
|
||||
self.branches = os.path.join(self.repo, "branches")
|
||||
self.source = os.path.join(self.branches, "issue-901-dirty-orphan")
|
||||
self.recovery = os.path.join(self.branches, RECOVERY_WT_NAME)
|
||||
os.makedirs(self.source, exist_ok=True)
|
||||
os.makedirs(self.branches, exist_ok=True)
|
||||
# seed dirty files in source
|
||||
with open(os.path.join(self.source, "a.py"), "wb") as fh:
|
||||
fh.write(b"dirty-a")
|
||||
with open(os.path.join(self.source, "b.py"), "wb") as fh:
|
||||
fh.write(b"dirty-b")
|
||||
self.journal_dir = os.path.join(self.tmp, "journals")
|
||||
self.lock = durable_lock(worktree_path=self.source)
|
||||
self.assessment = dorec.assess_dirty_orphan_recovery(
|
||||
self.lock,
|
||||
**base_kwargs(
|
||||
source_worktree_path=self.source,
|
||||
canonical_repo_root=self.repo,
|
||||
),
|
||||
)
|
||||
|
||||
class FakeGit(dorec.GitOps):
|
||||
def __init__(self, recovery_path, head):
|
||||
self.recovery_path = recovery_path
|
||||
self.head = head
|
||||
self.calls = []
|
||||
|
||||
def run(self, args, *, cwd):
|
||||
self.calls.append((args, cwd))
|
||||
if args[:3] == ["git", "worktree", "add"]:
|
||||
os.makedirs(self.recovery_path, exist_ok=True)
|
||||
return mock.Mock(returncode=0, stdout="", stderr="")
|
||||
if args[:2] == ["git", "checkout"]:
|
||||
return mock.Mock(returncode=0, stdout="", stderr="")
|
||||
if args[:2] == ["git", "rev-parse"]:
|
||||
return mock.Mock(returncode=0, stdout=self.head + "\n", stderr="")
|
||||
return mock.Mock(returncode=0, stdout="", stderr="")
|
||||
|
||||
self.git = FakeGit(self.recovery, REMOTE_HEAD)
|
||||
self.written_locks = []
|
||||
|
||||
def lock_writer(record):
|
||||
self.written_locks.append(record)
|
||||
|
||||
self.lock_writer = lock_writer
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self.tmp, ignore_errors=True)
|
||||
|
||||
def _run(self, **overrides):
|
||||
kwargs = {
|
||||
"assessment": self.assessment,
|
||||
"existing_lock": self.lock,
|
||||
"issue_number": 901,
|
||||
"branch_name": BRANCH,
|
||||
"source_worktree_path": self.source,
|
||||
"recovery_worktree_path": self.recovery,
|
||||
"remote": "prgs",
|
||||
"org": "Example-Org",
|
||||
"repo": "Example-Repo",
|
||||
"identity": "author-user",
|
||||
"profile": "prgs-author",
|
||||
"expected_local_head": LOCAL_HEAD,
|
||||
"expected_remote_head": REMOTE_HEAD,
|
||||
"expected_dirty_fingerprints": {"a.py": FP_A, "b.py": FP_B},
|
||||
"dirty_contents": {"a.py": b"dirty-a", "b.py": b"dirty-b"},
|
||||
"local_head_contents": {"a.py": b"base-a", "b.py": b"base-b"},
|
||||
"remote_head_contents": {"a.py": b"base-a", "b.py": b"base-b"},
|
||||
"canonical_repo_root": self.repo,
|
||||
"bind_lock": True,
|
||||
"lock_writer": self.lock_writer,
|
||||
"git_ops": self.git,
|
||||
"journal_dir": self.journal_dir,
|
||||
"session_pid": LIVE_PID,
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return dorec.run_dirty_orphan_recovery(**kwargs)
|
||||
|
||||
def test_success_preserves_dirty_bytes_and_source(self):
|
||||
result = self._run()
|
||||
self.assertTrue(result["success"])
|
||||
self.assertEqual(result["outcome"], dorec.RECOVERY_COMPLETED)
|
||||
self.assertTrue(os.path.isdir(self.source))
|
||||
with open(os.path.join(self.source, "a.py"), "rb") as fh:
|
||||
self.assertEqual(fh.read(), b"dirty-a")
|
||||
with open(os.path.join(self.recovery, "a.py"), "rb") as fh:
|
||||
self.assertEqual(fh.read(), b"dirty-a")
|
||||
with open(os.path.join(self.recovery, "b.py"), "rb") as fh:
|
||||
self.assertEqual(fh.read(), b"dirty-b")
|
||||
self.assertEqual(len(self.written_locks), 1)
|
||||
rec = self.written_locks[0]
|
||||
self.assertEqual(rec["session_pid"], LIVE_PID)
|
||||
self.assertTrue(rec["dirty_orphan_recovery"]["recovered"])
|
||||
self.assertTrue(rec["dirty_orphan_recovery"]["source_frozen"])
|
||||
|
||||
def test_conflict_leaves_governed_state(self):
|
||||
result = self._run(
|
||||
expected_dirty_fingerprints={"c.py": FP_C},
|
||||
dirty_contents={"c.py": b"dirty-c-conflict"},
|
||||
local_head_contents={"c.py": b"local-base"},
|
||||
remote_head_contents={"c.py": b"remote-changed"},
|
||||
)
|
||||
# #860 F4: session binding is NOT finalized while conflicts remain
|
||||
self.assertFalse(result["success"])
|
||||
self.assertEqual(result["outcome"], dorec.CONFLICTS_PRESENT)
|
||||
sidecar = os.path.join(self.recovery, "c.py.recovered-dirty")
|
||||
self.assertTrue(os.path.isfile(sidecar))
|
||||
state = os.path.join(
|
||||
self.recovery, dorec.CONFLICT_STATE_DIR, dorec.CONFLICT_STATE_FILE
|
||||
)
|
||||
self.assertTrue(os.path.isfile(state))
|
||||
with open(state, "r", encoding="utf-8") as fh:
|
||||
payload = json.load(fh)
|
||||
self.assertEqual(payload["resolution"], "author_edit_required")
|
||||
|
||||
def test_interrupt_before_journal_no_artifacts(self):
|
||||
result = self._run(interrupt_after_phase=dorec.PHASE_ELIGIBILITY)
|
||||
self.assertFalse(result["success"])
|
||||
self.assertEqual(result["outcome"], "INTERRUPTED")
|
||||
self.assertFalse(os.path.isdir(self.recovery))
|
||||
|
||||
def test_interrupt_after_journal_then_retry_idempotent(self):
|
||||
first = self._run(interrupt_after_phase=dorec.PHASE_JOURNAL_PERSISTED)
|
||||
self.assertEqual(first["outcome"], "INTERRUPTED")
|
||||
self.assertTrue(first["journal"]["artifacts_created"]["journal"])
|
||||
second = self._run()
|
||||
self.assertTrue(second["success"])
|
||||
# source still recoverable
|
||||
with open(os.path.join(self.source, "a.py"), "rb") as fh:
|
||||
self.assertEqual(fh.read(), b"dirty-a")
|
||||
|
||||
def test_interrupt_after_worktree_then_retry(self):
|
||||
first = self._run(interrupt_after_phase=dorec.PHASE_RECOVERY_WORKTREE)
|
||||
self.assertEqual(first["outcome"], "INTERRUPTED")
|
||||
self.assertTrue(os.path.isdir(self.recovery))
|
||||
second = self._run()
|
||||
self.assertTrue(second["success"])
|
||||
|
||||
def test_interrupt_after_binding_then_retry_complete(self):
|
||||
first = self._run(interrupt_after_phase=dorec.PHASE_BINDING)
|
||||
self.assertEqual(first["outcome"], "INTERRUPTED")
|
||||
second = self._run()
|
||||
self.assertTrue(second["success"])
|
||||
# completed journal makes further retries no-ops
|
||||
third = self._run()
|
||||
self.assertEqual(third["outcome"], dorec.RECOVERY_RESUMED)
|
||||
|
||||
def test_source_worktree_never_deleted(self):
|
||||
self._run()
|
||||
self.assertTrue(os.path.isdir(self.source))
|
||||
self.assertTrue(os.path.isfile(os.path.join(self.source, "a.py")))
|
||||
|
||||
def test_fingerprint_drift_refuses_without_mutation(self):
|
||||
result = self._run(dirty_contents={"a.py": b"CHANGED", "b.py": b"dirty-b"})
|
||||
self.assertFalse(result["success"])
|
||||
self.assertFalse(os.path.isdir(self.recovery))
|
||||
|
||||
|
||||
class SessionBindingPreflight(unittest.TestCase):
|
||||
def test_canonical_session_binding_recognized(self):
|
||||
lock = {
|
||||
"worktree_path": "/repo/branches/recovery",
|
||||
"session_pid": LIVE_PID,
|
||||
"dirty_orphan_recovery": {
|
||||
"recovered": True,
|
||||
"conflicts": [],
|
||||
"recovery_worktree_path": "/repo/branches/recovery",
|
||||
"source_worktree_path": SOURCE_WT,
|
||||
"accepted_head": REMOTE_HEAD,
|
||||
},
|
||||
}
|
||||
result = dorec.preflight_recognizes_recovered_provenance(lock)
|
||||
self.assertTrue(result["recognized"])
|
||||
|
||||
def test_conflicts_block_commit_preflight(self):
|
||||
lock = {
|
||||
"worktree_path": "/repo/branches/recovery",
|
||||
"session_pid": LIVE_PID,
|
||||
"dirty_orphan_recovery": {
|
||||
"recovered": True,
|
||||
"conflicts": [{"path": "c.py"}],
|
||||
},
|
||||
}
|
||||
result = dorec.preflight_recognizes_recovered_provenance(lock)
|
||||
self.assertFalse(result["recognized"])
|
||||
|
||||
def test_active_foreign_does_not_mutate(self):
|
||||
# assess-only path: foreign refused before run
|
||||
result = assess(identity="intruder")
|
||||
self.assertFalse(result["eligible"])
|
||||
|
||||
|
||||
class JournalSymlinkRefusal(unittest.TestCase):
|
||||
def test_symlink_journal_path_refused_on_load(self):
|
||||
tmp = tempfile.mkdtemp()
|
||||
try:
|
||||
real = os.path.join(tmp, "real.json")
|
||||
with open(real, "w", encoding="utf-8") as fh:
|
||||
fh.write("{}")
|
||||
link = os.path.join(tmp, "link.json")
|
||||
os.symlink(real, link)
|
||||
key = "symlink-test"
|
||||
jdir = tmp
|
||||
path = dorec._journal_path(key, journal_dir=jdir)
|
||||
with open(path, "w", encoding="utf-8") as fh:
|
||||
json.dump({"idempotency_key": key}, fh)
|
||||
os.remove(path)
|
||||
os.symlink(real, path)
|
||||
with self.assertRaises(ValueError):
|
||||
dorec.load_journal(key, journal_dir=jdir)
|
||||
finally:
|
||||
shutil.rmtree(tmp, ignore_errors=True)
|
||||
|
||||
|
||||
class RealGitMultiWorktreeIntegration(unittest.TestCase):
|
||||
def setUp(self):
|
||||
import subprocess
|
||||
self.tmp = tempfile.mkdtemp(prefix="git-integration-")
|
||||
self.repo = os.path.join(self.tmp, "repo")
|
||||
os.makedirs(self.repo, exist_ok=True)
|
||||
subprocess.run(["git", "init"], cwd=self.repo, check=True, capture_output=True)
|
||||
subprocess.run(["git", "config", "user.name", "Test User"], cwd=self.repo, check=True)
|
||||
subprocess.run(["git", "config", "user.email", "[email protected]"], cwd=self.repo, check=True)
|
||||
with open(os.path.join(self.repo, "init.txt"), "w") as fh:
|
||||
fh.write("init")
|
||||
subprocess.run(["git", "add", "."], cwd=self.repo, check=True)
|
||||
subprocess.run(["git", "commit", "-m", "init"], cwd=self.repo, check=True)
|
||||
branch = "fix/issue-999-test"
|
||||
subprocess.run(["git", "branch", branch], cwd=self.repo, check=True)
|
||||
self.branches = os.path.join(self.repo, "branches")
|
||||
self.source = os.path.join(self.branches, "issue-999-test")
|
||||
subprocess.run(["git", "worktree", "add", self.source, branch], cwd=self.repo, check=True)
|
||||
self.dirty_path = os.path.join(self.source, "dirty.txt")
|
||||
with open(self.dirty_path, "w") as fh:
|
||||
fh.write("dirty-data")
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self.tmp, ignore_errors=True)
|
||||
|
||||
def test_prepare_recovery_worktree_detached_no_exit_128(self):
|
||||
import subprocess
|
||||
head_sha = subprocess.check_output(["git", "rev-parse", "HEAD"], cwd=self.repo, text=True).strip()
|
||||
rec_wt = os.path.join(self.branches, "recovery-issue-999-test")
|
||||
res = dorec.prepare_recovery_worktree(
|
||||
canonical_repo_root=self.repo,
|
||||
recovery_worktree_path=rec_wt,
|
||||
branch_name="fix/issue-999-test",
|
||||
remote_head=head_sha,
|
||||
)
|
||||
self.assertTrue(res["success"], res.get("reasons"))
|
||||
self.assertTrue(os.path.isdir(rec_wt))
|
||||
|
||||
def test_real_lock_rebind_recovery_sanctioned(self):
|
||||
lock_dir = os.path.join(self.tmp, "locks")
|
||||
rec_wt = os.path.join(self.branches, "recovery-issue-999-test")
|
||||
os.makedirs(rec_wt, exist_ok=True)
|
||||
record = {
|
||||
"remote": "prgs",
|
||||
"org": "Example-Org",
|
||||
"repo": "Example-Repo",
|
||||
"issue_number": 999,
|
||||
"branch_name": "fix/issue-999-test",
|
||||
"worktree_path": rec_wt,
|
||||
"claimant": {"username": "author-user", "profile": "prgs-author"},
|
||||
}
|
||||
record_src = dict(record)
|
||||
record_src["worktree_path"] = self.source
|
||||
issue_lock_store.bind_session_lock(record_src, lock_dir=lock_dir)
|
||||
path = issue_lock_store.bind_session_lock(
|
||||
record,
|
||||
lock_dir=lock_dir,
|
||||
recovery_sanctioned=True,
|
||||
)
|
||||
self.assertTrue(os.path.isfile(path))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,244 @@
|
||||
"""#855 AC4: an expired reviewer lease must not indefinitely protect an
|
||||
already-merged branch when no live claimant exists.
|
||||
|
||||
Two layers are covered:
|
||||
|
||||
* ``branch_cleanup_guard.assess_expired_reviewer_lease_reclaim`` — the pure,
|
||||
fail-closed reclaim decision. Every condition must be provably satisfied or
|
||||
the lease keeps protecting the branch.
|
||||
* ``gitea_mcp_server._collect_branch_ownership_records`` — the wiring that
|
||||
supplies authoritative evidence (PR merged state, owner-process liveness,
|
||||
competing ownership) to that decision, and flips an expired reviewer lease
|
||||
to reclaimable only under the full policy.
|
||||
|
||||
All inputs are fabricated; no real repository, lease, or credential is used.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import branch_cleanup_guard
|
||||
|
||||
mcp_server = importlib.import_module("gitea_mcp_server")
|
||||
|
||||
FAKE_AUTH = "token fake"
|
||||
REMOTE = "prgs"
|
||||
ORG = "Scaled-Tech-Consulting"
|
||||
REPO = "Gitea-Tools"
|
||||
HOST = "gitea.prgs.cc"
|
||||
BRANCH = "feat/issue-638-webui-app-shell-phase1"
|
||||
PR_NUMBER = 818
|
||||
|
||||
|
||||
class TestAssessExpiredReviewerLeaseReclaim(unittest.TestCase):
|
||||
"""Pure fail-closed reclaim decision (#855 AC4)."""
|
||||
|
||||
def _call(self, **overrides):
|
||||
base = dict(
|
||||
role="reviewer",
|
||||
status="expired",
|
||||
pr_merged=True,
|
||||
owner_pid_alive=False,
|
||||
competing_active_claimant=False,
|
||||
)
|
||||
base.update(overrides)
|
||||
return branch_cleanup_guard.assess_expired_reviewer_lease_reclaim(**base)
|
||||
|
||||
def test_full_policy_satisfied_allows_reclaim(self):
|
||||
out = self._call()
|
||||
self.assertTrue(out["reclaim_allowed"])
|
||||
self.assertEqual(out["reasons"], [])
|
||||
self.assertEqual(out["decision"], "reclaim_expired_reviewer_lease")
|
||||
|
||||
def test_stale_dead_process_reviewer_also_reclaimable(self):
|
||||
out = self._call(status="stale_dead_process")
|
||||
self.assertTrue(out["reclaim_allowed"])
|
||||
|
||||
def test_non_reviewer_role_never_reclaims(self):
|
||||
for role in ("author", "merger", "controller", "reconciler", "unknown"):
|
||||
with self.subTest(role=role):
|
||||
out = self._call(role=role)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
self.assertTrue(out["reasons"])
|
||||
self.assertEqual(out["decision"], "keep_protecting")
|
||||
|
||||
def test_active_status_never_reclaims(self):
|
||||
out = self._call(status="active")
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_pr_not_merged_blocks_reclaim(self):
|
||||
out = self._call(pr_merged=False)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_pr_merged_unknown_fails_closed(self):
|
||||
out = self._call(pr_merged=None)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_owner_process_alive_blocks_reclaim(self):
|
||||
out = self._call(owner_pid_alive=True)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_owner_liveness_unknown_fails_closed(self):
|
||||
out = self._call(owner_pid_alive=None)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_competing_active_claimant_blocks_reclaim(self):
|
||||
out = self._call(competing_active_claimant=True)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_competing_claimant_unknown_fails_closed(self):
|
||||
out = self._call(competing_active_claimant=None)
|
||||
self.assertFalse(out["reclaim_allowed"])
|
||||
|
||||
def test_reasons_never_leak_secrets(self):
|
||||
out = self._call(role="author")
|
||||
blob = " ".join(out["reasons"]).lower()
|
||||
self.assertNotIn("token", blob)
|
||||
self.assertNotIn("password", blob)
|
||||
|
||||
|
||||
class _FakeLease(dict):
|
||||
pass
|
||||
|
||||
|
||||
class TestCollectorExpiredReviewerReclaimWiring(unittest.TestCase):
|
||||
"""`_collect_branch_ownership_records` supplies authoritative evidence and
|
||||
flips an expired reviewer lease to reclaimable only under the full policy."""
|
||||
|
||||
def _run(
|
||||
self,
|
||||
*,
|
||||
lease_role="reviewer",
|
||||
lease_freshness="stale_dead_process",
|
||||
owner_pid_alive=False,
|
||||
pr_merged=True,
|
||||
extra_leases=None,
|
||||
worktree_on_branch=False,
|
||||
):
|
||||
lease = _FakeLease(
|
||||
role=lease_role,
|
||||
work_kind="pr",
|
||||
work_number=PR_NUMBER,
|
||||
branch=BRANCH,
|
||||
status="active",
|
||||
owner_pid=999999,
|
||||
remote=REMOTE,
|
||||
org=ORG,
|
||||
repo=REPO,
|
||||
host=HOST,
|
||||
freshness={
|
||||
"freshness": lease_freshness,
|
||||
"owner_pid": 999999,
|
||||
"owner_pid_alive": owner_pid_alive,
|
||||
"expired_by_time": lease_freshness == "expired",
|
||||
},
|
||||
)
|
||||
leases = [lease] + list(extra_leases or [])
|
||||
|
||||
pr_payload = {
|
||||
"number": PR_NUMBER,
|
||||
"merged": pr_merged,
|
||||
"merged_at": "2026-07-23T00:00:00Z" if pr_merged else None,
|
||||
"head": {"ref": BRANCH},
|
||||
}
|
||||
|
||||
def fake_api_request(method, url, *a, **k):
|
||||
if method == "GET" and f"/pulls/{PR_NUMBER}" in url:
|
||||
return pr_payload
|
||||
raise AssertionError(f"unexpected api_request {method} {url}")
|
||||
|
||||
wt_entries = []
|
||||
if worktree_on_branch:
|
||||
wt_entries = [{"branch": BRANCH, "path": f"/x/branches/{BRANCH}"}]
|
||||
|
||||
with patch.object(
|
||||
mcp_server.lease_lifecycle,
|
||||
"list_active_leases",
|
||||
return_value={"leases": leases},
|
||||
), patch.object(
|
||||
mcp_server.control_plane_db, "get_db", return_value=object(), create=True
|
||||
), patch.object(
|
||||
mcp_server.issue_lock_store, "iter_lock_files", return_value=[]
|
||||
), patch.object(
|
||||
mcp_server.worktree_cleanup_audit,
|
||||
"list_worktrees",
|
||||
return_value=wt_entries,
|
||||
), patch.object(
|
||||
mcp_server, "api_get_all", return_value=[]
|
||||
), patch.object(
|
||||
mcp_server, "api_request", side_effect=fake_api_request
|
||||
):
|
||||
return mcp_server._collect_branch_ownership_records(
|
||||
remote=REMOTE,
|
||||
host=HOST,
|
||||
org=ORG,
|
||||
repo=REPO,
|
||||
branch=BRANCH,
|
||||
pr_number=PR_NUMBER,
|
||||
project_root="/x",
|
||||
auth=FAKE_AUTH,
|
||||
base_api="https://gitea.prgs.cc/api/v1/repos/x/y",
|
||||
)
|
||||
|
||||
def _reviewer_records(self, bundle):
|
||||
return [
|
||||
rec
|
||||
for rec in bundle["records"]
|
||||
if rec.get("category")
|
||||
== branch_cleanup_guard.OWNERSHIP_CATEGORY_REVIEWER_LEASE
|
||||
]
|
||||
|
||||
def test_merged_dead_uncontested_reviewer_lease_is_reclaimable(self):
|
||||
bundle = self._run()
|
||||
self.assertFalse(bundle["inventory_error"])
|
||||
recs = self._reviewer_records(bundle)
|
||||
self.assertEqual(len(recs), 1)
|
||||
self.assertTrue(recs[0]["reclaim_allowed"])
|
||||
# And the guard consequently does not block deletion on it.
|
||||
ownership = branch_cleanup_guard.assess_active_branch_ownership(
|
||||
remote=REMOTE, org=ORG, repo=REPO, branch=BRANCH, host=HOST,
|
||||
records=bundle["records"],
|
||||
)
|
||||
self.assertFalse(ownership["block"])
|
||||
|
||||
def test_unmerged_pr_keeps_reviewer_lease_protective(self):
|
||||
bundle = self._run(pr_merged=False)
|
||||
recs = self._reviewer_records(bundle)
|
||||
self.assertEqual(len(recs), 1)
|
||||
self.assertFalse(recs[0]["reclaim_allowed"])
|
||||
ownership = branch_cleanup_guard.assess_active_branch_ownership(
|
||||
remote=REMOTE, org=ORG, repo=REPO, branch=BRANCH, host=HOST,
|
||||
records=bundle["records"],
|
||||
)
|
||||
self.assertTrue(ownership["block"])
|
||||
|
||||
def test_owner_process_alive_keeps_reviewer_lease_protective(self):
|
||||
bundle = self._run(owner_pid_alive=True, lease_freshness="expired")
|
||||
recs = self._reviewer_records(bundle)
|
||||
self.assertFalse(recs[0]["reclaim_allowed"])
|
||||
|
||||
def test_competing_worktree_binding_keeps_reviewer_lease_protective(self):
|
||||
bundle = self._run(worktree_on_branch=True)
|
||||
recs = self._reviewer_records(bundle)
|
||||
self.assertFalse(recs[0]["reclaim_allowed"])
|
||||
ownership = branch_cleanup_guard.assess_active_branch_ownership(
|
||||
remote=REMOTE, org=ORG, repo=REPO, branch=BRANCH, host=HOST,
|
||||
records=bundle["records"],
|
||||
)
|
||||
self.assertTrue(ownership["block"])
|
||||
|
||||
def test_expired_author_lease_never_reclaimed_by_reviewer_policy(self):
|
||||
bundle = self._run(lease_role="author")
|
||||
author_recs = [
|
||||
rec
|
||||
for rec in bundle["records"]
|
||||
if rec.get("category")
|
||||
== branch_cleanup_guard.OWNERSHIP_CATEGORY_AUTHOR_LEASE
|
||||
]
|
||||
self.assertEqual(len(author_recs), 1)
|
||||
self.assertFalse(author_recs[0]["reclaim_allowed"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -24,6 +24,8 @@ def _lease(expires_at: str) -> dict:
|
||||
|
||||
|
||||
def _lock_record(**overrides) -> dict:
|
||||
# #860: live locks require a usable session pid; PID-less records are never
|
||||
# classified live merely because expiry/heartbeat fields are present.
|
||||
record = {
|
||||
"issue_number": 420,
|
||||
"branch_name": "feat/issue-420-server-code-parity",
|
||||
@@ -31,6 +33,8 @@ def _lock_record(**overrides) -> dict:
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
"worktree_path": "/tmp/wt-420",
|
||||
"session_pid": os.getpid(),
|
||||
"pid": os.getpid(),
|
||||
"work_lease": _lease("2999-01-01T00:00:00Z"),
|
||||
}
|
||||
record.update(overrides)
|
||||
@@ -88,6 +92,8 @@ class TestIssueLockStore(unittest.TestCase):
|
||||
existing = _lock_record(
|
||||
branch_name="feat/issue-420-other",
|
||||
worktree_path="/tmp/other",
|
||||
session_pid=os.getpid(),
|
||||
pid=os.getpid(),
|
||||
work_lease=_lease("2999-01-01T00:00:00Z"),
|
||||
)
|
||||
path = ils.lock_file_path(
|
||||
|
||||
@@ -0,0 +1,146 @@
|
||||
"""Tests for the MCP restart-path inventory and guards (#657).
|
||||
|
||||
Covers:
|
||||
* the registry is well-formed and every path is classified;
|
||||
* unknown restart attempts fail closed (AC "fail closed on unknown restart");
|
||||
* the previously-unguarded full-restart primitives stay guarded/absent
|
||||
against the real source tree (AC "tests for at least one previously
|
||||
unguarded path");
|
||||
* pkill of the daemon is still classified as contamination (#630, AC3);
|
||||
* the inventory doc and module stay in lock-step.
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
import mcp_restart_paths as rp
|
||||
import runtime_recovery_guard
|
||||
|
||||
REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
DOC_PATH = os.path.join(REPO_ROOT, "docs", "mcp-restart-path-inventory.md")
|
||||
|
||||
|
||||
class TestRegistryWellformed(unittest.TestCase):
|
||||
def test_registry_is_wellformed(self):
|
||||
# Must not raise.
|
||||
rp.assert_registry_wellformed()
|
||||
|
||||
def test_every_path_has_valid_classification(self):
|
||||
for path in rp.iter_restart_paths():
|
||||
self.assertIn(path.classification, rp.VALID_CLASSIFICATIONS)
|
||||
self.assertTrue(path.guard.strip(), path.path_id)
|
||||
self.assertTrue(path.references, path.path_id)
|
||||
self.assertTrue(path.locations, path.path_id)
|
||||
|
||||
def test_ids_are_unique(self):
|
||||
ids = [p.path_id for p in rp.iter_restart_paths()]
|
||||
self.assertEqual(len(ids), len(set(ids)))
|
||||
|
||||
def test_covers_every_classification(self):
|
||||
present = {p.classification for p in rp.iter_restart_paths()}
|
||||
self.assertEqual(present, set(rp.VALID_CLASSIFICATIONS))
|
||||
|
||||
|
||||
class TestUnknownAttemptFailsClosed(unittest.TestCase):
|
||||
def test_unknown_path_raises(self):
|
||||
with self.assertRaises(rp.UnknownRestartPathError):
|
||||
rp.assert_restart_attempt_registered("totally_novel_restart_hack")
|
||||
|
||||
def test_get_unknown_raises(self):
|
||||
with self.assertRaises(rp.UnknownRestartPathError):
|
||||
rp.get_restart_path("nope")
|
||||
|
||||
def test_registered_attempt_returns_path(self):
|
||||
path = rp.assert_restart_attempt_registered("manual_daemon_kill")
|
||||
self.assertEqual(path.classification, rp.CLASS_FORBIDDEN)
|
||||
|
||||
|
||||
class TestDaemonNeverSelfReplaces(unittest.TestCase):
|
||||
"""Previously-unguarded full-restart primitive: daemon self-replacement."""
|
||||
|
||||
def test_no_self_replacement_in_source(self):
|
||||
# The live daemon modules must contain no os.execv/os.kill/os._exit
|
||||
# self-restart call. Must not raise.
|
||||
rp.assert_no_daemon_self_replacement(REPO_ROOT)
|
||||
|
||||
def test_scanner_flags_injected_violation(self):
|
||||
# Guard the guard: prove the scanner catches a real self-replace call.
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
bad = Path(tmp) / "gitea_mcp_server.py"
|
||||
bad.write_text(
|
||||
"import os\n"
|
||||
"def restart():\n"
|
||||
" os.execv('/usr/bin/python', ['python'])\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
found = rp.scan_daemon_self_replacement(tmp)
|
||||
self.assertTrue(found)
|
||||
with self.assertRaises(AssertionError):
|
||||
rp.assert_no_daemon_self_replacement(tmp)
|
||||
|
||||
def test_scanner_ignores_comment_and_docstring_mentions(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
ok = Path(tmp) / "gitea_mcp_server.py"
|
||||
ok.write_text(
|
||||
"import os\n"
|
||||
"# NOT os.execv() to re-point the interpreter here.\n"
|
||||
'"""Never calls os._exit to restart."""\n'
|
||||
"value = 1\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
self.assertEqual(rp.scan_daemon_self_replacement(tmp), [])
|
||||
|
||||
|
||||
class TestLegacyAutoRestartHelperRemoved(unittest.TestCase):
|
||||
"""Previously-unguarded full-restart path: _trigger_mcp_auto_restart."""
|
||||
|
||||
def test_helper_absent_in_source(self):
|
||||
# Must not raise: helper was removed in #685.
|
||||
rp.assert_auto_restart_helper_absent(REPO_ROOT)
|
||||
|
||||
def test_scanner_flags_reintroduced_helper(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
bad = Path(tmp) / "mcp_server.py"
|
||||
bad.write_text(
|
||||
"def _trigger_mcp_auto_restart():\n return True\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
with self.assertRaises(AssertionError):
|
||||
rp.assert_auto_restart_helper_absent(tmp)
|
||||
|
||||
|
||||
class TestPkillStaysForbidden(unittest.TestCase):
|
||||
"""AC3: pkill of the daemon remains forbidden/contaminating (#630)."""
|
||||
|
||||
def test_manual_daemon_kill_registered_as_forbidden(self):
|
||||
path = rp.get_restart_path("manual_daemon_kill")
|
||||
self.assertEqual(path.classification, rp.CLASS_FORBIDDEN)
|
||||
|
||||
def test_pkill_classified_as_contamination(self):
|
||||
assessment = runtime_recovery_guard.assess_recovery_command(
|
||||
"pkill -f mcp_server.py"
|
||||
)
|
||||
self.assertTrue(assessment["contaminated"])
|
||||
|
||||
def test_read_only_probe_not_contamination(self):
|
||||
assessment = runtime_recovery_guard.assess_recovery_command(
|
||||
"ps aux | grep mcp_server"
|
||||
)
|
||||
self.assertFalse(assessment["contaminated"])
|
||||
|
||||
|
||||
class TestInventoryDocInSync(unittest.TestCase):
|
||||
def test_doc_exists(self):
|
||||
self.assertTrue(os.path.exists(DOC_PATH), DOC_PATH)
|
||||
|
||||
def test_doc_mentions_every_path_id(self):
|
||||
with open(DOC_PATH, encoding="utf-8") as handle:
|
||||
doc = handle.read()
|
||||
for path in rp.iter_restart_paths():
|
||||
self.assertIn(path.path_id, doc, f"doc missing {path.path_id}")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -37,6 +37,7 @@ def _live_lock(
|
||||
"operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE,
|
||||
"acquired_at": now.isoformat(),
|
||||
"expires_at": (now + timedelta(hours=2)).isoformat(),
|
||||
"session_pid": os.getpid(),
|
||||
"owner_pid": os.getpid(),
|
||||
"status": "active",
|
||||
}
|
||||
@@ -177,11 +178,24 @@ class TestAuthorOwnershipIssuePrMismatch(unittest.TestCase):
|
||||
self.assertFalse(result["proven"], result)
|
||||
self.assertTrue(any("branch" in r for r in result["reasons"]))
|
||||
|
||||
def test_no_lock_fail_closed(self):
|
||||
def test_pidless_durable_lock_rejected(self):
|
||||
"""A lock without any PID identity must be classified as malformed/non-live and fail closed."""
|
||||
lock = _live_lock(issue_number=727)
|
||||
lock.pop("session_pid", None)
|
||||
lock.pop("owner_pid", None)
|
||||
lock.pop("pid", None)
|
||||
path = issue_lock_store.lock_file_path(
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
issue_number=727,
|
||||
lock_dir=self.lock_dir,
|
||||
)
|
||||
issue_lock_store.save_lock_file(path, lock)
|
||||
result = mcp._prove_author_ownership_for_pr(
|
||||
pr_number=728,
|
||||
pr_title="feat: pr sync",
|
||||
pr_body="Closes #727",
|
||||
pr_body="Fixes #727",
|
||||
source_branch="feat/issue-727-pr-sync-status",
|
||||
remote="prgs",
|
||||
host=None,
|
||||
|
||||
Reference in New Issue
Block a user