Files
Gitea-Tools/issue_lock_renewal.py
sysadminandClaude Opus 4.8 a30a3ce4c3 fix(lock): make the exact-owner renewal waiver survive downstream gates (#760)
Addresses review #499 on PR #791.

F1 — the renewal sanction was computed and then discarded twice.

`assess_issue_lock_worktree` waived base-equivalence only for
`recovery_sanctioned`, and `gitea_lock_issue` never passed the renewal waiver
into it. A branch being renewed always carries committed work, so it is never
base-equivalent, and #753 recovery refuses when the recorded PID is alive —
which is the defining condition of a renewal. Every real renewal was therefore
granted by the assessor and then rejected one gate later.

Thread `renewal_sanctioned` into `assess_issue_lock_worktree` alongside
`recovery_sanctioned`, waiving base-equivalence on the same grounds and nothing
else. Cleanliness is evaluated before the waiver and is never relaxed; the
assessment now reports which of the two waivers applied.

The MCP-level regression then exposed a second discard point: the duplicate-work
gate rejected the renewal with "open PR already covers issue" — the very PR the
lock being renewed already owns. Add `owning_pr_renewal_evidence`, the mirror of
`issue_lock_recovery.owning_pr_recovery_evidence` (#755), and carry it into the
gate only when renewal was granted. It re-checks that the PR, local, and remote
heads agree, so truncated or hand-built evidence cannot authorize an exemption.

Neither waiver is caller-supplied and `gitea_lock_issue` still gains no
parameter. Absolute wall-clock expiry is unchanged. No #790 heartbeat, sliding
expiry, fencing-token, or shared-lifecycle behavior is introduced, and #753
recovery behavior is untouched.

F2 — add tests/test_issue_760_mcp_renewal_path.py, 10 cases driving the native
`gitea_lock_issue` path against a real git repository and a real durable lock:
expired lease, live recorded PID, committed non-base-equivalent branch. Proves
the renewal completes, records prior and replacement lease evidence, advances
the generation exactly once, produces a live lock that satisfies
`verify_lock_for_mutation`, and does not claim dead-session recovery. Negative
companions prove a foreign claimant, foreign profile, unpublished branch, or
mismatched PR head cannot use the waiver, that a dirty worktree still fails
closed, and that base-equivalence still applies with no waiver at all.

Tests: new MCP suite 10 passed; combined #760 suites 50 passed; lock/lease
regression set 200 passed with 2 subtests; full suite 4240 passed, 11 failed,
6 skipped, 493 subtests passed — the same 11 pre-existing failures as the
master baseline at 3d0c13fa, no new failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Ti8deB36iWcjHmE9cuxour
2026-07-22 01:10:48 -04:00

482 lines
18 KiB
Python

"""Exact-owner renewal of an expired author issue lease (#760).
An author issue lease carries an absolute wall-clock expiry stamped once at
lock time. The PID recorded alongside it is the long-lived MCP daemon, not the
authoring task, so a lease that expires while its daemon is still up is the
ordinary case for any author task that outlives the TTL — not an anomaly.
Before this module, that case was unreachable.
``issue_lock_store.assess_same_issue_lease_conflict`` computed same-owner
evidence and then returned on the expired branch before consulting it, and
``assess_expired_lock_reclaim`` only permits takeover on a dead PID or a
missing worktree. An exact owner whose daemon is alive and whose worktree is
present satisfied neither, so its own lock became permanently unmodifiable
through sanctioned tools.
This module is the pure evidence assessor for that one narrow case. It answers
a single question: may *this* session renew a lease it can prove it already
owns? It performs no mutation and no network I/O, and it never trusts a caller
assertion — every field is compared against durable lock state or a live
observation supplied by the caller and gathered server-side.
Deliberate boundaries:
* **Renewal is not takeover.** A refusal here never widens what
``assess_expired_lock_reclaim`` already allows; foreign expired locks keep
requiring a dead PID or missing worktree (#760 AC11), and a *live* foreign
lease stays non-recoverable by construction because only an expired lease is
ever a candidate (AC12).
* **PID liveness is never authorization.** A live recorded PID proves the
daemon is up, nothing more. It is recorded as evidence and is neither
necessary nor sufficient for renewal (AC16).
* **Absolute expiry is preserved.** Renewal issues a new absolute expiry from
the moment of the write. It does not introduce sliding heartbeat renewal,
lease generations as fencing tokens, or a shared cross-role lifecycle — that
is #790's scope and is deliberately not implemented here.
"""
from __future__ import annotations
import os
from typing import Any, Iterable, Mapping, Sequence
from issue_lock_store import AUTHOR_ISSUE_WORK_LEASE, is_lease_expired, is_process_alive
from reviewer_worktree import parse_dirty_tracked_files
# Outcome values.
RENEWAL_SANCTIONED = "RENEWAL_SANCTIONED"
NO_CANDIDATE = "NO_CANDIDATE"
REFUSED = "REFUSED"
# Durable fields a lock must carry before it can be considered at all.
REQUIRED_LOCK_FIELDS = ("issue_number", "branch_name", "worktree_path")
def _text(value: Any) -> str:
return str(value or "").strip()
def _same_realpath(left: str | None, right: str | None) -> bool:
if not left or not right:
return False
try:
return os.path.realpath(left) == os.path.realpath(right)
except OSError:
return left == right
def _lock_claimant(lock: Mapping[str, Any]) -> dict[str, Any]:
claimant = lock.get("claimant")
if not isinstance(claimant, Mapping):
lease = lock.get("work_lease")
claimant = lease.get("claimant") if isinstance(lease, Mapping) else None
return dict(claimant) if isinstance(claimant, Mapping) else {}
def _lock_lease(lock: Mapping[str, Any]) -> dict[str, Any]:
lease = lock.get("work_lease")
return dict(lease) if isinstance(lease, Mapping) else {}
def _lock_operation_type(lock: Mapping[str, Any]) -> str:
lease = _lock_lease(lock)
return _text(lease.get("operation_type")) or AUTHOR_ISSUE_WORK_LEASE
def _recorded_pid(lock: Mapping[str, Any]) -> Any:
pid = lock.get("session_pid")
if pid is None:
pid = lock.get("pid")
return pid
def _malformed_reasons(lock: Mapping[str, Any]) -> list[str]:
"""Names of durable fields that are missing or unusable."""
missing: list[str] = []
for field in REQUIRED_LOCK_FIELDS:
if not _text(lock.get(field)):
missing.append(field)
pid = _recorded_pid(lock)
if pid is None or _text(pid) == "":
missing.append("session_pid/pid")
else:
try:
if int(pid) <= 0:
missing.append("session_pid/pid")
except (TypeError, ValueError):
missing.append("session_pid/pid")
return missing
def _competing_lock_reasons(
competing_live_locks: Iterable[Mapping[str, Any]] | None,
*,
issue_number: int,
branch_name: str,
worktree_path: str,
) -> list[str]:
"""Live locks that would contend with this renewal (#760 AC7).
A live lock on the *same* issue cannot coexist with this expired lease, so
any live entry naming this issue, branch, or worktree belongs to somebody
else and refuses the renewal.
"""
reasons: list[str] = []
for entry in competing_live_locks or ():
if not isinstance(entry, Mapping):
continue
entry_issue = entry.get("issue_number")
entry_branch = _text(entry.get("branch_name"))
entry_worktree = _text(entry.get("worktree_path"))
if entry_issue == issue_number:
reasons.append(
f"a live lock already exists for issue #{issue_number} "
f"(pid {entry.get('pid')}); renewal would contend with it"
)
continue
if entry_branch and entry_branch == _text(branch_name):
reasons.append(
f"live lock for issue #{entry_issue} already holds branch "
f"'{branch_name}'"
)
if entry_worktree and _same_realpath(entry_worktree, worktree_path):
reasons.append(
f"live lock for issue #{entry_issue} already holds worktree "
f"'{worktree_path}'"
)
return reasons
def assess_exact_owner_lease_renewal(
existing_lock: Mapping[str, Any] | None,
*,
issue_number: int,
branch_name: str,
worktree_path: str,
remote: str,
org: str,
repo: str,
identity: str | None,
profile: str | None,
operation_type: str = AUTHOR_ISSUE_WORK_LEASE,
current_branch: str | None = None,
porcelain_status: str = "",
worktree_exists: bool = False,
head_sha: str | None = None,
remote_head_sha: str | None = None,
pr_head_sha: str | None = None,
pr_number: int | None = None,
competing_live_locks: Sequence[Mapping[str, Any]] | None = None,
candidate_branches: Sequence[str] | None = None,
current_pid: int | None = None,
now: Any = None,
) -> dict[str, Any]:
"""Decide whether an expired lease may be renewed by its exact owner.
Returns a disposition dict; it never raises and never mutates. A refusal
withholds permission, leaving every pre-existing guard to fail closed
exactly as before — this assessment can only ever *add* permission.
``NO_CANDIDATE`` means the situation is not an exact-owner renewal at all
(no lock, different issue, different operation, or an unexpired lease) and
the caller should carry on with its normal path. ``REFUSED`` means it looked
like one but the evidence did not hold, and ``reasons`` names exactly what
was missing.
"""
evidence: dict[str, Any] = {
"issue_number": issue_number,
"branch_name": branch_name,
"worktree_path": worktree_path,
"remote": remote,
"org": org,
"repo": repo,
"operation_type": operation_type,
"identity": identity,
"profile": profile,
}
def _result(outcome: str, reasons: list[str], **extra: Any) -> dict[str, Any]:
return {
"outcome": outcome,
"renewal_sanctioned": outcome == RENEWAL_SANCTIONED,
"is_candidate": outcome in (RENEWAL_SANCTIONED, REFUSED),
"reasons": reasons,
"evidence": {**evidence, **extra},
}
if not isinstance(existing_lock, Mapping) or not existing_lock:
return _result(NO_CANDIDATE, ["no existing lock to renew"])
if existing_lock.get("issue_number") != issue_number:
return _result(
NO_CANDIDATE,
[
f"existing lock is for issue #{existing_lock.get('issue_number')}, "
f"not #{issue_number}"
],
)
existing_operation = _lock_operation_type(existing_lock)
if existing_operation != operation_type:
return _result(
NO_CANDIDATE,
[
f"existing lease operation '{existing_operation}' is not "
f"'{operation_type}'"
],
)
# Only an *expired* lease is ever a renewal candidate. An unexpired lease —
# live, or stale by dead PID — is somebody else's problem: the first needs no
# renewal, and the second is #753's dead-session recovery. This is also what
# makes a live foreign lease non-recoverable here (#760 AC12).
if not is_lease_expired(existing_lock, now=now):
return _result(
NO_CANDIDATE,
["lease has not expired; renewal does not apply"],
)
malformed = _malformed_reasons(existing_lock)
if malformed:
return _result(
REFUSED,
["durable lock is missing or has unusable fields: " + ", ".join(malformed)],
)
lease = _lock_lease(existing_lock)
claimant = _lock_claimant(existing_lock)
recorded_pid = _recorded_pid(existing_lock)
prior_expires_at = _text(lease.get("expires_at"))
# #760 AC16: recorded purely as evidence. A live daemon PID is neither
# necessary nor sufficient for renewal, and nothing below branches on it.
recorded_pid_alive = is_process_alive(recorded_pid)
extra: dict[str, Any] = {
"prior_pid": recorded_pid,
"prior_pid_alive": recorded_pid_alive,
"prior_expires_at": prior_expires_at,
"replacement_pid": current_pid,
"recorded_claimant": claimant,
"head_sha": head_sha,
"remote_head_sha": remote_head_sha,
"pr_head_sha": pr_head_sha,
"pr_number": pr_number,
}
reasons: list[str] = []
# ── AC3: exact ownership identity ──
if _text(existing_lock.get("remote")) != _text(remote):
reasons.append(
f"recorded remote '{existing_lock.get('remote')}' does not match "
f"'{remote}'"
)
if _text(existing_lock.get("org")) != _text(org):
reasons.append(
f"recorded org '{existing_lock.get('org')}' does not match '{org}'"
)
if _text(existing_lock.get("repo")) != _text(repo):
reasons.append(
f"recorded repo '{existing_lock.get('repo')}' does not match '{repo}'"
)
if _text(existing_lock.get("branch_name")) != _text(branch_name):
reasons.append(
f"recorded branch '{existing_lock.get('branch_name')}' does not match "
f"'{branch_name}'"
)
if not _same_realpath(_text(existing_lock.get("worktree_path")), worktree_path):
reasons.append(
f"recorded worktree '{existing_lock.get('worktree_path')}' does not "
f"match '{worktree_path}'"
)
recorded_identity = _text(claimant.get("username"))
recorded_profile = _text(claimant.get("profile"))
if not recorded_identity or not recorded_profile:
reasons.append(
"durable lock does not record both a claimant username and profile"
)
if recorded_identity and recorded_identity != _text(identity):
reasons.append(
f"recorded claimant '{recorded_identity}' does not match active "
f"identity '{_text(identity) or 'unknown'}'"
)
if recorded_profile and recorded_profile != _text(profile):
reasons.append(
f"recorded profile '{recorded_profile}' does not match active profile "
f"'{_text(profile) or 'unknown'}'"
)
# ── AC4: the registered worktree still exists, is on the branch, and is clean ──
if not worktree_exists:
reasons.append(f"declared worktree '{worktree_path}' does not exist")
if _text(current_branch) != _text(branch_name):
reasons.append(
f"worktree is on branch '{_text(current_branch) or 'unknown'}', not "
f"'{branch_name}'"
)
dirty = parse_dirty_tracked_files(porcelain_status or "")
if dirty:
reasons.append(
"worktree has uncommitted tracked changes: " + ", ".join(sorted(dirty))
)
# ── AC5/AC6: published heads must agree ──
if not _text(head_sha):
reasons.append("local head could not be observed")
if not _text(remote_head_sha):
reasons.append(
"remote branch head could not be observed; an unpublished branch "
"cannot prove exact-owner renewal"
)
if _text(head_sha) and _text(remote_head_sha) and head_sha != remote_head_sha:
reasons.append(
f"local head {head_sha} does not equal remote head {remote_head_sha}"
)
if pr_number is not None:
if not _text(pr_head_sha):
reasons.append(f"owning PR #{pr_number} head could not be observed")
elif _text(head_sha) and pr_head_sha != head_sha:
reasons.append(
f"owning PR #{pr_number} head {pr_head_sha} does not equal local "
f"head {head_sha}"
)
# ── AC7: nothing else claims this work ──
reasons.extend(
_competing_lock_reasons(
competing_live_locks,
issue_number=issue_number,
branch_name=branch_name,
worktree_path=worktree_path,
)
)
other_branches = [
name
for name in (candidate_branches or ())
if _text(name) and _text(name) != _text(branch_name)
]
if other_branches:
reasons.append(
"other branches already carry this issue marker: "
+ ", ".join(sorted(other_branches))
)
if reasons:
return _result(REFUSED, reasons, **extra)
return _result(
RENEWAL_SANCTIONED,
[
f"exact owner '{recorded_identity}' ({recorded_profile}) proved "
f"ownership of issue #{issue_number} on branch '{branch_name}' from "
f"worktree '{worktree_path}'; local, remote"
+ (f", and PR #{pr_number}" if pr_number is not None else "")
+ f" heads all equal {head_sha}; lease expired at "
f"{prior_expires_at or 'unknown'}"
],
**extra,
)
def owning_pr_renewal_evidence(
assessment: Mapping[str, Any] | None,
) -> dict[str, Any] | None:
"""Server-derived proof of the open PR a sanctioned renewal already owns.
The mirror of ``issue_lock_recovery.owning_pr_recovery_evidence`` (#755) for
the renewal disposition. An exact-owner renewal of a published branch is, by
construction, renewal of work that already has an open PR — so the
duplicate-work gate's linked-open-PR blocker would otherwise discard every
sanctioned renewal, exactly as it once discarded every sanctioned recovery.
Returns ``None`` unless renewal was actually granted and the evidence names
one owning PR whose head agrees with both the local and remote heads the
assessor accepted. Nothing is caller-supplied: every field is copied from
evidence built out of durable lock state plus live git/Gitea observation.
Renewal has no descendant case — it requires the local, remote, and PR heads
to be equal — so there is only one head to report.
"""
if not isinstance(assessment, Mapping):
return None
if assessment.get("outcome") != RENEWAL_SANCTIONED:
return None
if not assessment.get("renewal_sanctioned"):
return None
evidence = assessment.get("evidence") or {}
branch_name = _text(evidence.get("branch_name"))
pr_head = _text(evidence.get("pr_head_sha"))
local_head = _text(evidence.get("head_sha"))
remote_head = _text(evidence.get("remote_head_sha"))
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:
return None
try:
pr_number = int(raw_pr_number)
issue_number = int(evidence.get("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": pr_head,
"accepted_head": pr_head,
"head_relation": "equal",
}
def build_renewal_record(
assessment: Mapping[str, Any] | None,
*,
renewed_at: str,
new_expires_at: str,
) -> dict[str, Any]:
"""Durable audit record for a sanctioned renewal (#760 AC9).
Records both sides of the transition — prior PID and expiry, replacement PID
and new expiry — so a renewed lock is never mistakable for an original
claim, and so the evidence the waiver was granted on stays inspectable.
"""
data = dict(assessment or {})
evidence = dict(data.get("evidence") or {})
recorded_claimant = dict(evidence.get("recorded_claimant") or {})
return {
"renewed": bool(data.get("renewal_sanctioned")),
"renewed_at": renewed_at,
"prior_pid": evidence.get("prior_pid"),
"prior_pid_alive": evidence.get("prior_pid_alive"),
"prior_expires_at": evidence.get("prior_expires_at"),
"replacement_pid": evidence.get("replacement_pid"),
"new_expires_at": new_expires_at,
"identity": recorded_claimant.get("username"),
"profile": recorded_claimant.get("profile"),
"branch_name": evidence.get("branch_name"),
"worktree_path": evidence.get("worktree_path"),
"head_sha": evidence.get("head_sha"),
"remote_head_sha": evidence.get("remote_head_sha"),
"pr_head_sha": evidence.get("pr_head_sha"),
"pr_number": evidence.get("pr_number"),
"reason": "expired lease renewed by its exact recorded owner",
"proof": list(data.get("reasons") or []),
}
def format_renewal_refusal(assessment: Mapping[str, Any] | None) -> str:
"""One-line refusal summary for a blocked caller."""
data = dict(assessment or {})
reasons = list(data.get("reasons") or [])
if not reasons:
return "exact-owner lease renewal was not available (no evidence recorded)"
return "exact-owner lease renewal refused: " + "; ".join(reasons)