Merge pull request 'fix(author-lock): dead-session recovery cross-session PR base-sync (Closes #872)' (#880) from fix/issue-872-dead-session-two-base-syncs into master
This commit was merged in pull request #880.
This commit is contained in:
@@ -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
|
||||
|
||||
+24
-15
@@ -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
|
||||
|
||||
@@ -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", "[email protected]")
|
||||
_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()
|
||||
Reference in New Issue
Block a user