Files
Gitea-Tools/tests/test_issue_760_exact_owner_lease_renewal.py
T
sysadminandClaude Opus 4.8 1a97ced133 feat(lock): allow exact-owner renewal of expired author issue locks (Closes #760)
An expired author issue lease could not be renewed by the exact session that
already owned it. `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. Because the recorded PID is the long-lived MCP daemon rather than the
authoring task, a lease that expires under a live daemon is the ordinary case
for any author task outliving the TTL — and in that case the owner's own lock
became permanently unmodifiable through sanctioned tools.

Add `issue_lock_renewal`, a pure evidence assessor for that one case, and
evaluate its disposition before the expired foreign-takeover return.

Renewal requires an exact match of remote, org, repo, issue number, operation
type, branch, realpath-normalized worktree, claimant username, and claimant
profile; a registered worktree that exists, sits on the locked branch, and is
clean; local and remote heads that agree; and, when an owning PR exists, a PR
head that agrees too. No competing live lock, competing branch claim, or other
owning PR may exist. Any missing or contradictory evidence fails closed.

PID liveness is never authorization: it is recorded as evidence and is neither
necessary nor sufficient (AC16). Only an expired lease is ever a candidate, so
a live foreign lease stays non-recoverable (AC12) and dead-PID takeover keeps
its existing #601 conditions (AC11). Renewal is never caller-declarable — the
waiver is server-computed and `gitea_lock_issue` gains no parameter (AC14).

A sanctioned renewal records prior PID, prior expiry, replacement PID, new
expiry, claimant, and its supporting proof under `lease_renewal`, and reuses
the #772 compare-and-swap so two sessions observing the same expired lease
cannot both win.

Absolute wall-clock expiry is preserved. Sliding heartbeat renewal, fencing
tokens, and the shared cross-role lifecycle remain #790's scope and are
deliberately not implemented here; #790 stays sequenced behind this change.

Tests: 40 new cases covering the positive path, every near-match and
foreign-owner refusal, the non-candidate cases, the gate-ordering regression,
the renewal record, downstream mutation ownership, and AC14/AC17. Two #772 test
doubles now forward the new keyword.

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

448 lines
17 KiB
Python

"""Exact-owner renewal of an expired author issue lease (#760).
Covers the renewal disposition that lets the exact recorded owner re-acquire
its own lock after the wall-clock lease expires — including while the recording
MCP daemon PID is still alive — plus every rejection condition that must keep
failing closed, and the pre-existing dead-PID and live-foreign dispositions
that must remain untouched.
"""
import inspect
import os
import subprocess
import sys
import tempfile
import unittest
from datetime import datetime, timedelta, timezone
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import issue_lock_renewal # noqa: E402
import issue_lock_store # noqa: E402
ISSUE = 5150
BRANCH = f"fix/issue-{ISSUE}-demo"
WORKTREE = "/scratch/wt-5150"
HEAD = "c" * 40
OTHER_SHA = "d" * 40
IDENTITY = "example-user"
PROFILE = "example-author"
REMOTE = "prgs"
ORG = "ExampleOrg"
REPO = "ExampleRepo"
def dead_pid() -> int:
"""A PID that has certainly exited (spawned, then reaped)."""
proc = subprocess.Popen([sys.executable, "-c", "pass"])
proc.wait()
return proc.pid
def past_ts(hours: int = 1) -> str:
return (
(datetime.now(timezone.utc) - timedelta(hours=hours))
.isoformat()
.replace("+00:00", "Z")
)
def future_ts(hours: int = 4) -> str:
return (
(datetime.now(timezone.utc) + timedelta(hours=hours))
.isoformat()
.replace("+00:00", "Z")
)
def make_lock(*, expires_at: str | None = None, pid: int | None = None, **overrides):
"""An expired lock owned by a still-alive daemon PID — the #760 condition."""
lock = {
"issue_number": ISSUE,
"branch_name": BRANCH,
"worktree_path": WORKTREE,
"remote": REMOTE,
"org": ORG,
"repo": REPO,
# os.getpid() is unambiguously alive: the whole point of #760 is that
# daemon liveness is not evidence of an active author task.
"session_pid": os.getpid() if pid is None else pid,
"lock_generation": 3,
"work_lease": {
"operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE,
"issue_number": ISSUE,
"branch": BRANCH,
"worktree_path": WORKTREE,
"claimant": {"username": IDENTITY, "profile": PROFILE},
"created_at": past_ts(5),
"expires_at": expires_at or past_ts(),
},
}
lease_overrides = overrides.pop("work_lease", None)
if lease_overrides:
lock["work_lease"].update(lease_overrides)
lock.update(overrides)
return lock
def assess(lock=None, **overrides):
"""Run the assessor with all-passing evidence unless overridden."""
kwargs = {
"issue_number": ISSUE,
"branch_name": BRANCH,
"worktree_path": WORKTREE,
"remote": REMOTE,
"org": ORG,
"repo": REPO,
"identity": IDENTITY,
"profile": PROFILE,
"current_branch": BRANCH,
"porcelain_status": "",
"worktree_exists": True,
"head_sha": HEAD,
"remote_head_sha": HEAD,
"pr_head_sha": None,
"pr_number": None,
"competing_live_locks": [],
"candidate_branches": [BRANCH],
"current_pid": 4242,
}
kwargs.update(overrides)
return issue_lock_renewal.assess_exact_owner_lease_renewal(
make_lock() if lock is None else lock, **kwargs
)
class ExactOwnerRenewalGranted(unittest.TestCase):
"""AC1/AC3-AC7: the positive path."""
def test_expired_lease_alive_pid_exact_owner_is_renewable(self):
result = assess()
self.assertEqual(result["outcome"], issue_lock_renewal.RENEWAL_SANCTIONED)
self.assertTrue(result["renewal_sanctioned"])
self.assertTrue(result["is_candidate"])
def test_renewal_holds_when_owning_pr_head_matches(self):
result = assess(pr_number=999, pr_head_sha=HEAD)
self.assertTrue(result["renewal_sanctioned"])
def test_evidence_records_both_sides_of_the_transition(self):
result = assess()
evidence = result["evidence"]
self.assertEqual(evidence["prior_pid"], os.getpid())
self.assertTrue(evidence["prior_pid_alive"])
self.assertEqual(evidence["replacement_pid"], 4242)
self.assertTrue(evidence["prior_expires_at"])
class ExactOwnerRenewalRefused(unittest.TestCase):
"""AC3-AC8: every near-match must fail closed, one reason at a time."""
def _refused(self, **overrides):
result = assess(**overrides)
self.assertEqual(result["outcome"], issue_lock_renewal.REFUSED)
self.assertFalse(result["renewal_sanctioned"])
self.assertTrue(result["reasons"])
return result
def test_different_branch_refused(self):
result = self._refused(branch_name=f"fix/issue-{ISSUE}-other")
self.assertTrue(any("branch" in r for r in result["reasons"]))
def test_different_worktree_refused(self):
result = self._refused(worktree_path="/scratch/somewhere-else")
self.assertTrue(any("worktree" in r for r in result["reasons"]))
def test_different_claimant_refused(self):
result = self._refused(identity="someone-else")
self.assertTrue(any("claimant" in r for r in result["reasons"]))
def test_different_profile_refused(self):
result = self._refused(profile="other-author")
self.assertTrue(any("profile" in r for r in result["reasons"]))
def test_different_remote_org_or_repo_refused(self):
self._refused(remote="dadeschools")
self._refused(org="OtherOrg")
self._refused(repo="OtherRepo")
def test_dirty_worktree_refused(self):
result = self._refused(porcelain_status=" M gitea_mcp_server.py\n")
self.assertTrue(any("uncommitted" in r for r in result["reasons"]))
def test_missing_worktree_refused(self):
result = self._refused(worktree_exists=False)
self.assertTrue(any("does not exist" in r for r in result["reasons"]))
def test_worktree_on_wrong_branch_refused(self):
self._refused(current_branch="master")
def test_local_and_remote_head_mismatch_refused(self):
result = self._refused(remote_head_sha=OTHER_SHA)
self.assertTrue(
any("does not equal remote head" in r for r in result["reasons"])
)
def test_unpublished_branch_refused(self):
result = self._refused(remote_head_sha=None)
self.assertTrue(any("remote branch head" in r for r in result["reasons"]))
def test_pr_head_mismatch_refused(self):
result = self._refused(pr_number=999, pr_head_sha=OTHER_SHA)
self.assertTrue(any("does not equal local" in r for r in result["reasons"]))
def test_unobservable_pr_head_refused(self):
self._refused(pr_number=999, pr_head_sha=None)
def test_competing_live_lock_on_same_issue_refused(self):
result = self._refused(
competing_live_locks=[
{"issue_number": ISSUE, "branch_name": BRANCH, "pid": 777}
]
)
self.assertTrue(any("live lock" in r for r in result["reasons"]))
def test_competing_live_lock_holding_the_branch_refused(self):
self._refused(
competing_live_locks=[
{"issue_number": 111, "branch_name": BRANCH, "worktree_path": ""}
]
)
def test_competing_branch_claim_refused(self):
result = self._refused(candidate_branches=[BRANCH, f"feat/issue-{ISSUE}-rival"])
self.assertTrue(any("issue marker" in r for r in result["reasons"]))
def test_malformed_durable_lock_refused(self):
lock = make_lock()
lock["worktree_path"] = ""
result = assess(lock)
self.assertEqual(result["outcome"], issue_lock_renewal.REFUSED)
def test_lock_without_recorded_claimant_refused(self):
lock = make_lock()
lock["work_lease"]["claimant"] = {}
result = assess(lock)
self.assertEqual(result["outcome"], issue_lock_renewal.REFUSED)
class NotARenewalCandidate(unittest.TestCase):
"""AC12 and scope: situations renewal must decline to judge at all."""
def test_live_foreign_lease_is_never_a_candidate(self):
lock = make_lock(expires_at=future_ts())
result = assess(lock, identity="someone-else")
self.assertEqual(result["outcome"], issue_lock_renewal.NO_CANDIDATE)
self.assertFalse(result["renewal_sanctioned"])
def test_unexpired_lease_is_never_a_candidate(self):
lock = make_lock(expires_at=future_ts())
result = assess(lock)
self.assertEqual(result["outcome"], issue_lock_renewal.NO_CANDIDATE)
def test_dead_pid_under_unexpired_lease_stays_with_753(self):
"""The opposite trigger; #760 must not re-own it."""
lock = make_lock(expires_at=future_ts(), pid=dead_pid())
result = assess(lock)
self.assertEqual(result["outcome"], issue_lock_renewal.NO_CANDIDATE)
def test_absent_lock_is_not_a_candidate(self):
result = assess({})
self.assertEqual(result["outcome"], issue_lock_renewal.NO_CANDIDATE)
def test_different_issue_is_not_a_candidate(self):
lock = make_lock()
lock["issue_number"] = ISSUE + 1
result = assess(lock)
self.assertEqual(result["outcome"], issue_lock_renewal.NO_CANDIDATE)
def test_different_operation_type_is_not_a_candidate(self):
lock = make_lock()
lock["work_lease"]["operation_type"] = "review_pr_work"
result = assess(lock)
self.assertEqual(result["outcome"], issue_lock_renewal.NO_CANDIDATE)
class DaemonPidIsNotTaskLiveness(unittest.TestCase):
"""AC16: a live recorded PID is never, by itself, authorization."""
def test_alive_pid_alone_does_not_authorize_renewal(self):
# Every ownership fact except the live PID is wrong.
result = assess(identity="someone-else", branch_name="fix/issue-1-nope")
self.assertEqual(result["outcome"], issue_lock_renewal.REFUSED)
self.assertTrue(result["evidence"]["prior_pid_alive"])
def test_renewal_does_not_require_a_dead_pid(self):
result = assess()
self.assertTrue(result["evidence"]["prior_pid_alive"])
self.assertTrue(result["renewal_sanctioned"])
def test_dead_pid_does_not_block_an_otherwise_exact_owner(self):
lock = make_lock(pid=dead_pid())
result = assess(lock)
self.assertTrue(result["renewal_sanctioned"])
class ConflictGateOrdering(unittest.TestCase):
"""AC2: the same-owner allowance is reachable on an expired lease.
These cases need a worktree that genuinely exists on disk. The #601 reclaim
affordance already permits takeover when the recorded worktree is missing,
so a fictional path would satisfy the gate for the wrong reason and never
exercise the ordering defect this issue is about.
"""
@classmethod
def setUpClass(cls):
cls._tmp = tempfile.TemporaryDirectory()
cls.worktree = cls._tmp.name
@classmethod
def tearDownClass(cls):
cls._tmp.cleanup()
def present_lock(self, **overrides):
return make_lock(worktree_path=self.worktree, **overrides)
def test_expired_same_owner_is_allowed_when_renewal_is_sanctioned(self):
block = issue_lock_store.assess_same_issue_lease_conflict(
self.present_lock(),
issue_number=ISSUE,
branch_name=BRANCH,
worktree_path=self.worktree,
renewal_sanctioned=True,
)
self.assertIsNone(block)
def test_expired_same_owner_still_blocks_without_the_waiver(self):
"""Regression for the ordering defect: no waiver, no change in behavior.
Live PID and a present worktree, so the #601 reclaim affordance refuses;
before #760 this was the permanent dead end for an exact owner.
"""
lock = self.present_lock()
self.assertFalse(
issue_lock_store.assess_expired_lock_reclaim(lock)["reclaim_allowed"]
)
block = issue_lock_store.assess_same_issue_lease_conflict(
lock,
issue_number=ISSUE,
branch_name=BRANCH,
worktree_path=self.worktree,
)
self.assertIsNotNone(block)
self.assertIn("Recovery review is required", block)
def test_waiver_does_not_unlock_a_different_owner(self):
"""AC11: the waiver is scoped by same_owner, not merely by its own flag."""
block = issue_lock_store.assess_same_issue_lease_conflict(
self.present_lock(),
issue_number=ISSUE,
branch_name=f"fix/issue-{ISSUE}-someone-else",
worktree_path=self.worktree,
renewal_sanctioned=True,
)
self.assertIsNotNone(block)
self.assertIn("Recovery review is required", block)
def test_live_lease_disposition_is_unchanged(self):
"""AC12: a live foreign lease still blocks, waiver or not."""
block = issue_lock_store.assess_same_issue_lease_conflict(
self.present_lock(expires_at=future_ts()),
issue_number=ISSUE,
branch_name=f"fix/issue-{ISSUE}-someone-else",
worktree_path="/scratch/other",
renewal_sanctioned=True,
)
self.assertIsNotNone(block)
self.assertIn("already has an active", block)
def test_dead_pid_reclaim_path_is_unchanged(self):
"""AC11: expired + dead PID still reclaims through the #601 affordance."""
lock = self.present_lock(pid=dead_pid())
reclaim = issue_lock_store.assess_expired_lock_reclaim(lock)
self.assertTrue(reclaim["reclaim_allowed"])
block = issue_lock_store.assess_same_issue_lease_conflict(
lock,
issue_number=ISSUE,
branch_name=BRANCH,
worktree_path=self.worktree,
)
self.assertIsNone(block)
class RenewalRecordAndDownstream(unittest.TestCase):
"""AC9/AC10: durable audit trail, and a renewed lock that actually works."""
def test_record_captures_prior_and_replacement_state(self):
assessment = assess()
record = issue_lock_renewal.build_renewal_record(
assessment,
renewed_at="2026-01-01T00:00:00Z",
new_expires_at="2026-01-01T04:00:00Z",
)
self.assertTrue(record["renewed"])
self.assertEqual(record["prior_pid"], os.getpid())
self.assertEqual(record["new_expires_at"], "2026-01-01T04:00:00Z")
self.assertEqual(record["renewed_at"], "2026-01-01T00:00:00Z")
self.assertEqual(record["identity"], IDENTITY)
self.assertEqual(record["profile"], PROFILE)
self.assertTrue(record["prior_expires_at"])
self.assertTrue(record["proof"])
def test_renewed_lock_satisfies_verify_lock_for_mutation(self):
renewed = make_lock(expires_at=future_ts())
renewed["session_pid"] = os.getpid()
renewed["lease_renewal"] = {"renewed": True}
verdict = issue_lock_store.verify_lock_for_mutation(
renewed,
issue_number=ISSUE,
branch_name=BRANCH,
)
self.assertTrue(verdict["proven"])
self.assertFalse(verdict["block"])
def test_refusal_message_names_the_missing_evidence(self):
assessment = assess(porcelain_status=" M gitea_mcp_server.py\n")
message = issue_lock_renewal.format_renewal_refusal(assessment)
self.assertIn("refused", message)
self.assertIn("uncommitted", message)
class NoCallerControlledRenewalFlag(unittest.TestCase):
"""AC14: renewal eligibility is never declarable by a caller."""
def test_lock_issue_tool_exposes_no_renewal_parameter(self):
import gitea_mcp_server
target = gitea_mcp_server.gitea_lock_issue
target = getattr(target, "fn", getattr(target, "__wrapped__", target))
params = set(inspect.signature(target).parameters)
for forbidden in ("renewal_sanctioned", "renew", "allow_renewal", "is_owner"):
self.assertNotIn(forbidden, params)
def test_store_defaults_to_no_waiver(self):
params = inspect.signature(
issue_lock_store.assess_same_issue_lease_conflict
).parameters
self.assertIs(params["renewal_sanctioned"].default, False)
bind_params = inspect.signature(issue_lock_store.bind_session_lock).parameters
self.assertIs(bind_params["renewal_sanctioned"].default, False)
class NoIssueNumberSpecialCasing(unittest.TestCase):
"""AC17: no repository issue or PR number is special-cased."""
def test_module_contains_no_hardcoded_issue_special_cases(self):
source = inspect.getsource(issue_lock_renewal)
code = "\n".join(
line for line in source.splitlines() if not line.strip().startswith("#")
)
for literal in ("757", "759", "760"):
self.assertNotIn(f"== {literal}", code)
self.assertNotIn(f"issue_number == {literal}", code)
if __name__ == "__main__":
unittest.main()