diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index a658eb1..fad38e4 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -2485,6 +2485,7 @@ def _evaluate_issue_lock_recovery( worktree_path, prior_head_sha=local_head, synced_head_sha=remote_head, + remote=remote, ) # #772: with no remote branch there is no head to measure against, so the diff --git a/issue_lock_worktree.py b/issue_lock_worktree.py index 8188536..b6de9bf 100644 --- a/issue_lock_worktree.py +++ b/issue_lock_worktree.py @@ -150,23 +150,14 @@ def read_merge_sync_provenance( *, prior_head_sha: str | None, synced_head_sha: str | None, + remote: str | None = None, ) -> dict: - """Observe whether ``synced_head_sha`` is a sanctioned merge-based branch sync - that advanced the PR branch past ``prior_head_sha`` (#871). + """Observe whether ``synced_head_sha`` is a sanctioned merge-sync of a base + into the branch above ``prior_head_sha`` (#871/#872). - ``gitea_update_pr_branch_by_merge`` advances a PR branch by merging the base - branch *into* the branch (``POST /pulls/{n}/update?style=merge``). The result - is a merge commit ``M`` on the branch whose **first** parent is the prior - branch head and whose second parent is the base tip. When the owning session - then dies without the durable lock's recorded head being refreshed, the local - worktree still sits at ``prior_head_sha`` while the live PR head is ``M``. - - Recovering that drift safely requires proving the remote head is *exactly* - such a merge-sync — not a rewrite, rebase, force-push, or an unrelated - commit. This is that server-side observation. It reports facts only; the - disposition lives in ``issue_lock_recovery``. Every field is read from git in - the declared worktree — nothing is supplied by, or reachable from, an MCP - caller (#871). + Reports server-derived git facts only; the recovery disposition lives in + ``issue_lock_recovery``. All comparisons are executed locally in the + declared worktree -- nothing is taken from caller parameters. Provenance is proven only when ALL hold: @@ -183,6 +174,7 @@ def read_merge_sync_provenance( path = (worktree_path or "").strip() prior = (prior_head_sha or "").strip() synced = (synced_head_sha or "").strip() + target_remote = (remote or "").strip() or None result: dict = { "prior_head_sha": prior or None, "synced_head_sha": synced or None, @@ -235,6 +227,23 @@ def read_merge_sync_provenance( try: result["prior_present"] = _present(prior) result["synced_present"] = _present(synced) + if not result["synced_present"] and path and os.path.isdir(path): + if target_remote: + subprocess.run( + ["git", "-C", path, "fetch", target_remote, "--quiet"], + capture_output=True, + text=True, + check=False, + ) + result["synced_present"] = _present(synced) + if not result["synced_present"]: + subprocess.run( + ["git", "-C", path, "fetch", "--quiet"], + capture_output=True, + text=True, + check=False, + ) + result["synced_present"] = _present(synced) except OSError as exc: # git unavailable — fail closed, never assume result["reasons"].append(f"merge-sync provenance probe could not run: {exc}") return result diff --git a/tests/test_issue_872_dead_session_two_base_syncs.py b/tests/test_issue_872_dead_session_two_base_syncs.py new file mode 100644 index 0000000..b5f2cf3 --- /dev/null +++ b/tests/test_issue_872_dead_session_two_base_syncs.py @@ -0,0 +1,332 @@ +"""Dead-session recovery for cross-session PR base-syncs (#872). + +Validates that when a sanctioned server-side base-sync (``gitea_update_pr_branch_by_merge``) +advances a PR's remote head from A to B (a merge commit whose first parent is A), +a subsequent author session whose local worktree is at A can recover the dead-session +lock via HEAD_RELATION_REMOTE_MERGE_SYNCED and execute a second base-sync (B to C) +without deadlock or RuntimeError. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +import tempfile +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import issue_lock_recovery # noqa: E402 +import issue_lock_store # noqa: E402 +import issue_lock_worktree # noqa: E402 + +ISSUE = 8720 +PR_NUMBER = 8721 +BRANCH = f"fix/issue-{ISSUE}-dead-session-two-base-syncs" +IDENTITY = "example-author" +PROFILE = "prgs-author" +REMOTE = "prgs" +ORG = "ExampleOrg" +REPO = "ExampleRepo" + + +def dead_pid() -> int: + proc = subprocess.Popen([sys.executable, "-c", "pass"]) + proc.wait() + return proc.pid + + +def future_ts(hours: int = 4) -> str: + return ( + (datetime.now(timezone.utc) + timedelta(hours=hours)) + .isoformat() + .replace("+00:00", "Z") + ) + + +def _git(cwd, *args): + return subprocess.run( + ["git", "-C", cwd, *args], + capture_output=True, + text=True, + check=True, + ) + + +def _rev(cwd, ref="HEAD") -> str: + return _git(cwd, "rev-parse", ref).stdout.strip() + + +def build_two_base_sync_repo(tmp: str) -> dict: + """Build a repo simulating two sequential base-sync merges.""" + _git(tmp, "init", "-q", "-b", "master") + _git(tmp, "config", "user.email", "author@example.com") + _git(tmp, "config", "user.name", "Author") + Path(tmp, "base.txt").write_text("base 1\n") + _git(tmp, "add", "-A") + _git(tmp, "commit", "-q", "-m", "initial master") + + # Feature branch cut from initial master -> Head A (prior/recorded head) + _git(tmp, "checkout", "-q", "-b", BRANCH) + Path(tmp, "feature.txt").write_text("feature work\n") + _git(tmp, "add", "-A") + _git(tmp, "commit", "-q", "-m", "feature commit A") + head_a = _rev(tmp) + + # Master advances -> Master 1 + _git(tmp, "checkout", "-q", "master") + Path(tmp, "base.txt").write_text("base 1\nbase 2\n") + _git(tmp, "add", "-A") + _git(tmp, "commit", "-q", "-m", "master advance 1") + master_1 = _rev(tmp) + + # First sync: merge master into feature -> Head B (merge commit, first parent = A) + _git(tmp, "checkout", "-q", BRANCH) + _git(tmp, "merge", "-q", "--no-ff", "-m", "First base-sync (merge master)", "master") + head_b = _rev(tmp) + + # Master advances again -> Master 2 + _git(tmp, "checkout", "-q", "master") + Path(tmp, "base.txt").write_text("base 1\nbase 2\nbase 3\n") + _git(tmp, "add", "-A") + _git(tmp, "commit", "-q", "-m", "master advance 2") + master_2 = _rev(tmp) + + # Second sync: merge master into feature -> Head C (merge commit, first parent = B) + _git(tmp, "checkout", "-q", BRANCH) + _git(tmp, "merge", "-q", "--no-ff", "-m", "Second base-sync (merge master)", "master") + head_c = _rev(tmp) + + # Non-merge rebase/force-pushed branch shape + _git(tmp, "checkout", "-q", "-b", "rebased-branch", head_a) + Path(tmp, "rebase.txt").write_text("rebased\n") + _git(tmp, "add", "-A") + _git(tmp, "commit", "-q", "-m", "rebased commit") + rebased_head = _rev(tmp) + + # Reset worktree back to head A, as a dead session leaving local worktree at A + _git(tmp, "checkout", "-q", BRANCH) + _git(tmp, "reset", "-q", "--hard", head_a) + + return { + "head_a": head_a, + "master_1": master_1, + "head_b": head_b, + "master_2": master_2, + "head_c": head_c, + "rebased_head": rebased_head, + } + + +def make_dead_lock(worktree, **over): + pid = dead_pid() + lock = { + "issue_number": ISSUE, + "branch_name": BRANCH, + "worktree_path": worktree, + "remote": REMOTE, + "org": ORG, + "repo": REPO, + "session_pid": pid, + "pid": pid, + "claimant": {"username": IDENTITY, "profile": PROFILE}, + "work_lease": { + "operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE, + "issue_number": ISSUE, + "branch": BRANCH, + "worktree_path": worktree, + "claimant": {"username": IDENTITY, "profile": PROFILE}, + "expires_at": future_ts(), + }, + } + lock.update(over) + return lock + + +class TestDeadSessionTwoBaseSyncs(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.shas = build_two_base_sync_repo(self.tmp) + self.head_a = self.shas["head_a"] + self.head_b = self.shas["head_b"] + self.head_c = self.shas["head_c"] + + def test_first_base_sync_dead_session_recovery(self): + """AC1: dead session after first base sync (remote=B, local=A) recovers via merge-sync.""" + lock = make_dead_lock(self.tmp, synced_pr_head=self.head_b) + obs = issue_lock_worktree.read_merge_sync_provenance( + self.tmp, + prior_head_sha=self.head_a, + synced_head_sha=self.head_b, + remote=REMOTE, + ) + self.assertTrue(obs["is_merge_sync"], obs["reasons"]) + self.assertTrue(obs["prior_is_ancestor"]) + self.assertTrue(obs["synced_is_merge"]) + self.assertTrue(obs["first_parent_reaches_prior"]) + + res = issue_lock_recovery.assess_dead_session_lock_recovery( + lock, + issue_number=ISSUE, + branch_name=BRANCH, + worktree_path=self.tmp, + remote=REMOTE, + org=ORG, + repo=REPO, + identity=IDENTITY, + profile=PROFILE, + current_branch=BRANCH, + porcelain_status="", + head_sha=self.head_a, + remote_head_sha=self.head_b, + pr_head_sha=self.head_b, + pr_number=PR_NUMBER, + competing_live_locks=[], + candidate_branches=[BRANCH], + current_pid=os.getpid(), + remote_branch_exists=True, + sync_provenance=obs, + ) + self.assertEqual(res["outcome"], issue_lock_recovery.RECOVERY_SANCTIONED, res["reasons"]) + self.assertEqual( + res["evidence"]["head_relation"], + issue_lock_recovery.HEAD_RELATION_REMOTE_MERGE_SYNCED, + ) + self.assertEqual(res["evidence"]["accepted_head"], self.head_b) + + def test_second_base_sync_head_refresh_after_recovery(self): + """AC2: after recovery, second base sync (remote B -> C) updates durable lock head.""" + lock_dir = tempfile.mkdtemp() + lock_data = make_dead_lock(self.tmp, synced_pr_head=self.head_b) + # Rebind to current PID as gitea_lock_issue does upon sanctioned recovery + lock_data["session_pid"] = os.getpid() + lock_data["pid"] = os.getpid() + issue_lock_store.save_lock_file( + issue_lock_store.lock_file_path( + remote=REMOTE, org=ORG, repo=REPO, issue_number=ISSUE, lock_dir=lock_dir + ), + lock_data, + ) + + refresh_res = issue_lock_store.apply_durable_lock_head_refresh( + remote=REMOTE, + org=ORG, + repo=REPO, + issue_number=ISSUE, + branch_name=BRANCH, + worktree_path=self.tmp, + pr_number=PR_NUMBER, + identity=IDENTITY, + profile=PROFILE, + current_pid=os.getpid(), + expected_old_head=self.head_b, + new_head=self.head_c, + synced_at=future_ts(0), + base_head=self.shas["master_2"], + lock_dir=lock_dir, + ) + self.assertTrue(refresh_res["refreshed"], refresh_res["reasons"]) + self.assertTrue(refresh_res["read_after_write_ok"]) + loaded = issue_lock_store.load_issue_lock( + remote=REMOTE, org=ORG, repo=REPO, issue_number=ISSUE, lock_dir=lock_dir + ) + self.assertEqual(loaded.get("synced_pr_head"), self.head_c) + + def test_dirty_worktree_blocks_recovery(self): + """AC3: tracked dirty edits block dead-session recovery.""" + lock = make_dead_lock(self.tmp) + obs = issue_lock_worktree.read_merge_sync_provenance( + self.tmp, prior_head_sha=self.head_a, synced_head_sha=self.head_b + ) + res = issue_lock_recovery.assess_dead_session_lock_recovery( + lock, + issue_number=ISSUE, + branch_name=BRANCH, + worktree_path=self.tmp, + remote=REMOTE, + org=ORG, + repo=REPO, + identity=IDENTITY, + profile=PROFILE, + current_branch=BRANCH, + porcelain_status=" M feature.txt\n", + head_sha=self.head_a, + remote_head_sha=self.head_b, + pr_head_sha=self.head_b, + pr_number=PR_NUMBER, + competing_live_locks=[], + candidate_branches=[BRANCH], + current_pid=os.getpid(), + remote_branch_exists=True, + sync_provenance=obs, + ) + self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED) + self.assertTrue(any("dirty" in r or "tracked" in r for r in res["reasons"])) + + def test_foreign_reclaimer_blocks_recovery(self): + """AC4: a foreign claimant cannot recover a dead-session lock.""" + lock = make_dead_lock(self.tmp) + obs = issue_lock_worktree.read_merge_sync_provenance( + self.tmp, prior_head_sha=self.head_a, synced_head_sha=self.head_b + ) + res = issue_lock_recovery.assess_dead_session_lock_recovery( + lock, + issue_number=ISSUE, + branch_name=BRANCH, + worktree_path=self.tmp, + remote=REMOTE, + org=ORG, + repo=REPO, + identity="intruder-user", + profile=PROFILE, + current_branch=BRANCH, + porcelain_status="", + head_sha=self.head_a, + remote_head_sha=self.head_b, + pr_head_sha=self.head_b, + pr_number=PR_NUMBER, + competing_live_locks=[], + candidate_branches=[BRANCH], + current_pid=os.getpid(), + remote_branch_exists=True, + sync_provenance=obs, + ) + self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED) + + def test_non_merge_rebased_remote_head_blocks_recovery(self): + """AC5: a rebased/force-pushed remote head (not a merge commit) is refused.""" + lock = make_dead_lock(self.tmp) + obs = issue_lock_worktree.read_merge_sync_provenance( + self.tmp, prior_head_sha=self.head_a, synced_head_sha=self.shas["rebased_head"] + ) + self.assertFalse(obs["is_merge_sync"]) + res = issue_lock_recovery.assess_dead_session_lock_recovery( + lock, + issue_number=ISSUE, + branch_name=BRANCH, + worktree_path=self.tmp, + remote=REMOTE, + org=ORG, + repo=REPO, + identity=IDENTITY, + profile=PROFILE, + current_branch=BRANCH, + porcelain_status="", + head_sha=self.head_a, + remote_head_sha=self.shas["rebased_head"], + pr_head_sha=self.shas["rebased_head"], + pr_number=PR_NUMBER, + competing_live_locks=[], + candidate_branches=[BRANCH], + current_pid=os.getpid(), + remote_branch_exists=True, + sync_provenance=obs, + ) + self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED) + + +if __name__ == "__main__": + unittest.main()