Merge branch 'master' into feat/issue-660-session-checkpoint-schema
This commit is contained in:
@@ -2485,6 +2485,7 @@ def _evaluate_issue_lock_recovery(
|
|||||||
worktree_path,
|
worktree_path,
|
||||||
prior_head_sha=local_head,
|
prior_head_sha=local_head,
|
||||||
synced_head_sha=remote_head,
|
synced_head_sha=remote_head,
|
||||||
|
remote=remote,
|
||||||
)
|
)
|
||||||
|
|
||||||
# #772: with no remote branch there is no head to measure against, so the
|
# #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,
|
prior_head_sha: str | None,
|
||||||
synced_head_sha: str | None,
|
synced_head_sha: str | None,
|
||||||
|
remote: str | None = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Observe whether ``synced_head_sha`` is a sanctioned merge-based branch sync
|
"""Observe whether ``synced_head_sha`` is a sanctioned merge-sync of a base
|
||||||
that advanced the PR branch past ``prior_head_sha`` (#871).
|
into the branch above ``prior_head_sha`` (#871/#872).
|
||||||
|
|
||||||
``gitea_update_pr_branch_by_merge`` advances a PR branch by merging the base
|
Reports server-derived git facts only; the recovery disposition lives in
|
||||||
branch *into* the branch (``POST /pulls/{n}/update?style=merge``). The result
|
``issue_lock_recovery``. All comparisons are executed locally in the
|
||||||
is a merge commit ``M`` on the branch whose **first** parent is the prior
|
declared worktree -- nothing is taken from caller parameters.
|
||||||
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).
|
|
||||||
|
|
||||||
Provenance is proven only when ALL hold:
|
Provenance is proven only when ALL hold:
|
||||||
|
|
||||||
@@ -183,6 +174,7 @@ def read_merge_sync_provenance(
|
|||||||
path = (worktree_path or "").strip()
|
path = (worktree_path or "").strip()
|
||||||
prior = (prior_head_sha or "").strip()
|
prior = (prior_head_sha or "").strip()
|
||||||
synced = (synced_head_sha or "").strip()
|
synced = (synced_head_sha or "").strip()
|
||||||
|
target_remote = (remote or "").strip() or None
|
||||||
result: dict = {
|
result: dict = {
|
||||||
"prior_head_sha": prior or None,
|
"prior_head_sha": prior or None,
|
||||||
"synced_head_sha": synced or None,
|
"synced_head_sha": synced or None,
|
||||||
@@ -235,6 +227,23 @@ def read_merge_sync_provenance(
|
|||||||
try:
|
try:
|
||||||
result["prior_present"] = _present(prior)
|
result["prior_present"] = _present(prior)
|
||||||
result["synced_present"] = _present(synced)
|
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
|
except OSError as exc: # git unavailable — fail closed, never assume
|
||||||
result["reasons"].append(f"merge-sync provenance probe could not run: {exc}")
|
result["reasons"].append(f"merge-sync provenance probe could not run: {exc}")
|
||||||
return result
|
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