fix(mcp): accept strict-descendant dead-session recovery (Closes #768)
Permit fail-closed recovery when a clean local head is a strict descendant of the recorded PR/remote head, with server-side ancestry proof. Propagate recovery evidence through commit, push, and PR duplicate gates so an owning PR is not re-blocked as competing work. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Claude Opus 4.8
+238
-24
@@ -22,6 +22,18 @@ by the caller. It performs no mutation and no network I/O.
|
||||
Recovery deliberately does **not** relax base-equivalence for brand-new issue
|
||||
claims — only for a lock whose own prior record already proves the branch,
|
||||
worktree, head, and author.
|
||||
|
||||
#768 extends the head requirement from strict equality to "equal, or a strict
|
||||
descendant". Equality alone made remediation after a session death unreachable:
|
||||
recovery needs a clean worktree, the only sanctioned way to clean one without
|
||||
discarding work is to commit, and committing advances the head past the value
|
||||
recorded at lock time. A commit that strictly descends from the recorded head,
|
||||
on the same branch, in the same worktree, by the same claimant, preserves
|
||||
everything equality protected — the recorded head is still reachable, still an
|
||||
ancestor, still unmodified — so it is accepted, and nothing else is. The
|
||||
descendant fact is observed server-side by
|
||||
``issue_lock_worktree.read_head_ancestry`` and handed in as ``head_ancestry``;
|
||||
no caller can assert it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -40,6 +52,10 @@ REFUSED = "REFUSED"
|
||||
# Durable fields a lock must carry before it can be considered at all.
|
||||
REQUIRED_LOCK_FIELDS = ("issue_number", "branch_name", "worktree_path")
|
||||
|
||||
# How the clean local head relates to the head recorded at lock time (#768).
|
||||
HEAD_RELATION_EQUAL = "equal"
|
||||
HEAD_RELATION_STRICT_DESCENDANT = "strict_descendant"
|
||||
|
||||
|
||||
def _same_realpath(left: str | None, right: str | None) -> bool:
|
||||
if not left or not right:
|
||||
@@ -87,6 +103,68 @@ def _malformed_reasons(lock: Mapping[str, Any]) -> list[str]:
|
||||
return missing
|
||||
|
||||
|
||||
def _assess_strict_descendant(
|
||||
head_ancestry: Mapping[str, Any] | None,
|
||||
*,
|
||||
recorded_head: str,
|
||||
local_head: str,
|
||||
) -> tuple[bool, list[str]]:
|
||||
"""Is ``local_head`` a proven strict descendant of ``recorded_head`` (#768)?
|
||||
|
||||
``head_ancestry`` is the server-side git observation from
|
||||
``issue_lock_worktree.read_head_ancestry``. Its own ``ancestor_sha`` /
|
||||
``descendant_sha`` are re-checked against the heads this assessment is
|
||||
actually reasoning about, so a probe taken for some other pair of commits —
|
||||
stale, mismatched, or hand-built — can never authorize a waiver.
|
||||
|
||||
Returns ``(proven, notes)``. Notes name the exact missing element so a
|
||||
refused caller sees why, never a bare "unproven".
|
||||
"""
|
||||
if not isinstance(head_ancestry, Mapping):
|
||||
return False, [
|
||||
"no server-derived ancestry observation was available; a local head "
|
||||
"that differs from the recorded head cannot be accepted"
|
||||
]
|
||||
|
||||
notes: list[str] = []
|
||||
probe_ancestor = _text(head_ancestry.get("ancestor_sha"))
|
||||
probe_descendant = _text(head_ancestry.get("descendant_sha"))
|
||||
if probe_ancestor != recorded_head or probe_descendant != local_head:
|
||||
return False, [
|
||||
f"ancestry observation covers {probe_ancestor or 'unknown'} -> "
|
||||
f"{probe_descendant or 'unknown'}, not the heads under assessment "
|
||||
f"({recorded_head} -> {local_head})"
|
||||
]
|
||||
if not head_ancestry.get("probe_ok"):
|
||||
notes.extend(
|
||||
list(head_ancestry.get("reasons") or [])
|
||||
or ["ancestry probe did not complete; ancestry unproven"]
|
||||
)
|
||||
return False, notes
|
||||
if not head_ancestry.get("ancestor_present"):
|
||||
return False, [
|
||||
f"recorded head {recorded_head} is no longer reachable; a rewritten "
|
||||
"or force-moved head cannot be recovered"
|
||||
]
|
||||
if not head_ancestry.get("is_strict_descendant"):
|
||||
notes.extend(
|
||||
list(head_ancestry.get("reasons") or [])
|
||||
or [
|
||||
f"local head {local_head} is not a strict descendant of the "
|
||||
f"recorded head {recorded_head}"
|
||||
]
|
||||
)
|
||||
return False, notes
|
||||
|
||||
proof = _text(head_ancestry.get("proof")) or (
|
||||
f"{recorded_head} is an ancestor of {local_head}"
|
||||
)
|
||||
return True, [
|
||||
f"local head {local_head} strictly descends from recorded head "
|
||||
f"{recorded_head} ({proof})"
|
||||
]
|
||||
|
||||
|
||||
def assess_dead_session_lock_recovery(
|
||||
existing_lock: Mapping[str, Any] | None,
|
||||
*,
|
||||
@@ -107,6 +185,7 @@ def assess_dead_session_lock_recovery(
|
||||
competing_live_locks: Sequence[Mapping[str, Any]] | None = None,
|
||||
candidate_branches: Iterable[str] | None = None,
|
||||
current_pid: int | None = None,
|
||||
head_ancestry: Mapping[str, Any] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Decide whether a dead-session author lock may be natively recovered.
|
||||
|
||||
@@ -218,31 +297,66 @@ def assess_dead_session_lock_recovery(
|
||||
)
|
||||
evidence["dirty_files"] = dirty_files
|
||||
|
||||
# ── Head agreement: local == remote == PR ───────────────────────────────
|
||||
# ── Head agreement: local is the recorded head, or strictly descends it ──
|
||||
# The recorded head is what the remote branch still carries. A local head
|
||||
# equal to it is the #753 case. A local head that strictly descends from it
|
||||
# is the #768 case: the author committed remediation, which is the only way
|
||||
# to reach the clean worktree recovery itself demands.
|
||||
local_head = _text(head_sha)
|
||||
remote_head = _text(remote_head_sha)
|
||||
head_relation: str | None = None
|
||||
ancestry_proof: str | None = None
|
||||
if not local_head:
|
||||
reasons.append("local head SHA could not be determined")
|
||||
if not remote_head:
|
||||
reasons.append(
|
||||
f"remote head for branch '{locked_branch}' could not be determined"
|
||||
)
|
||||
if local_head and remote_head and local_head != remote_head:
|
||||
reasons.append(
|
||||
f"local head {local_head} does not match remote branch head {remote_head}"
|
||||
)
|
||||
if local_head and remote_head:
|
||||
if local_head == remote_head:
|
||||
head_relation = HEAD_RELATION_EQUAL
|
||||
else:
|
||||
descends, notes = _assess_strict_descendant(
|
||||
head_ancestry,
|
||||
recorded_head=remote_head,
|
||||
local_head=local_head,
|
||||
)
|
||||
if descends:
|
||||
head_relation = HEAD_RELATION_STRICT_DESCENDANT
|
||||
ancestry_proof = notes[0] if notes else None
|
||||
else:
|
||||
reasons.append(
|
||||
f"local head {local_head} does not match remote branch head "
|
||||
f"{remote_head}"
|
||||
)
|
||||
reasons.extend(notes)
|
||||
evidence["local_head"] = local_head or None
|
||||
evidence["remote_head"] = remote_head or None
|
||||
# ``recorded_head`` is the head recovery is being measured against;
|
||||
# ``accepted_head`` is the head this recovery actually adopts. They differ
|
||||
# only in the descendant case, and downstream gates need both (#768 AC2/AC7).
|
||||
evidence["recorded_head"] = remote_head or None
|
||||
evidence["accepted_head"] = local_head or None
|
||||
evidence["head_relation"] = head_relation
|
||||
evidence["ancestry_proof"] = ancestry_proof
|
||||
|
||||
pr_head = _text(pr_head_sha)
|
||||
if pr_head:
|
||||
evidence["pr_head"] = pr_head
|
||||
evidence["pr_number"] = pr_number
|
||||
if local_head and pr_head != local_head:
|
||||
reasons.append(
|
||||
f"open PR #{pr_number} head {pr_head} does not match local head "
|
||||
f"{local_head}"
|
||||
)
|
||||
# A descendant recovery has not been published yet, so the open PR
|
||||
# legitimately still points at the recorded head. Any other
|
||||
# disagreement is a real mismatch.
|
||||
if not (
|
||||
head_relation == HEAD_RELATION_STRICT_DESCENDANT
|
||||
and remote_head
|
||||
and pr_head == remote_head
|
||||
):
|
||||
reasons.append(
|
||||
f"open PR #{pr_number} head {pr_head} does not match local head "
|
||||
f"{local_head}"
|
||||
)
|
||||
|
||||
# ── Author identity ─────────────────────────────────────────────────────
|
||||
claimant = _lock_claimant(lock)
|
||||
@@ -335,16 +449,14 @@ def assess_dead_session_lock_recovery(
|
||||
if reasons:
|
||||
return _result(REFUSED, False, reasons, evidence)
|
||||
|
||||
return _result(
|
||||
RECOVERY_SANCTIONED,
|
||||
True,
|
||||
[
|
||||
f"durable lock for issue #{issue_number} matches branch "
|
||||
f"'{locked_branch}', worktree '{locked_worktree}', head {local_head}, "
|
||||
f"and claimant '{locked_identity}'; recorded pid {recorded_pid} is dead"
|
||||
],
|
||||
evidence,
|
||||
)
|
||||
proof = [
|
||||
f"durable lock for issue #{issue_number} matches branch "
|
||||
f"'{locked_branch}', worktree '{locked_worktree}', head {local_head}, "
|
||||
f"and claimant '{locked_identity}'; recorded pid {recorded_pid} is dead"
|
||||
]
|
||||
if head_relation == HEAD_RELATION_STRICT_DESCENDANT and ancestry_proof:
|
||||
proof.append(ancestry_proof)
|
||||
return _result(RECOVERY_SANCTIONED, True, proof, evidence)
|
||||
|
||||
|
||||
def _result(
|
||||
@@ -374,10 +486,16 @@ def owning_pr_recovery_evidence(
|
||||
lock already owns" apart from "a competing duplicate PR".
|
||||
|
||||
Returns ``None`` unless recovery was actually granted and the assessment's
|
||||
own evidence names exactly one owning PR whose head agrees with the local
|
||||
and remote heads. Nothing here is caller-supplied: every field is copied
|
||||
from evidence the assessor built out of durable lock state plus live
|
||||
own evidence names exactly one owning PR whose head agrees with the heads
|
||||
the assessor accepted. Nothing here is caller-supplied: every field is
|
||||
copied from evidence the assessor built out of durable lock state plus live
|
||||
git/Gitea observation, so a caller cannot manufacture an exemption.
|
||||
|
||||
#768: a descendant recovery carries two heads. ``head_sha`` stays the head
|
||||
the open PR currently shows (the recorded head, since the remediation is not
|
||||
published yet) and ``accepted_head`` is the local descendant that
|
||||
publication will move it to. Downstream gates accept either, so the
|
||||
exemption survives the very push it exists to permit.
|
||||
"""
|
||||
if not isinstance(assessment, Mapping):
|
||||
return None
|
||||
@@ -391,13 +509,28 @@ def owning_pr_recovery_evidence(
|
||||
pr_head = _text(evidence.get("pr_head"))
|
||||
local_head = _text(evidence.get("local_head"))
|
||||
remote_head = _text(evidence.get("remote_head"))
|
||||
recorded_head = _text(evidence.get("recorded_head")) or remote_head
|
||||
accepted_head = _text(evidence.get("accepted_head")) or local_head
|
||||
relation = _text(evidence.get("head_relation")) or HEAD_RELATION_EQUAL
|
||||
raw_pr_number = evidence.get("pr_number")
|
||||
|
||||
if raw_pr_number is None or not branch_name or not pr_head:
|
||||
return None
|
||||
# The assessor already required these to agree. Re-check, so a truncated or
|
||||
# hand-built evidence map can never authorize an exemption.
|
||||
if pr_head != local_head or pr_head != remote_head:
|
||||
if relation == HEAD_RELATION_EQUAL:
|
||||
if pr_head != local_head or pr_head != remote_head:
|
||||
return None
|
||||
elif relation == HEAD_RELATION_STRICT_DESCENDANT:
|
||||
# The PR must still be at the recorded head, and the accepted head must
|
||||
# actually be a different commit — otherwise this is not a descendant.
|
||||
if not recorded_head or pr_head != recorded_head:
|
||||
return None
|
||||
if not accepted_head or accepted_head == recorded_head:
|
||||
return None
|
||||
if accepted_head != local_head:
|
||||
return None
|
||||
else:
|
||||
return None
|
||||
try:
|
||||
pr_number = int(raw_pr_number)
|
||||
@@ -410,6 +543,77 @@ def owning_pr_recovery_evidence(
|
||||
"pr_number": pr_number,
|
||||
"branch_name": branch_name,
|
||||
"head_sha": pr_head,
|
||||
"recorded_head": recorded_head or None,
|
||||
"accepted_head": accepted_head or None,
|
||||
"head_relation": relation,
|
||||
}
|
||||
|
||||
|
||||
def recovered_owning_pr_from_lock(
|
||||
lock_record: Mapping[str, Any] | None,
|
||||
) -> dict[str, Any] | None:
|
||||
"""Rebuild owning-PR recovery evidence from a persisted lock (#768 AC2).
|
||||
|
||||
``gitea_lock_issue`` holds the live assessment only for the duration of the
|
||||
lock call. The commit, push, create-PR, and duplicate-assessment gates run
|
||||
later, in their own calls, and re-derive ownership from scratch — so an open
|
||||
PR that recovery already proved belongs to this author reappears there as
|
||||
competing duplicate work.
|
||||
|
||||
This reads the same proof back out of the durable ``dead_session_recovery``
|
||||
block that only the server writes, on a lock the caller must already own.
|
||||
It is a re-read of server-derived state, not a new assertion: a caller that
|
||||
could forge this could equally forge the lock file itself, which every other
|
||||
ownership gate already treats as authoritative.
|
||||
"""
|
||||
if not isinstance(lock_record, Mapping):
|
||||
return None
|
||||
record = lock_record.get("dead_session_recovery")
|
||||
if not isinstance(record, Mapping) or not record.get("recovered"):
|
||||
return None
|
||||
|
||||
branch_name = _text(record.get("branch_name")) or _text(
|
||||
lock_record.get("branch_name")
|
||||
)
|
||||
pr_head = _text(record.get("pr_head"))
|
||||
recorded_head = _text(record.get("recorded_head")) or _text(
|
||||
record.get("remote_head")
|
||||
)
|
||||
accepted_head = _text(record.get("accepted_head")) or _text(
|
||||
record.get("local_head")
|
||||
)
|
||||
relation = _text(record.get("head_relation")) or HEAD_RELATION_EQUAL
|
||||
raw_pr_number = record.get("pr_number")
|
||||
raw_issue_number = lock_record.get("issue_number")
|
||||
|
||||
if raw_pr_number is None or raw_issue_number is None:
|
||||
return None
|
||||
if not branch_name or not pr_head:
|
||||
return None
|
||||
if relation == HEAD_RELATION_EQUAL:
|
||||
if accepted_head and accepted_head != pr_head:
|
||||
return None
|
||||
elif relation == HEAD_RELATION_STRICT_DESCENDANT:
|
||||
if not recorded_head or pr_head != recorded_head:
|
||||
return None
|
||||
if not accepted_head or accepted_head == recorded_head:
|
||||
return None
|
||||
else:
|
||||
return None
|
||||
try:
|
||||
pr_number = int(raw_pr_number)
|
||||
issue_number = int(raw_issue_number)
|
||||
except (TypeError, ValueError):
|
||||
return None
|
||||
|
||||
return {
|
||||
"issue_number": issue_number,
|
||||
"pr_number": pr_number,
|
||||
"branch_name": branch_name,
|
||||
"head_sha": pr_head,
|
||||
"recorded_head": recorded_head or None,
|
||||
"accepted_head": accepted_head or None,
|
||||
"head_relation": relation,
|
||||
}
|
||||
|
||||
|
||||
@@ -418,7 +622,13 @@ def build_recovery_record(
|
||||
*,
|
||||
recovered_at: str,
|
||||
) -> dict[str, Any]:
|
||||
"""Durable, secret-free provenance for a completed recovery (#753 AC2/AC6)."""
|
||||
"""Durable, secret-free provenance for a completed recovery (#753 AC2/AC6).
|
||||
|
||||
#768 AC7: a granted recovery records, atomically with the lock itself, both
|
||||
session identities, the head it was measured against, the head it adopted,
|
||||
how those two relate, and the ancestry proof — so a descendant recovery can
|
||||
be audited after the fact without re-running any probe.
|
||||
"""
|
||||
evidence = dict(assessment.get("evidence") or {})
|
||||
return {
|
||||
"recovered": True,
|
||||
@@ -431,6 +641,10 @@ def build_recovery_record(
|
||||
"worktree_path": evidence.get("locked_worktree_path"),
|
||||
"local_head": evidence.get("local_head"),
|
||||
"remote_head": evidence.get("remote_head"),
|
||||
"recorded_head": evidence.get("recorded_head"),
|
||||
"accepted_head": evidence.get("accepted_head"),
|
||||
"head_relation": evidence.get("head_relation"),
|
||||
"ancestry_proof": evidence.get("ancestry_proof"),
|
||||
"pr_head": evidence.get("pr_head"),
|
||||
"pr_number": evidence.get("pr_number"),
|
||||
"identity": evidence.get("locked_identity"),
|
||||
|
||||
Reference in New Issue
Block a user