fix(gate): preserve exact-owner renewal evidence across duplicate rechecks (Closes #945)
An exact-owner lease renewal (#760) is granted the owning-PR duplicate-work
waiver inside gitea_lock_issue, but every later enforcement path rebuilt that
proof from the durable lock through
issue_lock_recovery.recovered_owning_pr_from_lock, which reads only the
dead_session_recovery block. A plain renewal persists its proof under
lease_renewal instead, so the waiver expired with the lock call and the next
author mutation was refused duplicate_commit_prevented with
owning_pr_recovery_exempted: false on the very PR the renewal had just proved.
Add issue_lock_renewal.owning_pr_renewal_from_lock as the renewal mirror of the
recovery rebuild, and resolve both halves through one helper,
_owning_pr_continuation_from_lock, in the same precedence gitea_lock_issue
applies. The three enforcement paths that previously saw only recovery evidence
now share that resolver: the commit/create-PR duplicate recheck, the read-only
duplicate assessor, and the push-ownership prover.
The rebuild re-applies the equality the renewal assessor required (PR head ==
local head == remote head) and additionally binds the record to the claimant the
lock names, so a renewal block cannot be reused under another identity, profile,
or workflow session. Validation of the resulting token against live PR state is
unchanged and still owned solely by
issue_work_duplicate_gate._assess_owning_pr_exemption, so repository, issue, PR,
branch and head binding continue to be enforced in exactly one place.
No public signature, MCP tool schema, refusal shape, or reason code changes.
Dead-session recovery behaviour is untouched.
Tests: tests/test_issue_945_owning_pr_renewal_continuation.py - 49 tests,
8 subtests, all in-memory (no durable branch, worktree, lock, lease or PR).
Against unmodified aab54d48 the suite is 47 failed / 2 passed; the 2 that pass
are the defect-pinning reproductions. Full suite 28F/5574P/6S/1002 subtests at
head vs 28F/5525P/6S/994 at base, with identical failing test id sets.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
@@ -436,6 +436,90 @@ def owning_pr_renewal_evidence(
|
||||
}
|
||||
|
||||
|
||||
def owning_pr_renewal_from_lock(
|
||||
lock_record: Mapping[str, Any] | None,
|
||||
) -> dict[str, Any] | None:
|
||||
"""Rebuild owning-PR renewal evidence from a persisted lock (#945).
|
||||
|
||||
The renewal mirror of ``issue_lock_recovery.recovered_owning_pr_from_lock``.
|
||||
``owning_pr_renewal_evidence`` supplies the waiver for the duration of the
|
||||
``gitea_lock_issue`` call only. The commit, push, create-PR, and
|
||||
duplicate-assessment gates run later in their own calls and re-derive
|
||||
ownership from the durable lock instead — so without this the open PR that
|
||||
renewal already proved belongs to this author reappears there as competing
|
||||
duplicate work, and the exact owner is refused with
|
||||
``duplicate_commit_prevented`` despite complete matching evidence.
|
||||
|
||||
This reads only the ``lease_renewal`` block that the server itself writes,
|
||||
on a lock the caller must already own. Like the recovery mirror it is a
|
||||
re-read of server-derived state, never a fresh assertion: a caller able to
|
||||
forge it could equally forge the lock file every other ownership gate
|
||||
already treats as authoritative.
|
||||
|
||||
Renewal has no descendant case — the assessor required the local, remote and
|
||||
PR heads to be equal — so that equality is re-checked here, and the record
|
||||
must still name the claimant the lock records.
|
||||
"""
|
||||
if not isinstance(lock_record, Mapping):
|
||||
return None
|
||||
record = lock_record.get("lease_renewal")
|
||||
if not isinstance(record, Mapping) or not record.get("renewed"):
|
||||
return None
|
||||
|
||||
branch_name = _text(record.get("branch_name")) or _text(
|
||||
lock_record.get("branch_name")
|
||||
)
|
||||
pr_head = _text(record.get("pr_head_sha"))
|
||||
local_head = _text(record.get("head_sha"))
|
||||
remote_head = _text(record.get("remote_head_sha"))
|
||||
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
|
||||
# The assessor required all three heads to agree before it granted renewal.
|
||||
# Re-check, so a truncated, drifted, or hand-built record cannot widen the
|
||||
# exemption past the single head the renewal disposition actually proved.
|
||||
if not local_head or not remote_head:
|
||||
return None
|
||||
if pr_head != local_head or pr_head != remote_head:
|
||||
return None
|
||||
# Renewal is refused outright unless the durable lock records both a
|
||||
# claimant username and profile, so a sanctioned record always carries them.
|
||||
# Requiring them to still agree keeps a renewal block from being reused
|
||||
# under an identity or profile the lock no longer names.
|
||||
claimant = lock_record.get("claimant")
|
||||
if not isinstance(claimant, Mapping):
|
||||
lease = lock_record.get("work_lease")
|
||||
claimant = lease.get("claimant") if isinstance(lease, Mapping) else None
|
||||
if not isinstance(claimant, Mapping):
|
||||
return None
|
||||
identity = _text(record.get("identity"))
|
||||
profile = _text(record.get("profile"))
|
||||
if not identity or identity != _text(claimant.get("username")):
|
||||
return None
|
||||
if not profile or profile != _text(claimant.get("profile")):
|
||||
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": pr_head,
|
||||
"accepted_head": pr_head,
|
||||
"head_relation": "equal",
|
||||
}
|
||||
|
||||
|
||||
def build_renewal_record(
|
||||
assessment: Mapping[str, Any] | None,
|
||||
*,
|
||||
|
||||
Reference in New Issue
Block a user