fix(author-lock): dead-session recovery cross-session PR base-sync (Closes #872) #880

Merged
sysadmin merged 2 commits from fix/issue-872-dead-session-two-base-syncs into master 2026-07-24 15:13:56 -05:00
Owner

Summary of Changes

Fixes Issue #872 by updating read_merge_sync_provenance in issue_lock_worktree.py and _evaluate_issue_lock_recovery in gitea_mcp_server.py to accept the remote parameter and fetch missing remote commit SHAs in worktree_path when assessing dead-session recovery.

  • Permits sanctioned dead-session recovery under HEAD_RELATION_REMOTE_MERGE_SYNCED after a prior server-side gitea_update_pr_branch_by_merge advanced the remote PR head.
  • Ensures a second base-sync across author sessions proceeds cleanly without deadlock, RuntimeError, or history rewriting.
  • Added comprehensive unit and integration tests covering multi-base-sync dead-session recovery (tests/test_issue_872_dead_session_two_base_syncs.py).

Closes #872.

### Summary of Changes Fixes Issue #872 by updating `read_merge_sync_provenance` in `issue_lock_worktree.py` and `_evaluate_issue_lock_recovery` in `gitea_mcp_server.py` to accept the `remote` parameter and fetch missing remote commit SHAs in `worktree_path` when assessing dead-session recovery. - Permits sanctioned dead-session recovery under `HEAD_RELATION_REMOTE_MERGE_SYNCED` after a prior server-side `gitea_update_pr_branch_by_merge` advanced the remote PR head. - Ensures a second base-sync across author sessions proceeds cleanly without deadlock, `RuntimeError`, or history rewriting. - Added comprehensive unit and integration tests covering multi-base-sync dead-session recovery (`tests/test_issue_872_dead_session_two_base_syncs.py`). Closes #872.
jcwalker3 added 1 commit 2026-07-24 07:45:21 -05:00
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #880
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 18798-f94661c47008
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-202607240951
phase: claimed
candidate_head: e42756b27f
target_branch: master
target_branch_sha: 2976c21ee6
last_activity: 2026-07-24T13:52:29Z
expires_at: 2026-07-24T14:02:29Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #880 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 18798-f94661c47008 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-202607240951 phase: claimed candidate_head: e42756b27f2b958b87b2f893a37daffb41e3a7b1 target_branch: master target_branch_sha: 2976c21ee6ce1feedb7101ccdf4c5c48e2c3bd18 last_activity: 2026-07-24T13:52:29Z expires_at: 2026-07-24T14:02:29Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #880
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 49636-f4b4854633a4
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-872-dead-session-two-base-syncs
phase: claimed
candidate_head: e42756b27f
target_branch: master
target_branch_sha: 2976c21ee6
last_activity: 2026-07-24T14:03:17Z
expires_at: 2026-07-24T14:13:17Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #880 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 49636-f4b4854633a4 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-872-dead-session-two-base-syncs phase: claimed candidate_head: e42756b27f2b958b87b2f893a37daffb41e3a7b1 target_branch: master target_branch_sha: 2976c21ee6ce1feedb7101ccdf4c5c48e2c3bd18 last_activity: 2026-07-24T14:03:17Z expires_at: 2026-07-24T14:13:17Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #880
issue: #872
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: prgs-reviewer-97460-b5db4219
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358
phase: claimed
candidate_head: e42756b27f
target_branch: master
target_branch_sha: 36fe4785ec
last_activity: 2026-07-24T14:44:17Z
expires_at: 2026-07-24T14:54:17Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #880 issue: #872 reviewer_identity: sysadmin profile: prgs-reviewer session_id: prgs-reviewer-97460-b5db4219 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358 phase: claimed candidate_head: e42756b27f2b958b87b2f893a37daffb41e3a7b1 target_branch: master target_branch_sha: 36fe4785eccf712b3bc0dec8583c798e8cf1f218 last_activity: 2026-07-24T14:44:17Z expires_at: 2026-07-24T14:54:17Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #880
issue: #872
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: prgs-reviewer-97460-b5db4219
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358
phase: validated
candidate_head: e42756b27f
target_branch: master
target_branch_sha: 36fe4785ec
last_activity: 2026-07-24T14:44:55Z
expires_at: 2026-07-24T14:54:55Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #880 issue: #872 reviewer_identity: sysadmin profile: prgs-reviewer session_id: prgs-reviewer-97460-b5db4219 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358 phase: validated candidate_head: e42756b27f2b958b87b2f893a37daffb41e3a7b1 target_branch: master target_branch_sha: 36fe4785eccf712b3bc0dec8583c798e8cf1f218 last_activity: 2026-07-24T14:44:55Z expires_at: 2026-07-24T14:54:55Z blocker: none
sysadmin approved these changes 2026-07-24 09:46:29 -05:00
Dismissed
sysadmin left a comment
Owner

Review: PR #880 — dead-session recovery cross-session PR base-sync (#872)

Verdict: APPROVE

Reviewer sysadmin / prgs-reviewer (not author jcwalker3). Head e42756b27f2b958b87b2f893a37daffb41e3a7b1 not already on master. Scope is three files: remote-aware merge-sync provenance fetch, recovery path wiring, and focused tests. Focused suite: 5 passed. Secret sweep clean. No reviewer file edits.

Review Metadata:

  • LLM-Agent-SHA: llm-a7f3c91e2b40
  • LLM-Role: reviewer
  • Authenticated-Gitea-User: sysadmin
  • MCP-Profile: prgs-reviewer
  • Eligibility: passed

Canonical PR State

STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merger assess sync status then merge only if merge_now at this head
NEXT_PROMPT:

As prgs-merger on PR #880 at head e42756b27f2b958b87b2f893a37daffb41e3a7b1: gitea_assess_pr_sync_status; if merge_now, adopt/acquire merger lease and merge; do not update author branch from merger; stop

WHAT_HAPPENED: Reviewer validated PR #880 against #872 and submitted APPROVE via native MCP gitea_submit_pr_review
WHY: Provenance fetch for cross-session merge-sync SHAs is correctly wired; focused tests pass; scope limited to issue #872; no secrets; author safety clean
ISSUE: #872
HEAD_SHA: e42756b27f
REVIEW_STATUS: approved
MERGE_READY: pending merger preflight
BLOCKERS: none
VALIDATION: focused tests/test_issue_872_dead_session_two_base_syncs.py — 5 passed; worktree clean; secret sweep clean; not already-landed
NATIVE_REVIEW_PROOF: transport=native_mcp; entrypoint=mcp_server; token_fingerprint=11d0a28f8701a278
LAST_UPDATED_BY: prgs-reviewer (sysadmin)

## Review: PR #880 — dead-session recovery cross-session PR base-sync (#872) **Verdict: APPROVE** Reviewer `sysadmin` / `prgs-reviewer` (not author `jcwalker3`). Head `e42756b27f2b958b87b2f893a37daffb41e3a7b1` not already on master. Scope is three files: remote-aware merge-sync provenance fetch, recovery path wiring, and focused tests. Focused suite: 5 passed. Secret sweep clean. No reviewer file edits. Review Metadata: - LLM-Agent-SHA: llm-a7f3c91e2b40 - LLM-Role: reviewer - Authenticated-Gitea-User: sysadmin - MCP-Profile: prgs-reviewer - Eligibility: passed ## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: Merger assess sync status then merge only if merge_now at this head NEXT_PROMPT: ```text As prgs-merger on PR #880 at head e42756b27f2b958b87b2f893a37daffb41e3a7b1: gitea_assess_pr_sync_status; if merge_now, adopt/acquire merger lease and merge; do not update author branch from merger; stop ``` WHAT_HAPPENED: Reviewer validated PR #880 against #872 and submitted APPROVE via native MCP gitea_submit_pr_review WHY: Provenance fetch for cross-session merge-sync SHAs is correctly wired; focused tests pass; scope limited to issue #872; no secrets; author safety clean ISSUE: #872 HEAD_SHA: e42756b27f2b958b87b2f893a37daffb41e3a7b1 REVIEW_STATUS: approved MERGE_READY: pending merger preflight BLOCKERS: none VALIDATION: focused tests/test_issue_872_dead_session_two_base_syncs.py — 5 passed; worktree clean; secret sweep clean; not already-landed NATIVE_REVIEW_PROOF: transport=native_mcp; entrypoint=mcp_server; token_fingerprint=11d0a28f8701a278 LAST_UPDATED_BY: prgs-reviewer (sysadmin)
jcwalker3 added 1 commit 2026-07-24 15:04:59 -05:00
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #880
issue: #872
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 73614-610a005605f3
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358
phase: claimed
candidate_head: 64e6d7b7df
target_branch: master
target_branch_sha: 35714258f0
last_activity: 2026-07-24T20:09:44Z
expires_at: 2026-07-24T20:19:44Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #880 issue: #872 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 73614-610a005605f3 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358 phase: claimed candidate_head: 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd target_branch: master target_branch_sha: 35714258f0560dddc96c4ceb8b36179322c48a78 last_activity: 2026-07-24T20:09:44Z expires_at: 2026-07-24T20:19:44Z blocker: none
sysadmin approved these changes 2026-07-24 15:10:39 -05:00
sysadmin left a comment
Owner

APPROVE — issue #872 verified at head 64e6d7b7df (post base-sync onto master 35714258f0). No blockers. Two non-blocking LOW observations are recorded below for a follow-up issue.

NATIVE_REVIEW_PROOF: reviewed in native worktree /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358 pinned at 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd; diff, source reads, caller tracing, and test execution performed against that checkout.

Scope

git diff 35714258 64e6d7b7 is 3 files, +358/-16: issue_lock_worktree.py (+41/-16), gitea_mcp_server.py (+1), tests/test_issue_872_dead_session_two_base_syncs.py (new, 332). One non-merge commit, e42756b. Nothing outside #872.

Behavior verified

  • read_merge_sync_provenance gains an optional remote parameter. When the synced head is not present in the local object store, it fetches the named remote, re-probes presence, and falls back to a bare git fetch before re-probing again. _evaluate_issue_lock_recovery passes remote=remote.
  • This is the exact gap #872 describes: after a server-side gitea_update_pr_branch_by_merge advanced the remote PR head, a dead-session worktree still sitting at the prior head could not observe the merge commit, so provenance came back unproven and recovery deadlocked.
  • No gate weakening. The proof conditions are unchanged: the synced commit must be a two-parent merge whose first parent is the prior head, and the disposition still lives in issue_lock_recovery. The new code only makes an already-required object locally readable; it never asserts a relation it has not observed.
  • Failure handling is fail-closed. Both fetches use check=False and the result is decided solely by re-probing object presence, so a failed fetch leaves synced_present false and provenance unproven.
  • No injection surface. Both calls use list-form subprocess.run (no shell). The remote value cannot be an arbitrary URL: gitea_lock_issue runs _resolve(remote, ...) before recovery evaluation, and _resolve raises ValueError: Unknown remote for anything outside the REMOTES allowlist, so only a configured instance name can reach git fetch.
  • Side effects are additive. The fetches write objects only; no refspec is given, so no local ref or worktree state is rewritten.

Test evidence (this session, at 64e6d7b7)

  • venv/bin/python -m pytest tests/test_issue_872_dead_session_two_base_syncs.py tests/test_issue_lock_worktree.py -q20 passed.

Coverage includes both the first and second base-sync recovery, plus three refusal paths that matter most here: dirty worktree blocks recovery, a foreign reclaimer blocks recovery, and a non-merge rebased remote head blocks recovery.

Non-blocking observations (LOW — follow-up issue, not merge blockers)

  1. No timeout on the network fetches. These are the first network-touching subprocess.run calls in issue_lock_worktree.py; the other thirteen are local git plumbing that cannot stall. An unreachable or slow remote can block lock recovery for as long as git waits. Suggest timeout= on both fetches with the timeout treated as "not present", which preserves the fail-closed result.
  2. Docstring overstates isolation. The rewritten docstring says comparisons are executed locally and "nothing is taken from caller parameters", which is now inaccurate: remote is a caller parameter and selects the fetch target. The allowlist makes it safe, but the sentence should say so rather than deny the parameter exists.

Canonical PR State

STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge pull request 880 at head 64e6d7b7df using the merger role, then close issue 872 and clean up the merged branch
NEXT_PROMPT:

Merge pull request 880 as prgs-merger at head 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd; the approval is recorded at that exact head and the branch is zero commits behind master 35714258f0560dddc96c4ceb8b36179322c48a78; stop after the merge

WHAT_HAPPENED: Reviewed pull request 880 at head 64e6d7b7df in a pinned reviewer worktree after the author base-synced the branch onto master 35714258, traced the caller chain for the new remote parameter, confirmed the provenance proof conditions and their fail-closed behavior are unchanged, ran the focused and lock-worktree test scopes, and approved at that exact head with two non-blocking LOW observations.
WHY: The change is required to break the #872 deadlock in which a dead author session could not complete a second cross-session base-sync recovery, because the merge commit produced server-side by gitea_update_pr_branch_by_merge was absent from the local object store and provenance therefore could not be proven; the fix supplies the missing object without relaxing any proof condition.
ISSUE: 872
HEAD_SHA: 64e6d7b7df
REVIEW_STATUS: approved
MERGE_READY: yes
BLOCKERS: none
VALIDATION: venv/bin/python -m pytest tests/test_issue_872_dead_session_two_base_syncs.py tests/test_issue_lock_worktree.py -q — 20 passed, run at 64e6d7b7df in the reviewer worktree. Scope confirmed as 3 files, +358/-16, all within #872.
LAST_UPDATED_BY: prgs-reviewer (sysadmin), session 73614-610a005605f3

APPROVE — issue #872 verified at head 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd (post base-sync onto master 35714258f0560dddc96c4ceb8b36179322c48a78). No blockers. Two non-blocking LOW observations are recorded below for a follow-up issue. NATIVE_REVIEW_PROOF: reviewed in native worktree /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358 pinned at 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd; diff, source reads, caller tracing, and test execution performed against that checkout. ## Scope `git diff 35714258 64e6d7b7` is 3 files, +358/-16: `issue_lock_worktree.py` (+41/-16), `gitea_mcp_server.py` (+1), `tests/test_issue_872_dead_session_two_base_syncs.py` (new, 332). One non-merge commit, `e42756b`. Nothing outside #872. ## Behavior verified - `read_merge_sync_provenance` gains an optional `remote` parameter. When the synced head is not present in the local object store, it fetches the named remote, re-probes presence, and falls back to a bare `git fetch` before re-probing again. `_evaluate_issue_lock_recovery` passes `remote=remote`. - This is the exact gap #872 describes: after a server-side `gitea_update_pr_branch_by_merge` advanced the remote PR head, a dead-session worktree still sitting at the prior head could not observe the merge commit, so provenance came back unproven and recovery deadlocked. - **No gate weakening.** The proof conditions are unchanged: the synced commit must be a two-parent merge whose first parent is the prior head, and the disposition still lives in `issue_lock_recovery`. The new code only makes an already-required object locally readable; it never asserts a relation it has not observed. - **Failure handling is fail-closed.** Both fetches use `check=False` and the result is decided solely by re-probing object presence, so a failed fetch leaves `synced_present` false and provenance unproven. - **No injection surface.** Both calls use list-form `subprocess.run` (no shell). The `remote` value cannot be an arbitrary URL: `gitea_lock_issue` runs `_resolve(remote, ...)` before recovery evaluation, and `_resolve` raises `ValueError: Unknown remote` for anything outside the `REMOTES` allowlist, so only a configured instance name can reach `git fetch`. - **Side effects are additive.** The fetches write objects only; no refspec is given, so no local ref or worktree state is rewritten. ## Test evidence (this session, at 64e6d7b7) - `venv/bin/python -m pytest tests/test_issue_872_dead_session_two_base_syncs.py tests/test_issue_lock_worktree.py -q` — **20 passed**. Coverage includes both the first and second base-sync recovery, plus three refusal paths that matter most here: dirty worktree blocks recovery, a foreign reclaimer blocks recovery, and a non-merge rebased remote head blocks recovery. ## Non-blocking observations (LOW — follow-up issue, not merge blockers) 1. **No timeout on the network fetches.** These are the first network-touching `subprocess.run` calls in `issue_lock_worktree.py`; the other thirteen are local git plumbing that cannot stall. An unreachable or slow remote can block lock recovery for as long as git waits. Suggest `timeout=` on both fetches with the timeout treated as "not present", which preserves the fail-closed result. 2. **Docstring overstates isolation.** The rewritten docstring says comparisons are executed locally and "nothing is taken from caller parameters", which is now inaccurate: `remote` is a caller parameter and selects the fetch target. The allowlist makes it safe, but the sentence should say so rather than deny the parameter exists. ## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: Merge pull request 880 at head 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd using the merger role, then close issue 872 and clean up the merged branch NEXT_PROMPT: ```text Merge pull request 880 as prgs-merger at head 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd; the approval is recorded at that exact head and the branch is zero commits behind master 35714258f0560dddc96c4ceb8b36179322c48a78; stop after the merge ``` WHAT_HAPPENED: Reviewed pull request 880 at head 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd in a pinned reviewer worktree after the author base-synced the branch onto master 35714258, traced the caller chain for the new remote parameter, confirmed the provenance proof conditions and their fail-closed behavior are unchanged, ran the focused and lock-worktree test scopes, and approved at that exact head with two non-blocking LOW observations. WHY: The change is required to break the #872 deadlock in which a dead author session could not complete a second cross-session base-sync recovery, because the merge commit produced server-side by gitea_update_pr_branch_by_merge was absent from the local object store and provenance therefore could not be proven; the fix supplies the missing object without relaxing any proof condition. ISSUE: 872 HEAD_SHA: 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd REVIEW_STATUS: approved MERGE_READY: yes BLOCKERS: none VALIDATION: venv/bin/python -m pytest tests/test_issue_872_dead_session_two_base_syncs.py tests/test_issue_lock_worktree.py -q — 20 passed, run at 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd in the reviewer worktree. Scope confirmed as 3 files, +358/-16, all within #872. LAST_UPDATED_BY: prgs-reviewer (sysadmin), session 73614-610a005605f3
Owner

adopted_at: 2026-07-24T20:13:28Z
adopted_by_identity: sysadmin
adopted_by_profile: prgs-merger
adopted_from_session_id: 73614-610a005605f3
adopted_from_profile: prgs-reviewer
adopted_from_reviewer_identity: sysadmin
adopted_from_comment_id: 16065
adoption_reason: merger-handoff-approved-head

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #880
issue: #872
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 73614-610a005605f3
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358
phase: adopted
candidate_head: 64e6d7b7df
target_branch: master
target_branch_sha: 35714258f0
last_activity: 2026-07-24T20:13:28Z
expires_at: 2026-07-24T20:23:28Z
blocker: none

<!-- mcp-review-lease-adoption:v1 --> adopted_at: 2026-07-24T20:13:28Z adopted_by_identity: sysadmin adopted_by_profile: prgs-merger adopted_from_session_id: 73614-610a005605f3 adopted_from_profile: prgs-reviewer adopted_from_reviewer_identity: sysadmin adopted_from_comment_id: 16065 adoption_reason: merger-handoff-approved-head <!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #880 issue: #872 reviewer_identity: sysadmin profile: prgs-merger session_id: 73614-610a005605f3 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr880-20260724144358 phase: adopted candidate_head: 64e6d7b7dfe0e5a74526e37f5e53bb88522401dd target_branch: master target_branch_sha: 35714258f0560dddc96c4ceb8b36179322c48a78 last_activity: 2026-07-24T20:13:28Z expires_at: 2026-07-24T20:23:28Z blocker: none
sysadmin merged commit 9b5289940c into master 2026-07-24 15:13:56 -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-24T20:13:58.492902+00:00` - last terminal: `approve` on PR #880 - PR state: `closed` (merged=True) - merge_commit_sha: `9b5289940c898d75e90b8d05612d942f36008fb8` - 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#880