Merge pull request 'fix(mcp): recover clean unpublished author work after the owning session exits (Closes #772)' (#774) from fix/issue-772-unpublished-claim-recovery into master
This commit was merged in pull request #774.
This commit is contained in:
+187
-63
@@ -2240,7 +2240,7 @@ def _resolve_issue_lock_for_pr(
|
||||
return lock_data
|
||||
|
||||
|
||||
def _save_issue_lock(data: dict) -> str:
|
||||
def _save_issue_lock(data: dict, *, expected_generation: int | None = None) -> str:
|
||||
existing = issue_lock_store.load_issue_lock(
|
||||
remote=str(data.get("remote") or ""),
|
||||
org=str(data.get("org") or ""),
|
||||
@@ -2251,11 +2251,127 @@ def _save_issue_lock(data: dict) -> str:
|
||||
if overwrite_block:
|
||||
raise RuntimeError(overwrite_block)
|
||||
try:
|
||||
return issue_lock_store.bind_session_lock(data)
|
||||
return issue_lock_store.bind_session_lock(
|
||||
data, expected_generation=expected_generation
|
||||
)
|
||||
except Exception as e:
|
||||
raise RuntimeError(f"Could not write issue lock file: {e}") from e
|
||||
|
||||
|
||||
def _evaluate_issue_lock_recovery(
|
||||
existing_lock: dict,
|
||||
*,
|
||||
issue_number: int,
|
||||
branch_name: str,
|
||||
worktree_path: str,
|
||||
remote: str,
|
||||
h: str | None,
|
||||
o: str,
|
||||
r: str,
|
||||
git_state: dict,
|
||||
) -> dict:
|
||||
"""Gather recovery evidence and decide the disposition (#753/#768/#772).
|
||||
|
||||
The single decision point for dead-session issue-lock recovery. The mutating
|
||||
``gitea_lock_issue`` path and the read-only diagnostic assessor both call
|
||||
this, so the two cannot report different verdicts or different supporting
|
||||
evidence for the same durable state (#772 AC9) — a divergence between them
|
||||
would itself be the defect.
|
||||
|
||||
Every input is either durable lock state or a live server-side observation
|
||||
(Gitea branch/PR inventory, git in the declared worktree). Nothing is
|
||||
reachable from an MCP caller's parameters (#772 AC1).
|
||||
"""
|
||||
recovery_auth = _auth(h)
|
||||
try:
|
||||
recovery_branches = api_get_all(
|
||||
f"{repo_api_url(h, o, r)}/branches", recovery_auth
|
||||
)
|
||||
except Exception as exc:
|
||||
raise RuntimeError(
|
||||
f"Could not list branches to verify issue-lock recovery: {exc}"
|
||||
)
|
||||
remote_head: str | None = None
|
||||
remote_branch_exists = False
|
||||
candidates: list[str] = []
|
||||
for entry in recovery_branches:
|
||||
entry_name = _branch_entry_name(entry)
|
||||
if entry_name == branch_name:
|
||||
remote_branch_exists = True
|
||||
remote_head = _branch_entry_commit_sha(entry)
|
||||
if issue_lock_adoption.branch_carries_issue_marker(entry_name, issue_number):
|
||||
candidates.append(entry_name)
|
||||
|
||||
pr_head: str | None = None
|
||||
pr_number: int | None = None
|
||||
for pull in _list_open_pulls(h, o, r, recovery_auth):
|
||||
pull_head = pull.get("head") or {}
|
||||
if str(pull_head.get("ref") or "") == branch_name:
|
||||
pr_head = pull_head.get("sha")
|
||||
pr_number = pull.get("number")
|
||||
break
|
||||
|
||||
local_head = git_state.get("head_sha")
|
||||
|
||||
# #768: the recovering author's clean worktree is, by construction, one
|
||||
# commit ahead of the head recorded at lock time — committing is the only
|
||||
# sanctioned way to clean it without discarding the work. Observe the
|
||||
# ancestry here, server-side, so the assessor can tell an honest
|
||||
# fast-forward from a rewritten head. Probed only when the heads actually
|
||||
# differ, and never from any caller-supplied value.
|
||||
head_ancestry: dict | None = None
|
||||
if remote_head and local_head and remote_head != local_head:
|
||||
head_ancestry = issue_lock_worktree.read_head_ancestry(
|
||||
worktree_path,
|
||||
ancestor_sha=remote_head,
|
||||
descendant_sha=local_head,
|
||||
)
|
||||
|
||||
# #772: with no remote branch there is no head to measure against, so the
|
||||
# base the branch was cut from is observed instead. Probed only in that
|
||||
# case, so the published path's evidence is untouched (#772 AC8).
|
||||
recorded_base: str | None = None
|
||||
base_ancestry: dict | None = None
|
||||
if not remote_branch_exists and local_head:
|
||||
base_observation = issue_lock_worktree.read_recorded_base(
|
||||
worktree_path,
|
||||
head_sha=local_head,
|
||||
)
|
||||
recorded_base = base_observation.get("base_sha")
|
||||
if recorded_base:
|
||||
base_ancestry = issue_lock_worktree.read_head_ancestry(
|
||||
worktree_path,
|
||||
ancestor_sha=recorded_base,
|
||||
descendant_sha=local_head,
|
||||
)
|
||||
|
||||
claimant = _work_lease_claimant(h)
|
||||
return issue_lock_recovery.assess_dead_session_lock_recovery(
|
||||
existing_lock,
|
||||
issue_number=issue_number,
|
||||
branch_name=branch_name,
|
||||
worktree_path=worktree_path,
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
identity=claimant.get("username"),
|
||||
profile=claimant.get("profile"),
|
||||
current_branch=git_state.get("current_branch"),
|
||||
porcelain_status=git_state.get("porcelain_status") or "",
|
||||
head_sha=local_head,
|
||||
remote_head_sha=remote_head,
|
||||
pr_head_sha=pr_head,
|
||||
pr_number=pr_number,
|
||||
competing_live_locks=issue_lock_store.list_live_locks(),
|
||||
candidate_branches=candidates,
|
||||
current_pid=os.getpid(),
|
||||
head_ancestry=head_ancestry,
|
||||
remote_branch_exists=remote_branch_exists,
|
||||
recorded_base_sha=recorded_base,
|
||||
base_ancestry=base_ancestry,
|
||||
)
|
||||
|
||||
|
||||
def _work_lease_claimant(host: str | None) -> dict:
|
||||
profile = get_profile()
|
||||
username = _IDENTITY_CACHE.get(host) if host else None
|
||||
@@ -3542,70 +3658,16 @@ def gitea_lock_issue(
|
||||
and existing_issue_lock.get("issue_number") == issue_number
|
||||
and not issue_lock_store.is_lease_live(existing_issue_lock)
|
||||
):
|
||||
recovery_auth = _auth(h)
|
||||
try:
|
||||
recovery_branches = api_get_all(
|
||||
f"{repo_api_url(h, o, r)}/branches", recovery_auth
|
||||
)
|
||||
except Exception as exc:
|
||||
raise RuntimeError(
|
||||
f"Could not list branches to verify issue-lock recovery: {exc}"
|
||||
)
|
||||
recovery_remote_head: str | None = None
|
||||
recovery_candidates: list[str] = []
|
||||
for entry in recovery_branches:
|
||||
entry_name = _branch_entry_name(entry)
|
||||
if entry_name == branch_name:
|
||||
recovery_remote_head = _branch_entry_commit_sha(entry)
|
||||
if issue_lock_adoption.branch_carries_issue_marker(entry_name, issue_number):
|
||||
recovery_candidates.append(entry_name)
|
||||
recovery_pr_head: str | None = None
|
||||
recovery_pr_number: int | None = None
|
||||
for pull in _list_open_pulls(h, o, r, recovery_auth):
|
||||
pull_head = pull.get("head") or {}
|
||||
if str(pull_head.get("ref") or "") == branch_name:
|
||||
recovery_pr_head = pull_head.get("sha")
|
||||
recovery_pr_number = pull.get("number")
|
||||
break
|
||||
recovery_claimant = _work_lease_claimant(h)
|
||||
# #768: the recovering author's clean worktree is, by construction, one
|
||||
# commit ahead of the head recorded at lock time — committing is the
|
||||
# only sanctioned way to clean it without discarding the work. Observe
|
||||
# the ancestry here, server-side, so the assessor can tell an honest
|
||||
# fast-forward from a rewritten head. Probed only when the heads
|
||||
# actually differ, and never from any caller-supplied value.
|
||||
recovery_local_head = git_state.get("head_sha")
|
||||
recovery_ancestry: dict | None = None
|
||||
if (
|
||||
recovery_remote_head
|
||||
and recovery_local_head
|
||||
and recovery_remote_head != recovery_local_head
|
||||
):
|
||||
recovery_ancestry = issue_lock_worktree.read_head_ancestry(
|
||||
resolved_worktree,
|
||||
ancestor_sha=recovery_remote_head,
|
||||
descendant_sha=recovery_local_head,
|
||||
)
|
||||
recovery_assessment = issue_lock_recovery.assess_dead_session_lock_recovery(
|
||||
recovery_assessment = _evaluate_issue_lock_recovery(
|
||||
existing_issue_lock,
|
||||
issue_number=issue_number,
|
||||
branch_name=branch_name,
|
||||
worktree_path=resolved_worktree,
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
identity=recovery_claimant.get("username"),
|
||||
profile=recovery_claimant.get("profile"),
|
||||
current_branch=git_state.get("current_branch"),
|
||||
porcelain_status=git_state.get("porcelain_status") or "",
|
||||
head_sha=git_state.get("head_sha"),
|
||||
remote_head_sha=recovery_remote_head,
|
||||
pr_head_sha=recovery_pr_head,
|
||||
pr_number=recovery_pr_number,
|
||||
competing_live_locks=issue_lock_store.list_live_locks(),
|
||||
candidate_branches=recovery_candidates,
|
||||
current_pid=os.getpid(),
|
||||
head_ancestry=recovery_ancestry,
|
||||
h=h,
|
||||
o=o,
|
||||
r=r,
|
||||
git_state=git_state,
|
||||
)
|
||||
|
||||
recovery_sanctioned = bool(
|
||||
@@ -3715,7 +3777,17 @@ def gitea_lock_issue(
|
||||
recovered_at=_work_lease_timestamp(_work_lease_now()),
|
||||
)
|
||||
|
||||
lock_file_path = _save_issue_lock(data)
|
||||
# #772 AC5: a recovery replaces a claim another session already owned, so
|
||||
# its write is a compare-and-swap against the generation the assessment was
|
||||
# made on. Two replacement sessions that both observed the same dead owner
|
||||
# cannot both succeed — the second finds a moved generation and fails
|
||||
# closed. Ordinary first-time claims keep the unconditional write.
|
||||
expected_generation = (
|
||||
issue_lock_store.lock_generation(existing_issue_lock)
|
||||
if recovery_sanctioned
|
||||
else None
|
||||
)
|
||||
lock_file_path = _save_issue_lock(data, expected_generation=expected_generation)
|
||||
lock_record = issue_lock_store.read_lock_file(lock_file_path) or data
|
||||
freshness = issue_lock_store.assess_lock_freshness(lock_record)
|
||||
competing = [
|
||||
@@ -3832,7 +3904,59 @@ def gitea_assess_work_issue_duplicate(
|
||||
phase=phase,
|
||||
recovered_owning_pr=recovered_owning_pr,
|
||||
)
|
||||
return {"success": not gate.get("block"), **gate}
|
||||
# #772 AC9: report the recovery disposition this issue's durable lock would
|
||||
# receive, decided by the same evaluator the mutating lock path uses, so the
|
||||
# diagnostic and the mutator can never disagree. Read-only: assessment only,
|
||||
# never a write, and a probe failure degrades to a reported reason rather
|
||||
# than turning a diagnostic into a hard error.
|
||||
lock_recovery: dict | None = None
|
||||
if (
|
||||
lock_data
|
||||
and int(lock_data.get("issue_number") or 0) == int(issue_number)
|
||||
and not issue_lock_store.is_lease_live(lock_data)
|
||||
):
|
||||
recovery_worktree = str(lock_data.get("worktree_path") or "")
|
||||
recovery_branch = str(
|
||||
branch_name or lock_data.get("branch_name") or ""
|
||||
)
|
||||
if recovery_worktree and recovery_branch:
|
||||
try:
|
||||
recovery_state = issue_lock_worktree.read_worktree_git_state(
|
||||
recovery_worktree
|
||||
)
|
||||
assessment = _evaluate_issue_lock_recovery(
|
||||
lock_data,
|
||||
issue_number=int(issue_number),
|
||||
branch_name=recovery_branch,
|
||||
worktree_path=recovery_worktree,
|
||||
remote=remote,
|
||||
h=h,
|
||||
o=o,
|
||||
r=r,
|
||||
git_state=recovery_state,
|
||||
)
|
||||
lock_recovery = {
|
||||
"outcome": assessment.get("outcome"),
|
||||
"recovery_sanctioned": assessment.get("recovery_sanctioned"),
|
||||
"is_candidate": assessment.get("is_candidate"),
|
||||
"reasons": assessment.get("reasons"),
|
||||
"evidence": assessment.get("evidence"),
|
||||
}
|
||||
except Exception as exc:
|
||||
lock_recovery = {
|
||||
"outcome": issue_lock_recovery.REFUSED,
|
||||
"recovery_sanctioned": False,
|
||||
"is_candidate": True,
|
||||
"reasons": [
|
||||
f"recovery evidence could not be gathered: {exc}"
|
||||
],
|
||||
"evidence": {},
|
||||
}
|
||||
return {
|
||||
"success": not gate.get("block"),
|
||||
**gate,
|
||||
"lock_recovery": lock_recovery,
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
|
||||
Reference in New Issue
Block a user