#753 added the dead-session author-lock recovery assessor, but no sanctioned
recovery could ever reach the lock write. A dead-session lock is by construction
a lock for work that already has an open PR, and the #400 duplicate-work gate
blocked unconditionally on any linked open PR. gitea_lock_issue computed
recovery_sanctioned, then discarded it one gate later:
open PR #750 already covers issue #749 (fail closed)
Because the recovered lock was never persisted, it stayed non-live, so
_prove_author_ownership_for_pr found no live author lock and
gitea_update_pr_branch_by_merge failed closed too. Both blockers had a single
cause, so only the first one is fixed here.
issue_lock_recovery.owning_pr_recovery_evidence distils a granted assessment
into the minimum evidence the duplicate gate needs: issue, branch, owning PR
number, and head. It returns None unless the outcome is RECOVERY_SANCTIONED and
the assessment's own evidence agrees that local head == remote head == PR head,
so a partial or hand-built assessment cannot authorize anything.
The duplicate gate takes that evidence and re-checks every element against the
live PR list it was handed: exactly one linked open PR, matching number, head
branch, head SHA, and locked branch. Only that exact self-owned PR is exempt.
A different PR, several linked PRs, a different branch or head, or evidence for
another issue all keep failing closed, with a diagnostic naming which element
disagreed. The competing-branch, claim-lease and commit/push/create_pr arms are
untouched, and with no evidence the gate behaves exactly as before.
Ownership proof needed no change: once the recovered lock persists under the
live session pid, _prove_author_ownership_for_pr matches it as before.
Out of scope: the PHASE_COMMIT/PHASE_PUSH/PHASE_CREATE_PR rechecks still block
on a linked open PR for every author, recovered or not. That is pre-existing
#400 behavior, unrelated to lock recovery, and #755 does not cover it.
Tests
- tests/test_issue_755_owning_pr_recovery.py (new, 31 tests) drives the real
mcp_server.gitea_lock_issue handler, not only the pure assessor: sanctioned
recovery relocks, persists a live lease with truthful dead_session_recovery
provenance, and satisfies _prove_author_ownership_for_pr; competing PR,
multiple linked PRs, mismatched head/branch/worktree, dirty worktree, live
prior pid, and a fresh claim with no prior lock all stay blocked.
- Neutralizing owning_pr_recovery_evidence regresses all three success tests to
the exact production error above, so the coverage is load-bearing.
- Focused suites (755, 753, duplicate gates, lock registration, provenance) —
102 passed.
- Lock/ownership/worktree/handoff suites — 172 passed, 16 subtests.
- Full suite tests/ — 3529 passed, 6 skipped, 2 failed, 365 subtests.
- The same 2 failures reproduce on a pristine detached worktree at
08f67007c5 (3498 passed, 6 skipped, 2 failed):
test_issue_702_review_findings_f1_f6 F1 recovery-before-probe and
test_reconciler_supersession_close org/repo forwarding. Pre-existing.
- git diff --check clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_017BNsk3KUuFchaZyPJsCxjk
311 lines
11 KiB
Python
311 lines
11 KiB
Python
"""Early duplicate-work detection for author work-issue sessions (#400)."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import Any, Mapping
|
|
|
|
import issue_claim_heartbeat as claim_hb
|
|
|
|
PHASE_LOCK = "lock_issue"
|
|
PHASE_COMMIT = "commit"
|
|
PHASE_PUSH = "push"
|
|
PHASE_CREATE_PR = "create_pr"
|
|
|
|
OUTCOME_DUPLICATE_PR_PREVENTED = "duplicate_pr_prevented"
|
|
OUTCOME_DUPLICATE_BRANCH_PREVENTED = "duplicate_branch_prevented"
|
|
OUTCOME_DUPLICATE_COMMIT_PREVENTED = "duplicate_commit_prevented"
|
|
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED = "duplicate_work_not_prevented"
|
|
|
|
_ACTIVE_CLAIM_STATUSES = frozenset({"active", "awaiting_review"})
|
|
|
|
|
|
def _issue_pattern(issue_number: int) -> str:
|
|
return f"issue-{int(issue_number)}"
|
|
|
|
|
|
def _linked_open_pr(issue_number: int, open_prs: list[dict]) -> dict | None:
|
|
return claim_hb._linked_open_pr(issue_number, open_prs)
|
|
|
|
|
|
def _pr_links_issue(issue_number: int, pr: Mapping[str, Any]) -> bool:
|
|
"""Same linkage rule ``claim_hb._linked_open_pr`` applies, per PR.
|
|
|
|
``_linked_open_pr`` only yields the *first* match, which cannot answer
|
|
"is there exactly one linked PR?" — a question the owning-PR exemption
|
|
below must answer before it can trust any of them.
|
|
"""
|
|
pattern = _issue_pattern(issue_number)
|
|
head = (pr.get("head") or {}).get("ref") or ""
|
|
text = f"{pr.get('title', '')} {pr.get('body', '')}".lower()
|
|
if pattern in head.lower():
|
|
return True
|
|
return (
|
|
f"closes #{int(issue_number)}" in text
|
|
or f"fixes #{int(issue_number)}" in text
|
|
)
|
|
|
|
|
|
def _all_linked_open_prs(
|
|
issue_number: int, open_prs: list[dict]
|
|
) -> list[Mapping[str, Any]]:
|
|
return [pr for pr in (open_prs or []) if _pr_links_issue(issue_number, pr)]
|
|
|
|
|
|
def _assess_owning_pr_exemption(
|
|
issue_number: int,
|
|
*,
|
|
linked_open_prs: list[Mapping[str, Any]],
|
|
locked_branch: str | None,
|
|
recovered_owning_pr: Mapping[str, Any] | None,
|
|
) -> tuple[bool, list[str]]:
|
|
"""Is the linked open PR provably the one a sanctioned recovery owns (#755)?
|
|
|
|
``recovered_owning_pr`` is produced by
|
|
``issue_lock_recovery.owning_pr_recovery_evidence`` from a completed
|
|
server-side recovery assessment — it is never a caller-supplied field.
|
|
Every element is re-checked here against the live PR list this gate was
|
|
given, so a stale or partial token cannot widen the exemption.
|
|
|
|
Returns ``(exempt, diagnostic_reasons)``. Diagnostics are only emitted when
|
|
a token was offered and rejected, so a blocked caller can see which element
|
|
of ownership disagreed.
|
|
"""
|
|
if not recovered_owning_pr:
|
|
return False, []
|
|
|
|
notes: list[str] = []
|
|
token_issue = recovered_owning_pr.get("issue_number")
|
|
token_pr = recovered_owning_pr.get("pr_number")
|
|
token_branch = str(recovered_owning_pr.get("branch_name") or "").strip()
|
|
token_head = str(recovered_owning_pr.get("head_sha") or "").strip()
|
|
locked = (locked_branch or "").strip()
|
|
|
|
if token_issue is not None and int(token_issue) != int(issue_number):
|
|
notes.append(
|
|
f"recovery evidence is for issue #{token_issue}, not "
|
|
f"#{issue_number} (no owning-PR exemption)"
|
|
)
|
|
return False, notes
|
|
if not locked or not token_branch or locked != token_branch:
|
|
notes.append(
|
|
f"recovery evidence branch '{token_branch or 'unknown'}' does not "
|
|
f"match the branch being locked '{locked or 'unknown'}' "
|
|
"(no owning-PR exemption)"
|
|
)
|
|
return False, notes
|
|
if len(linked_open_prs) != 1:
|
|
numbers = ", ".join(
|
|
f"#{pr.get('number')}" for pr in linked_open_prs
|
|
) or "none"
|
|
notes.append(
|
|
f"{len(linked_open_prs)} open PRs link issue #{issue_number} "
|
|
f"({numbers}); recovery may only own exactly one "
|
|
"(no owning-PR exemption)"
|
|
)
|
|
return False, notes
|
|
|
|
only = linked_open_prs[0]
|
|
head_obj = only.get("head") or {}
|
|
only_number = only.get("number")
|
|
only_ref = str(head_obj.get("ref") or "").strip()
|
|
only_sha = str(head_obj.get("sha") or "").strip()
|
|
|
|
if token_pr is None or only_number is None or int(only_number) != int(token_pr):
|
|
notes.append(
|
|
f"linked open PR #{only_number} is not the recovered owning PR "
|
|
f"#{token_pr} (no owning-PR exemption)"
|
|
)
|
|
return False, notes
|
|
if only_ref != token_branch:
|
|
notes.append(
|
|
f"open PR #{only_number} head branch '{only_ref}' does not match "
|
|
f"the recovered branch '{token_branch}' (no owning-PR exemption)"
|
|
)
|
|
return False, notes
|
|
if not token_head or not only_sha or only_sha != token_head:
|
|
notes.append(
|
|
f"open PR #{only_number} head {only_sha or 'unknown'} does not "
|
|
f"match the recovered head {token_head or 'unknown'} "
|
|
"(no owning-PR exemption)"
|
|
)
|
|
return False, notes
|
|
|
|
return True, [
|
|
f"open PR #{only_number} is the exact PR already owned by the "
|
|
f"recovering lock for issue #{issue_number} (branch '{token_branch}', "
|
|
f"head {token_head}); not duplicate work"
|
|
]
|
|
|
|
|
|
def _matching_branches(
|
|
issue_number: int,
|
|
branch_names: list[str],
|
|
*,
|
|
locked_branch: str | None = None,
|
|
) -> list[str]:
|
|
pattern = _issue_pattern(issue_number)
|
|
matches = [
|
|
name for name in (branch_names or [])
|
|
if pattern in (name or "").lower()
|
|
]
|
|
if locked_branch:
|
|
locked = locked_branch.strip()
|
|
matches = [name for name in matches if name != locked]
|
|
return matches
|
|
|
|
|
|
def assess_work_issue_duplicate_gate(
|
|
issue_number: int,
|
|
*,
|
|
open_prs: list[dict] | None = None,
|
|
branch_names: list[str] | None = None,
|
|
claim_entry: dict | None = None,
|
|
locked_branch: str | None = None,
|
|
phase: str = PHASE_LOCK,
|
|
recovered_owning_pr: Mapping[str, Any] | None = None,
|
|
) -> dict[str, Any]:
|
|
"""Fail closed when duplicate work is already in flight for an issue.
|
|
|
|
``recovered_owning_pr`` (#755) is server-derived evidence that a sanctioned
|
|
dead-session lock recovery already owns one specific open PR. It exempts
|
|
*only* that exact PR from the linked-open-PR blocker; every other duplicate
|
|
signal, and every mismatch, keeps failing closed.
|
|
"""
|
|
reasons: list[str] = []
|
|
outcome = OUTCOME_DUPLICATE_WORK_NOT_PREVENTED
|
|
prs = list(open_prs or [])
|
|
branches = list(branch_names or [])
|
|
pattern = _issue_pattern(issue_number)
|
|
|
|
linked = _linked_open_pr(issue_number, prs)
|
|
linked_open_prs = _all_linked_open_prs(issue_number, prs)
|
|
owning_pr_exempted = False
|
|
exemption_notes: list[str] = []
|
|
if linked:
|
|
owning_pr_exempted, exemption_notes = _assess_owning_pr_exemption(
|
|
issue_number,
|
|
linked_open_prs=linked_open_prs,
|
|
locked_branch=locked_branch,
|
|
recovered_owning_pr=recovered_owning_pr,
|
|
)
|
|
if not owning_pr_exempted:
|
|
reasons.append(
|
|
f"open PR #{linked.get('number')} already covers issue "
|
|
f"#{issue_number} (fail closed)"
|
|
)
|
|
reasons.extend(exemption_notes)
|
|
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
|
|
|
|
conflicting_branches = _matching_branches(
|
|
issue_number, branches, locked_branch=locked_branch
|
|
)
|
|
if conflicting_branches:
|
|
names = ", ".join(conflicting_branches[:5])
|
|
reasons.append(
|
|
f"remote branch(es) already match issue pattern '{pattern}': "
|
|
f"{names} (fail closed)"
|
|
)
|
|
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
|
|
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
|
|
|
|
entry = claim_entry or {}
|
|
if entry.get("linked_open_pr") and not linked:
|
|
reasons.append(
|
|
f"claim inventory reports open PR #{entry['linked_open_pr']} "
|
|
f"for issue #{issue_number} (fail closed)"
|
|
)
|
|
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
|
|
|
|
status = (entry.get("status") or "").strip().lower()
|
|
if status in _ACTIVE_CLAIM_STATUSES and not linked:
|
|
heartbeat = entry.get("latest_heartbeat") or {}
|
|
claim_branch = (heartbeat.get("branch") or "").strip()
|
|
if locked_branch and claim_branch and claim_branch != locked_branch:
|
|
reasons.append(
|
|
f"active claim lease on branch '{claim_branch}' blocks "
|
|
f"work on '{locked_branch}' for issue #{issue_number} "
|
|
"(fail closed)"
|
|
)
|
|
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
|
|
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
|
|
elif not locked_branch and status == "active":
|
|
reasons.append(
|
|
f"issue #{issue_number} has an active claim lease "
|
|
"(fail closed)"
|
|
)
|
|
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
|
|
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
|
|
|
|
if phase in {PHASE_COMMIT, PHASE_PUSH} and reasons:
|
|
if outcome == OUTCOME_DUPLICATE_PR_PREVENTED:
|
|
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
|
|
elif outcome == OUTCOME_DUPLICATE_BRANCH_PREVENTED:
|
|
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
|
|
|
|
block = bool(reasons)
|
|
return {
|
|
"block": block,
|
|
"performed": not block,
|
|
"issue_number": issue_number,
|
|
"phase": phase,
|
|
"outcome": outcome,
|
|
"linked_open_pr": linked.get("number") if linked else entry.get("linked_open_pr"),
|
|
"linked_open_pr_count": len(linked_open_prs),
|
|
"owning_pr_recovery_exempted": owning_pr_exempted,
|
|
"owning_pr_recovery_notes": list(exemption_notes),
|
|
"conflicting_branches": conflicting_branches,
|
|
"claim_status": status or None,
|
|
"reasons": reasons,
|
|
"safe_next_action": (
|
|
"stop before mutating; preserve local work and produce a "
|
|
"reconciliation handoff if a concurrent PR appeared after push"
|
|
if block and phase == PHASE_CREATE_PR
|
|
else "stop before mutating; do not commit or push duplicate work"
|
|
if block
|
|
else "proceed"
|
|
),
|
|
}
|
|
|
|
|
|
def assess_work_issue_duplicate_report(report_text: str) -> dict[str, Any]:
|
|
"""Require explicit duplicate-work outcome wording in work-issue reports."""
|
|
text = (report_text or "").lower()
|
|
markers = {
|
|
OUTCOME_DUPLICATE_PR_PREVENTED: (
|
|
"duplicate pr prevented",
|
|
"duplicate_pr_prevented",
|
|
),
|
|
OUTCOME_DUPLICATE_BRANCH_PREVENTED: (
|
|
"duplicate branch prevented",
|
|
"duplicate_branch_prevented",
|
|
),
|
|
OUTCOME_DUPLICATE_COMMIT_PREVENTED: (
|
|
"duplicate commit prevented",
|
|
"duplicate_commit_prevented",
|
|
),
|
|
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: (
|
|
"duplicate work not prevented",
|
|
"duplicate_work_not_prevented",
|
|
"no duplicate work",
|
|
),
|
|
}
|
|
matched = [
|
|
key for key, phrases in markers.items()
|
|
if any(phrase in text for phrase in phrases)
|
|
]
|
|
if len(matched) != 1:
|
|
return {
|
|
"complete": False,
|
|
"downgraded": True,
|
|
"reasons": [
|
|
"work-issue report must state exactly one duplicate-work "
|
|
"outcome (duplicate PR/branch/commit prevented, or "
|
|
"duplicate work not prevented)"
|
|
],
|
|
}
|
|
return {
|
|
"complete": True,
|
|
"downgraded": False,
|
|
"outcome": matched[0],
|
|
"reasons": [],
|
|
} |