fix(mcp): allow author-lock recovery after the owning session exits (Closes #753) #754

Merged
sysadmin merged 1 commits from fix/issue-753-dead-pid-author-lock-recovery into master 2026-07-18 20:03:27 -05:00
Owner

Closes #753.

Base: 0425bf9a4325a25b72739fba81604463e3e5e363

Problem

A durable author issue lock records the PID of the MCP session that took it. When that process exits, assess_lock_freshness marks the lock stale (live=false) even while its lease is still within TTL, so every ownership check that requires a live lock fails closed — including gitea_update_pr_branch_by_merge.

Re-taking the lock was unreachable for real work:

  • assess_issue_lock_worktree requires the worktree HEAD to match a base branch SHA (master/main/dev), and a branch that already carries commits is ahead of its base by construction.
  • assess_expired_lock_reclaim does not apply: assess_same_issue_lease_conflict only consults it once the lease has expired, so a dead PID under an unexpired lease never reaches it.
  • assess_own_branch_adoption already speaks of "lock recovery", but it runs at handler step 12 — after the base gate at step 9 — so it was never reached for a PR worktree.

Observed on PR #750 / issue #749: lock matched the issue, the exact head branch, and the exact registered worktree; worktree clean; local head, remote head, and PR head all e349839f; recorded pid 48545 dead; lease still ~55 minutes from expiry.

Design

New issue_lock_recovery.py — a pure assessor granting a narrow waiver only when all durable ownership evidence agrees:

Requirement Refused when
remote / org / repo / issue any mismatch
locked branch, and worktree actually on it different or detached
registered worktree path different path
worktree clean any tracked edit
local head == remote head == open PR head any divergence
claimant identity + profile different author
recorded PID demonstrably dead prior PID alive
no competing live lock or lease competitor present
no ambiguous branch claims multiple issue-marked branches
complete durable record malformed or missing fields

The waiver suppresses the base-match requirement and nothing else. Cleanliness and every other precondition still apply, and brand-new issue claims keep the full requirement.

A refused assessment never raises. It withholds the waiver and lets the pre-existing guard fail closed exactly as before, so recovery can only ever add permission — never remove a guard. Refusal reasons are appended to the block message so a caller sees the exact missing evidence instead of only the base text.

A completed recovery is stamped on the lock as dead_session_recovery with prior/replacement session PIDs, heads, and claimant, so the takeover is auditable and never looks like an original claim. Rebinding sets the live session PID, so the recovered lock satisfies verify_lock_for_mutation and downstream PR update paths.

Files changed

File Change
issue_lock_recovery.py New pure assessor + recovery record/refusal formatting
gitea_mcp_server.py Assess before the worktree gate; stamp provenance; surface refusals
issue_lock_worktree.py recovery_sanctioned kwarg waives only the base check
issue_lock_adoption.py Expose branch_carries_issue_marker publicly
tests/test_issue_753_dead_pid_lock_recovery.py New regression coverage

Tests

  • New focused suite — 33 passed
  • Issue-lock / adoption / store / provenance / registration / duplicate-gate / worktree / create-issue-guard — 120 passed
  • MCP server, commit payloads, handoff ledger, PR ownership, branch cleanup — 286 passed
  • Full suite — 3498 passed, 6 skipped, 2 failed

The 2 failures reproduce identically on pristine master 0425bf9a (verified in a throwaway baseline worktree), so they are pre-existing and not caused by this change:

  • test_issue_702_review_findings_f1_f6.py::TestF1RecoveryBeforeTerminalProbe::test_removed_worktree_recovers_before_probe

  • test_reconciler_supersession_close.py::TestReconcilerSupersessionMcpTool::test_tool_posts_comment_and_closes_superseded_pr_issue

  • git diff --check — clean

Safety

  • No config, launcher, credential, or memory changes
  • PR #750 / issue #749 and PR #746 / issue #745 untouched
  • Worktree and branch preserved for review
  • No review or merge performed in this session
Closes #753. Base: `0425bf9a4325a25b72739fba81604463e3e5e363` ## Problem A durable author issue lock records the PID of the MCP session that took it. When that process exits, `assess_lock_freshness` marks the lock `stale` (`live=false`) even while its lease is still within TTL, so every ownership check that requires a live lock fails closed — including `gitea_update_pr_branch_by_merge`. Re-taking the lock was unreachable for real work: * `assess_issue_lock_worktree` requires the worktree HEAD to match a base branch SHA (`master`/`main`/`dev`), and a branch that already carries commits is ahead of its base by construction. * `assess_expired_lock_reclaim` does not apply: `assess_same_issue_lease_conflict` only consults it once the lease has **expired**, so a dead PID under an unexpired lease never reaches it. * `assess_own_branch_adoption` already speaks of "lock recovery", but it runs at handler step 12 — after the base gate at step 9 — so it was never reached for a PR worktree. Observed on PR #750 / issue #749: lock matched the issue, the exact head branch, and the exact registered worktree; worktree clean; local head, remote head, and PR head all `e349839f`; recorded pid `48545` dead; lease still ~55 minutes from expiry. ## Design New `issue_lock_recovery.py` — a pure assessor granting a narrow waiver only when **all** durable ownership evidence agrees: | Requirement | Refused when | |---|---| | remote / org / repo / issue | any mismatch | | locked branch, and worktree actually on it | different or detached | | registered worktree path | different path | | worktree clean | any tracked edit | | local head == remote head == open PR head | any divergence | | claimant identity + profile | different author | | recorded PID demonstrably dead | prior PID alive | | no competing live lock or lease | competitor present | | no ambiguous branch claims | multiple issue-marked branches | | complete durable record | malformed or missing fields | The waiver suppresses the base-match requirement **and nothing else**. Cleanliness and every other precondition still apply, and brand-new issue claims keep the full requirement. **A refused assessment never raises.** It withholds the waiver and lets the pre-existing guard fail closed exactly as before, so recovery can only ever add permission — never remove a guard. Refusal reasons are appended to the block message so a caller sees the exact missing evidence instead of only the base text. A completed recovery is stamped on the lock as `dead_session_recovery` with prior/replacement session PIDs, heads, and claimant, so the takeover is auditable and never looks like an original claim. Rebinding sets the live session PID, so the recovered lock satisfies `verify_lock_for_mutation` and downstream PR update paths. ## Files changed | File | Change | |------|--------| | `issue_lock_recovery.py` | New pure assessor + recovery record/refusal formatting | | `gitea_mcp_server.py` | Assess before the worktree gate; stamp provenance; surface refusals | | `issue_lock_worktree.py` | `recovery_sanctioned` kwarg waives only the base check | | `issue_lock_adoption.py` | Expose `branch_carries_issue_marker` publicly | | `tests/test_issue_753_dead_pid_lock_recovery.py` | New regression coverage | ## Tests * New focused suite — **33 passed** * Issue-lock / adoption / store / provenance / registration / duplicate-gate / worktree / create-issue-guard — **120 passed** * MCP server, commit payloads, handoff ledger, PR ownership, branch cleanup — **286 passed** * Full suite — **3498 passed, 6 skipped, 2 failed** The 2 failures reproduce identically on pristine master `0425bf9a` (verified in a throwaway baseline worktree), so they are pre-existing and not caused by this change: * `test_issue_702_review_findings_f1_f6.py::TestF1RecoveryBeforeTerminalProbe::test_removed_worktree_recovers_before_probe` * `test_reconciler_supersession_close.py::TestReconcilerSupersessionMcpTool::test_tool_posts_comment_and_closes_superseded_pr_issue` * `git diff --check` — clean ## Safety * No config, launcher, credential, or memory changes * PR #750 / issue #749 and PR #746 / issue #745 untouched * Worktree and branch preserved for review * No review or merge performed in this session
jcwalker3 added 1 commit 2026-07-18 18:33:05 -05:00
A durable author issue lock records the PID of the MCP session that took it.
When that process exits, assess_lock_freshness marks the lock stale (live=False)
even while its lease is still within TTL, so every ownership check that needs a
live lock fails closed -- including gitea_update_pr_branch_by_merge.

Re-taking the lock was unreachable for real work. assess_issue_lock_worktree
requires the worktree to be base-equivalent to master/main/dev, and a branch
that already carries commits is ahead of its base by construction. The existing
assess_expired_lock_reclaim affordance does not apply either: it is only
consulted once the lease has expired, so a dead PID under an unexpired lease
never reaches it. assess_own_branch_adoption already speaks of "lock recovery",
but it runs after the base-equivalence gate and so was never reached.

This adds issue_lock_recovery, a pure assessor that grants a narrow waiver only
when every element of durable ownership still matches exactly and the recorded
process is demonstrably dead: same remote/org/repo/issue, same branch (and the
worktree actually on it), same registered worktree, clean worktree, local head
== remote head == open PR head, same claimant identity/profile, no competing
live lock or lease, and no ambiguous branch claims. A malformed or incomplete
lock record can never prove ownership.

The waiver suppresses base-equivalence and nothing else. Cleanliness and every
other precondition still apply, and brand-new issue claims keep the full
requirement. A refused assessment never raises: it withholds the waiver and
lets the pre-existing guard fail closed exactly as before, so recovery can only
ever add permission, never remove a guard. Refusal reasons are appended to the
block message so a caller sees the exact missing evidence.

A completed recovery is recorded on the lock as dead_session_recovery with the
prior and replacement session PIDs, heads, and claimant, so the takeover is
auditable and never looks like an original claim. Rebinding sets the live
session PID, so the recovered lock satisfies verify_lock_for_mutation and the
downstream PR update paths.

Validation:
* new tests/test_issue_753_dead_pid_lock_recovery.py -- 33 passed
* issue-lock, adoption, store, provenance, registration, duplicate-gate,
  worktree, create-issue-guard suites -- 120 passed
* MCP server, commit payloads, handoff ledger, PR ownership, branch cleanup
  suites -- 286 passed
* full suite -- 3498 passed, 6 skipped, 2 failed
* the same 2 failures reproduce identically on pristine master 0425bf9a
  (test_issue_702_review_findings_f1_f6 F1 recovery-before-probe and
  test_reconciler_supersession_close org/repo forwarding), so they are
  pre-existing and unrelated
* git diff --check clean

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01L9jMhtvTjm5EajofqqaR3F
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #754
issue: #753
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 71140-266c51604a50
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-754-3edeba4d-independent
phase: claimed
candidate_head: 3edeba4d7f
target_branch: master
target_branch_sha: 0425bf9a43
last_activity: 2026-07-19T00:43:47Z
expires_at: 2026-07-19T00:53:47Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #754 issue: #753 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 71140-266c51604a50 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-754-3edeba4d-independent phase: claimed candidate_head: 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce target_branch: master target_branch_sha: 0425bf9a4325a25b72739fba81604463e3e5e363 last_activity: 2026-07-19T00:43:47Z expires_at: 2026-07-19T00:53:47Z blocker: none
sysadmin approved these changes 2026-07-18 19:48:04 -05:00
sysadmin left a comment
Owner

Independent review — PR #754 @ 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce

Reviewer: sysadmin / prgs-reviewer (Grok)
Author: jcwalker3 (no self-review)
Base: 0425bf9a4325a25b72739fba81604463e3e5e363 · mergeable: true
Scope: 5 files only (issue_lock_recovery.py, gitea_mcp_server.py, issue_lock_worktree.py, issue_lock_adoption.py, tests/test_issue_753_dead_pid_lock_recovery.py)

Acceptance criteria (issue #753)

AC Verdict
AC1 exact ownership evidence PASS
AC2 rebind + recovery provenance PASS
AC3 fail-closed refusals PASS
AC4 waive only base-equivalence after proven recovery; new claims keep base PASS
AC5 recovered lock satisfies update-by-merge ownership PASS
AC6 durable secret-free recovery evidence PASS
AC7 regression suite (33 focused) PASS

Design notes (safe)

  • Recovery only for dead recorded PID when durable ownership matches; live prior PID blocks
  • Worktree cleanliness never waived
  • Refusal withholds waiver; does not weaken worktree guard
  • recovery_sanctioned is server-computed only (not caller-controlled)
  • Assessor pure; wiring supplies live branch/PR/porcelain/identity evidence

Validation (this session)

  • Focused #753 suite: 33 passed
  • Issue-lock / adoption / store / provenance / guards / ownership: 105 passed
  • MCP lock filter + registration / handoff / cleanup: 97+ passed
  • git diff --check clean; syntax compile clean
  • Full suite not re-run (author baseline failures on pristine 0425bf9 accepted)

Verdict

APPROVE — safe, complete, fail-closed for #753. Ready for a fresh merger session. No merge performed.

Canonical PR State

STATE:
PR #754 is open against master at head 3edeba4d7f with an independent APPROVE from prgs-reviewer (sysadmin). Mergeable true. No merger lease acquired.

WHO_IS_NEXT:
merger

NEXT_ACTION:
Fresh merger session (prgs-merger) adopts or acquires a merger lease, re-pins head 3edeba4d7f, re-runs pre-merge gates, and merges only with explicit operator authorization.

NEXT_PROMPT:

Merge PR #754 in Scaled-Tech-Consulting/Gitea-Tools on remote prgs in a fresh prgs-merger session. Pin head 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce. Do not re-review unless the head moved. Adopt/acquire merger lease only; do not use the author lock path. Confirm mergeable and approval at the pinned head before MERGE PR 754.

WHAT_HAPPENED:
Independent Grok reviewer (sysadmin/prgs-reviewer) inspected the five-file diff for dead-session author-lock recovery (#753), ran focused and affected suites, and submitted APPROVE at the exact pinned head via gitea_submit_pr_review on the gitea-reviewer MCP namespace.

WHY:
Issue #753 requires a native fail-closed recovery path when a durable author lock's recording PID is dead but ownership evidence still matches. The change is scoped, tested, and does not weaken cleanliness or brand-new claim base-equivalence.

ISSUE:
#753

HEAD_SHA:
3edeba4d7f

REVIEW_STATUS:
APPROVED

MERGE_READY:
true

BLOCKERS:
none

VALIDATION:
33 focused #753 tests passed; 105 issue-lock/ownership related tests passed; MCP registration/handoff/cleanup suites passed; git diff --check clean; py_compile clean

NATIVE_REVIEW_PROOF: transport=native_mcp; entrypoint=mcp_server; tool=gitea_submit_pr_review; namespace=gitea-reviewer; profile=prgs-reviewer; identity=sysadmin; head=3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce; session=71140-266c51604a50

LAST_UPDATED_BY:
reviewer (sysadmin / prgs-reviewer / Grok)

## Independent review — PR #754 @ `3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce` **Reviewer:** sysadmin / prgs-reviewer (Grok) **Author:** jcwalker3 (no self-review) **Base:** `0425bf9a4325a25b72739fba81604463e3e5e363` · mergeable: true **Scope:** 5 files only (`issue_lock_recovery.py`, `gitea_mcp_server.py`, `issue_lock_worktree.py`, `issue_lock_adoption.py`, `tests/test_issue_753_dead_pid_lock_recovery.py`) ### Acceptance criteria (issue #753) | AC | Verdict | |---|---| | AC1 exact ownership evidence | PASS | | AC2 rebind + recovery provenance | PASS | | AC3 fail-closed refusals | PASS | | AC4 waive only base-equivalence after proven recovery; new claims keep base | PASS | | AC5 recovered lock satisfies update-by-merge ownership | PASS | | AC6 durable secret-free recovery evidence | PASS | | AC7 regression suite (33 focused) | PASS | ### Design notes (safe) - Recovery only for dead recorded PID when durable ownership matches; live prior PID blocks - Worktree cleanliness never waived - Refusal withholds waiver; does not weaken worktree guard - recovery_sanctioned is server-computed only (not caller-controlled) - Assessor pure; wiring supplies live branch/PR/porcelain/identity evidence ### Validation (this session) - Focused #753 suite: 33 passed - Issue-lock / adoption / store / provenance / guards / ownership: 105 passed - MCP lock filter + registration / handoff / cleanup: 97+ passed - git diff --check clean; syntax compile clean - Full suite not re-run (author baseline failures on pristine 0425bf9 accepted) ### Verdict APPROVE — safe, complete, fail-closed for #753. Ready for a fresh merger session. No merge performed. ## Canonical PR State STATE: PR #754 is open against master at head 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce with an independent APPROVE from prgs-reviewer (sysadmin). Mergeable true. No merger lease acquired. WHO_IS_NEXT: merger NEXT_ACTION: Fresh merger session (prgs-merger) adopts or acquires a merger lease, re-pins head 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce, re-runs pre-merge gates, and merges only with explicit operator authorization. NEXT_PROMPT: ```text Merge PR #754 in Scaled-Tech-Consulting/Gitea-Tools on remote prgs in a fresh prgs-merger session. Pin head 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce. Do not re-review unless the head moved. Adopt/acquire merger lease only; do not use the author lock path. Confirm mergeable and approval at the pinned head before MERGE PR 754. ``` WHAT_HAPPENED: Independent Grok reviewer (sysadmin/prgs-reviewer) inspected the five-file diff for dead-session author-lock recovery (#753), ran focused and affected suites, and submitted APPROVE at the exact pinned head via gitea_submit_pr_review on the gitea-reviewer MCP namespace. WHY: Issue #753 requires a native fail-closed recovery path when a durable author lock's recording PID is dead but ownership evidence still matches. The change is scoped, tested, and does not weaken cleanliness or brand-new claim base-equivalence. ISSUE: #753 HEAD_SHA: 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce REVIEW_STATUS: APPROVED MERGE_READY: true BLOCKERS: none VALIDATION: 33 focused #753 tests passed; 105 issue-lock/ownership related tests passed; MCP registration/handoff/cleanup suites passed; git diff --check clean; py_compile clean NATIVE_REVIEW_PROOF: transport=native_mcp; entrypoint=mcp_server; tool=gitea_submit_pr_review; namespace=gitea-reviewer; profile=prgs-reviewer; identity=sysadmin; head=3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce; session=71140-266c51604a50 LAST_UPDATED_BY: reviewer (sysadmin / prgs-reviewer / Grok)
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #754
issue: #753
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 71140-266c51604a50
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-754-3edeba4d-independent
phase: released
candidate_head: 3edeba4d7f
target_branch: master
target_branch_sha: 0425bf9a43
last_activity: 2026-07-19T00:48:11Z
expires_at: 2026-07-19T00:58:11Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #754 issue: #753 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 71140-266c51604a50 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-754-3edeba4d-independent phase: released candidate_head: 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce target_branch: master target_branch_sha: 0425bf9a4325a25b72739fba81604463e3e5e363 last_activity: 2026-07-19T00:48:11Z expires_at: 2026-07-19T00:58:11Z blocker: manual-release
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #754
issue: #753
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 77700-897fe2a11e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/merge-pr-754-3edeba4d
phase: claimed
candidate_head: 3edeba4d7f
target_branch: master
target_branch_sha: 0425bf9a43
last_activity: 2026-07-19T01:03:15Z
expires_at: 2026-07-19T01:13:15Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #754 issue: #753 reviewer_identity: sysadmin profile: prgs-merger session_id: 77700-897fe2a11e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/merge-pr-754-3edeba4d phase: claimed candidate_head: 3edeba4d7f2ae320fb46f76b0e848c952b3cd9ce target_branch: master target_branch_sha: 0425bf9a4325a25b72739fba81604463e3e5e363 last_activity: 2026-07-19T01:03:15Z expires_at: 2026-07-19T01:13:15Z blocker: none
sysadmin merged commit 08f67007c5 into master 2026-07-18 20:03:27 -05:00
Owner

Stale #332 review-decision lock cleanup (#594)

Status: APPLIED

Manual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-19T01:03:27.847698+00:00` - last terminal: `approve` on PR #754 - PR state: `closed` (merged=True) - merge_commit_sha: `08f67007c5984d239e1f52684e9e47543ff41785` - prior live_mutations_count: `1` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#754