fix(mcp): recover clean unpublished author work after the owning session exits #772

Closed
opened 2026-07-20 13:48:28 -05:00 by jcwalker3 · 1 comment
Owner

Summary

An author claim whose work exists only as a clean local commit — never pushed, never turned into a PR — becomes permanently unrecoverable once the owning MCP session exits. Every existing recovery path is reachable only for published claims, because each one derives ownership from a remote branch head or an owning PR head. An unpublished claim has neither, so no sanctioned path can prove ownership that the control plane already recorded.

The result is that correct, clean, locally-committed author work is stranded: it cannot be published, and it cannot be legitimately abandoned without discarding valid work.

Observed live against master 0c2f45abb7c0db96c477e9a6db35a6728e654311, using issue #617 as evidence.

Observed state

Fact Value
Evidence issue #617, open, allocated through the canonical allocator
Assignment asn-3099d44fc0da4403
Lease lease-3ea53153ef714e58
Prior owner PID 68614 — dead (kill -0 reports no such process)
Branch fix/issue-617-mutation-budget-classifier
Registered worktree branches/issue-617-mutation-budget-classifier, clean
Preserved commit b46f0f9f138d569edbc73e0d36df4b34239f9934
Commit parent / recorded base 0568f44cb2d87e78fd394a27a670e33c84f7842f
Remote branch none — git ls-remote --heads prgs 'fix/issue-617*' returns empty
Open PR none — duplicate assessor reports linked_open_pr_count: 0, conflicting_branches: [], claim_status: not_claimed

The durable file-store lock record does exist and is internally consistent:

issue_number       617
branch_name        fix/issue-617-mutation-budget-classifier
worktree_path      branches/issue-617-mutation-budget-classifier
operation_type     author_issue_work
claimant           jcwalker3 / prgs-author
pid, session_pid   68614          (dead)
written_at         2026-07-20T18:15:04Z   by gitea_lock_issue
work_lease.expires_at   2026-07-20T22:15:04Z   (NOT expired)
work_lease.pr_number    null

b46f0f9f is clean, locally reachable, and a strict descendant of its recorded base 0568f44cgit merge-base master b46f0f9f returns 0568f44c, which is also the commit's immediate parent. No history was rewritten. Master has since advanced to 0c2f45ab, so the branch is ahead of its base and behind current master; neither fact contradicts ownership.

The gap

Three recovery dispositions exist, and none is reachable for an unpublished claim.

  1. Fresh gitea_lock_issue is refused by base-equivalence. assess_issue_lock_worktree requires the worktree HEAD to equal a base-branch SHA before a lock may be taken. A branch carrying a commit is ahead of its base by construction, so this can never be satisfied once work exists. This is the same structural wall documented in #753.

  2. Dead-session recovery (#753) cannot derive its required evidence. Its AC1 requires that the local head equal the remote branch head, and that an open PR head equal that same head when a PR exists. For an unpublished claim there is no remote branch and no PR, so the head-equality evidence is not derivable at all. The disposition does not reject on a mismatch — it has nothing to compare against.

  3. Descendant recovery (#768) is scoped to a recorded PR head. It relaxes strict head equality to strict descendancy, but the value it compares against is the recorded PR/remote head. With work_lease.pr_number = null and no remote ref, that comparison target does not exist.

The lease is unexpired and the PID is dead — the exact freshness combination #753 was built for — yet the publication evidence that path demands was never created, because the session died before publishing. Recovery is gated on an artifact that only publication produces, while publication is gated on recovery.

Net effect: the control plane holds a complete, self-consistent assignment, lease, lock, identity, branch, and registered worktree; the filesystem holds a clean descendant commit; and there is still no sanctioned route to publish.

Why this is not a duplicate

  • #753 (closed) — dead-PID under an unexpired lease. Same trigger, but its recovery requires local/remote/PR head agreement. This issue covers the case where those heads do not exist.
  • #768 (closed) — strict-descendant recovery. Relaxes the comparison but still against a recorded PR head; scoped to a published owning PR.
  • #755 (closed) — owning-PR duplicate gate during recovery. Presupposes an owning PR.
  • #760 (open) — expired lease under a live daemon PID, exact-owner renewal. Opposite liveness trigger, and also assumes published heads (its AC5/AC6). Do not re-own or merge scope.

The unpublished-claim case is the remaining uncovered quadrant: dead owner, unexpired lease, durable lock present, zero published artifacts.

Why the tempting workarounds are wrong

Each launders false evidence through a guard and must stay blocked:

  • pushing the branch by raw git push to manufacture a remote head — bypasses the publication gates the lock exists to protect;
  • hand-editing the durable lock JSON to insert a pr_number or head — fabricates ownership evidence;
  • resetting the worktree back to its base to satisfy base-equivalence — discards valid committed work;
  • creating a throwaway base-equivalent worktree while the real work lives elsewhere — the escape already refused in #753;
  • restarting the daemon to clear the dead session — masks the condition without proving ownership;
  • opening the PR through the web UI — leaves the durable lock and the native gates unaware of the resulting state.

The fix must make the legitimate path reachable, not weaken the gates blocking these.

Root cause

Ownership recovery is implemented as re-derivation from published state. Every disposition reconstructs "is this still yours?" by comparing local head against a remote or PR head. That is sound for a published claim and vacuous for an unpublished one.

The control plane already holds sufficient server-derived evidence to answer the question without any published artifact: the assignment, the lease, the durable lock's claimant/profile/branch/worktree, the recorded base, the liveness of the recorded PID, and the ancestry of the current clean HEAD. That evidence is never consulted as a self-sufficient basis; it is treated only as a cross-check on published heads.

Acceptance criteria

AC1. Recovery is based entirely on server-derived control-plane evidence. No caller-provided recovery token, boolean, or any other parameter by which a session can declare its own recovery eligibility.

AC2. Recovery is permitted only when the prior author process is demonstrably dead and the repository, identity, profile, issue, branch, and registered worktree all match the prior assignment and lease. Any missing or contradictory element fails closed.

AC3. The registered worktree must be clean, its HEAD must descend from the recorded base, and its committed delta must remain locally reachable.

AC4. Recovery must reject: a live prior owner; a foreign identity or profile; a dirty worktree; a branch mismatch; a head unrelated to the recorded base; rewritten or force-moved ancestry; a competing live lock; and a competing PR or remote branch claiming the issue.

AC5. Successful recovery atomically creates a new durable issue lock with generation/CAS protection, so two replacement sessions cannot both recover the same claim.

AC6. After recovery, the #618 lock-derived author-worktree resolution must permit native commit, publication, and PR creation with no environment rebinding, no direct git push, and no web-UI intervention.

AC7. Tests cover the #617 shape — durable lock present, lease unexpired, PID dead, no remote branch, no PR — plus every negative case in AC4, without special-casing any issue number.

AC8. Existing published-owning-PR recovery behavior from #753 and #768 must not regress.

AC9. The diagnostic assessor must report the same recovery decision, and the same supporting evidence, as the mutating lock path. A divergence between assessor and mutator is itself a defect.

AC10. Documentation must distinguish unpublished-claim recovery from owning-PR recovery, including which evidence each one requires and why they cannot share a single head-comparison implementation.

Validation expectations

  • Focused unit tests for the unpublished-claim disposition, exercising each rejection reason independently.
  • An MCP-level regression through native gitea_lock_issue and the downstream publication tools, not only against assessor functions.
  • An explicit regression proving that absence of a remote head is not itself treated as permission — a foreign or mismatched unpublished claim must still be refused.
  • An explicit regression proving assessor/mutator agreement (AC9).
  • Full-suite results reported against the known pre-existing master drift baseline (#737), not as an absolute pass count.

Linkage and ownership boundary

  • #617 — evidence only. Its issue record, branch fix/issue-617-mutation-budget-classifier, worktree branches/issue-617-mutation-budget-classifier, preserved commit b46f0f9f, and baseline worktree branches/baseline-master-617 must not be modified, reallocated, published, reset, or cleaned as part of this work.
  • #618 — closed; author worktree binding disappearing mid-session with inconsistent guard enforcement. Its fix is loaded and correctly resolves an explicitly proven author worktree, but publication cannot reach that fix while ownership is unrecoverable. AC6 depends on it.
  • PR #771 — merged; durable author worktree resolution without control fallback, landing the #618 fix at master 0c2f45ab.
  • #753 — closed; dead-PID dead-session recovery under an unexpired lease. Introduced the recovery path this issue extends into the unpublished case.
  • #768 — closed; strict-descendant recovery for a published owning PR. AC8 protects it.
  • #760 — open; exact-owner renewal under a live daemon PID. Adjacent, different trigger. Do not re-own or merge scope.

Verify current line numbers and call sites during implementation rather than treating any reported location as immutable.

Evidence correction

The originating report stated that no durable issue-lock record exists for #617. That is not accurate. The lock file is present, complete, and quoted above; its lease had not expired at the time of filing. The actual unmet precondition is the absence of any determinable remote or PR head, which is what makes the #753 and #768 dispositions unreachable. This issue is scoped to that corrected finding.

The mutating gitea_lock_issue path was deliberately not invoked against #617 during this filing, because doing so would mutate the ownership state of a preserved evidence claim. The rejection analysis above is derived from the recorded lock contents, the verified repository state, and the documented preconditions of #753 and #768.

Canonical Issue State

STATE: ready-for-author
WHO_IS_NEXT: author
NEXT_ACTION: Lock this issue in a separately bounded author task, then implement AC1-AC10
NEXT_PROMPT: Author the unpublished-claim dead-session ownership recovery disposition and its assessor parity; PR; stop

Controller Handoff

[THREAD STATE LEDGER]
PHASE: issue-filed
ROLE: author (prgs-author / jcwalker3)
RUNTIME: master 0c2f45abb7c0db96c477e9a6db35a6728e654311, in_parity, control checkout clean
SCOPE: file one ownership-recovery defect; no implementation performed
GITEA_MUTATIONS: 1 (this issue creation)
LOCAL_MUTATIONS: none
DUPLICATE_SEARCH: open + closed issues reviewed; #753, #755, #760, #768 examined in full; no exact duplicate
EVIDENCE_PRESERVED: #617 issue, branch, worktree, commit b46f0f9f, baseline-master-617 — all untouched
BLOCKER_CLASS: none
WHO_IS_NEXT: author
NEXT_ACTION: separately bounded author task implements AC1-AC10 against this issue
## Summary An author claim whose work exists only as a clean local commit — never pushed, never turned into a PR — becomes permanently unrecoverable once the owning MCP session exits. Every existing recovery path is reachable only for *published* claims, because each one derives ownership from a remote branch head or an owning PR head. An unpublished claim has neither, so no sanctioned path can prove ownership that the control plane already recorded. The result is that correct, clean, locally-committed author work is stranded: it cannot be published, and it cannot be legitimately abandoned without discarding valid work. Observed live against master `0c2f45abb7c0db96c477e9a6db35a6728e654311`, using issue #617 as evidence. ## Observed state | Fact | Value | |---|---| | Evidence issue | #617, open, allocated through the canonical allocator | | Assignment | `asn-3099d44fc0da4403` | | Lease | `lease-3ea53153ef714e58` | | Prior owner PID | `68614` — dead (`kill -0` reports no such process) | | Branch | `fix/issue-617-mutation-budget-classifier` | | Registered worktree | `branches/issue-617-mutation-budget-classifier`, clean | | Preserved commit | `b46f0f9f138d569edbc73e0d36df4b34239f9934` | | Commit parent / recorded base | `0568f44cb2d87e78fd394a27a670e33c84f7842f` | | Remote branch | none — `git ls-remote --heads prgs 'fix/issue-617*'` returns empty | | Open PR | none — duplicate assessor reports `linked_open_pr_count: 0`, `conflicting_branches: []`, `claim_status: not_claimed` | The durable file-store lock record **does** exist and is internally consistent: ```text issue_number 617 branch_name fix/issue-617-mutation-budget-classifier worktree_path branches/issue-617-mutation-budget-classifier operation_type author_issue_work claimant jcwalker3 / prgs-author pid, session_pid 68614 (dead) written_at 2026-07-20T18:15:04Z by gitea_lock_issue work_lease.expires_at 2026-07-20T22:15:04Z (NOT expired) work_lease.pr_number null ``` `b46f0f9f` is clean, locally reachable, and a strict descendant of its recorded base `0568f44c` — `git merge-base master b46f0f9f` returns `0568f44c`, which is also the commit's immediate parent. No history was rewritten. Master has since advanced to `0c2f45ab`, so the branch is ahead of its base and behind current master; neither fact contradicts ownership. ## The gap Three recovery dispositions exist, and none is reachable for an unpublished claim. 1. **Fresh `gitea_lock_issue` is refused by base-equivalence.** `assess_issue_lock_worktree` requires the worktree HEAD to equal a base-branch SHA before a lock may be taken. A branch carrying a commit is ahead of its base by construction, so this can never be satisfied once work exists. This is the same structural wall documented in #753. 2. **Dead-session recovery (#753) cannot derive its required evidence.** Its AC1 requires that the local head equal the remote branch head, and that an open PR head equal that same head when a PR exists. For an unpublished claim there is no remote branch and no PR, so the head-equality evidence is not derivable at all. The disposition does not *reject* on a mismatch — it has nothing to compare against. 3. **Descendant recovery (#768) is scoped to a recorded PR head.** It relaxes strict head equality to strict descendancy, but the value it compares against is the recorded PR/remote head. With `work_lease.pr_number = null` and no remote ref, that comparison target does not exist. The lease is unexpired and the PID is dead — the exact freshness combination #753 was built for — yet the publication evidence that path demands was never created, because the session died before publishing. Recovery is gated on an artifact that only publication produces, while publication is gated on recovery. Net effect: the control plane holds a complete, self-consistent assignment, lease, lock, identity, branch, and registered worktree; the filesystem holds a clean descendant commit; and there is still no sanctioned route to publish. ## Why this is not a duplicate * **#753** (closed) — dead-PID under an unexpired lease. Same trigger, but its recovery requires local/remote/PR head agreement. This issue covers the case where those heads do not exist. * **#768** (closed) — strict-descendant recovery. Relaxes the comparison but still against a recorded PR head; scoped to a published owning PR. * **#755** (closed) — owning-PR duplicate gate during recovery. Presupposes an owning PR. * **#760** (open) — expired lease under a **live** daemon PID, exact-owner renewal. Opposite liveness trigger, and also assumes published heads (its AC5/AC6). Do not re-own or merge scope. The unpublished-claim case is the remaining uncovered quadrant: dead owner, unexpired lease, durable lock present, **zero published artifacts**. ## Why the tempting workarounds are wrong Each launders false evidence through a guard and must stay blocked: * pushing the branch by raw `git push` to manufacture a remote head — bypasses the publication gates the lock exists to protect; * hand-editing the durable lock JSON to insert a `pr_number` or head — fabricates ownership evidence; * resetting the worktree back to its base to satisfy base-equivalence — discards valid committed work; * creating a throwaway base-equivalent worktree while the real work lives elsewhere — the escape already refused in #753; * restarting the daemon to clear the dead session — masks the condition without proving ownership; * opening the PR through the web UI — leaves the durable lock and the native gates unaware of the resulting state. The fix must make the legitimate path reachable, not weaken the gates blocking these. ## Root cause Ownership recovery is implemented as *re-derivation from published state*. Every disposition reconstructs "is this still yours?" by comparing local head against a remote or PR head. That is sound for a published claim and vacuous for an unpublished one. The control plane already holds sufficient server-derived evidence to answer the question without any published artifact: the assignment, the lease, the durable lock's claimant/profile/branch/worktree, the recorded base, the liveness of the recorded PID, and the ancestry of the current clean HEAD. That evidence is never consulted as a self-sufficient basis; it is treated only as a cross-check on published heads. ## Acceptance criteria **AC1.** Recovery is based entirely on server-derived control-plane evidence. No caller-provided recovery token, boolean, or any other parameter by which a session can declare its own recovery eligibility. **AC2.** Recovery is permitted only when the prior author process is demonstrably dead **and** the repository, identity, profile, issue, branch, and registered worktree all match the prior assignment and lease. Any missing or contradictory element fails closed. **AC3.** The registered worktree must be clean, its HEAD must descend from the recorded base, and its committed delta must remain locally reachable. **AC4.** Recovery must reject: a live prior owner; a foreign identity or profile; a dirty worktree; a branch mismatch; a head unrelated to the recorded base; rewritten or force-moved ancestry; a competing live lock; and a competing PR or remote branch claiming the issue. **AC5.** Successful recovery atomically creates a new durable issue lock with generation/CAS protection, so two replacement sessions cannot both recover the same claim. **AC6.** After recovery, the #618 lock-derived author-worktree resolution must permit native commit, publication, and PR creation with no environment rebinding, no direct `git push`, and no web-UI intervention. **AC7.** Tests cover the #617 shape — durable lock present, lease unexpired, PID dead, no remote branch, no PR — plus every negative case in AC4, without special-casing any issue number. **AC8.** Existing published-owning-PR recovery behavior from #753 and #768 must not regress. **AC9.** The diagnostic assessor must report the same recovery decision, and the same supporting evidence, as the mutating lock path. A divergence between assessor and mutator is itself a defect. **AC10.** Documentation must distinguish unpublished-claim recovery from owning-PR recovery, including which evidence each one requires and why they cannot share a single head-comparison implementation. ## Validation expectations * Focused unit tests for the unpublished-claim disposition, exercising each rejection reason independently. * An MCP-level regression through native `gitea_lock_issue` and the downstream publication tools, not only against assessor functions. * An explicit regression proving that absence of a remote head is not itself treated as permission — a foreign or mismatched unpublished claim must still be refused. * An explicit regression proving assessor/mutator agreement (AC9). * Full-suite results reported against the known pre-existing master drift baseline (#737), not as an absolute pass count. ## Linkage and ownership boundary * **#617** — evidence only. Its issue record, branch `fix/issue-617-mutation-budget-classifier`, worktree `branches/issue-617-mutation-budget-classifier`, preserved commit `b46f0f9f`, and baseline worktree `branches/baseline-master-617` must not be modified, reallocated, published, reset, or cleaned as part of this work. * **#618** — closed; author worktree binding disappearing mid-session with inconsistent guard enforcement. Its fix is loaded and correctly resolves an explicitly proven author worktree, but publication cannot reach that fix while ownership is unrecoverable. AC6 depends on it. * **PR #771** — merged; durable author worktree resolution without control fallback, landing the #618 fix at master `0c2f45ab`. * **#753** — closed; dead-PID dead-session recovery under an unexpired lease. Introduced the recovery path this issue extends into the unpublished case. * **#768** — closed; strict-descendant recovery for a published owning PR. AC8 protects it. * **#760** — open; exact-owner renewal under a live daemon PID. Adjacent, different trigger. Do not re-own or merge scope. Verify current line numbers and call sites during implementation rather than treating any reported location as immutable. ## Evidence correction The originating report stated that no durable issue-lock record exists for #617. That is not accurate. The lock file is present, complete, and quoted above; its lease had not expired at the time of filing. The actual unmet precondition is the absence of any determinable remote or PR head, which is what makes the #753 and #768 dispositions unreachable. This issue is scoped to that corrected finding. The mutating `gitea_lock_issue` path was deliberately **not** invoked against #617 during this filing, because doing so would mutate the ownership state of a preserved evidence claim. The rejection analysis above is derived from the recorded lock contents, the verified repository state, and the documented preconditions of #753 and #768. ## Canonical Issue State ```text STATE: ready-for-author WHO_IS_NEXT: author NEXT_ACTION: Lock this issue in a separately bounded author task, then implement AC1-AC10 NEXT_PROMPT: Author the unpublished-claim dead-session ownership recovery disposition and its assessor parity; PR; stop ``` ## Controller Handoff ```text [THREAD STATE LEDGER] PHASE: issue-filed ROLE: author (prgs-author / jcwalker3) RUNTIME: master 0c2f45abb7c0db96c477e9a6db35a6728e654311, in_parity, control checkout clean SCOPE: file one ownership-recovery defect; no implementation performed GITEA_MUTATIONS: 1 (this issue creation) LOCAL_MUTATIONS: none DUPLICATE_SEARCH: open + closed issues reviewed; #753, #755, #760, #768 examined in full; no exact duplicate EVIDENCE_PRESERVED: #617 issue, branch, worktree, commit b46f0f9f, baseline-master-617 — all untouched BLOCKER_CLASS: none WHO_IS_NEXT: author NEXT_ACTION: separately bounded author task implements AC1-AC10 against this issue ```
jcwalker3 added status:pr-open and removed status:ready labels 2026-07-20 14:20:19 -05:00
Owner

Reviewer Handoff — PR #774 reviewed at ca76dacd73f6793e80837afbb36a4926597f997c

An independent reviewer (sysadmin / prgs-reviewer, distinct from author jcwalker3) reviewed PR #774 against this issue's AC1–AC10 and recorded a formal REQUEST_CHANGES verdict at the pinned head. Merge not performed, no code edited, no branch pushed.

Outcome

The implementation is sound. I found no defect in the shipped logic and I am not asking for any design change. AC1, AC2, AC3, AC4, AC5, AC7, AC8, and AC10 are independently proven. The verdict rests entirely on two acceptance criteria whose explicitly named validation requirements in this issue have no corresponding test.

F1 — AC9 assessor/mutator parity is untested. _evaluate_issue_lock_recovery is referenced by no test, and the lock_recovery key now returned by gitea_assess_work_issue_duplicate has zero assertions. Sharing one callee is good design, but the diagnostic projects a five-field subset of the assessment and converts probe failures to REFUSED where the mutating path raises. That projection and that divergence are precisely what AC9 exists to pin, and a future change to either would go undetected with the suite green.

F2 — AC6 has no MCP-level regression. All 33 new tests are unit-level. The PR offers its own publication as end-to-end proof, but PR #774 was published under a fresh lock on #772, not a recovered one — recovery_mode = unpublished_claim has never executed through gitea_lock_issue. This matters beyond bookkeeping: the recovery_sanctioned ? lock_generation(...) : None ternary that arms AC5's compare-and-swap is unguarded, and bind_session_lock skips CAS entirely on None. An inversion there silently disables AC5 in the only case it was written for, and test_second_recoverer_loses_the_race would not catch it because it exercises the store directly.

Both remedies are additive tests. Neither requires reworking the mechanism.

Evidence boundary honored

Issue #617, its branch, worktree, preserved commit b46f0f9f, branches/baseline-master-617, and its durable lock, assignment, and lease were not modified, published, reset, or cleaned. Issue #765 and PR #773 were not touched; no lease was acquired against either.

Canonical Issue State

STATE: pr-open-changes-requested
WHO_IS_NEXT: author
NEXT_ACTION: Remediate reviewer findings F1 and F2 on PR #774, push to the same branch, and request a fresh independent review at the new head
NEXT_PROMPT:

Remediate PR #774 review findings F1 and F2 at head ca76dacd73f6793e80837afbb36a4926597f997c.

F1: add an explicit regression proving gitea_assess_work_issue_duplicate and the gitea_lock_issue path report the same decision and the same evidence for one durable lock state, covering the projected lock_recovery fields and the documented probe-failure behavior of each.

F2: add an MCP-level regression driving gitea_lock_issue against a durable unpublished-claim lock with a dead PID, asserting recovery is sanctioned, branch and worktree are recorded unchanged, expected_generation is applied to the write, and a second replacement session loses the race through the tool.

Do not modify issue #617, its branch fix/issue-617-mutation-budget-classifier, its worktree, its preserved commit b46f0f9f138d569edbc73e0d36df4b34239f9934, branches/baseline-master-617, or its durable lock, assignment, or lease. Do not touch anything belonging to issue #765 or PR #773. Do not force-push or rewrite the published head; the escaped co-author trailer is accepted as cosmetic.

WHAT_HAPPENED: Independent reviewer sysadmin reviewed PR #774 at pinned head ca76dacd in a clean detached reviewer worktree. Verified AC1, AC2, AC3, AC4, AC5, AC7, AC8, and AC10 against the implementation, including that read_recorded_base takes no caller-reachable base input, that mode selection requires a positive absence observation rather than a failed head lookup, and that the lock_generation compare-and-swap sits inside the flock critical section with a re-read. Independently reproduced the focused, affected, and full-suite runs, and reproduced the 11 pre-existing failures at pristine base 0c2f45ab. Confirmed the published head tree is identical to the local commit tree, so the escaped co-author trailer is the only difference. Recorded request_changes at the pinned head for two test-coverage gaps against AC9 and AC6.
WHY: AC9 and AC6 are each backed by an explicitly named validation requirement in this issue that no test satisfies. The AC9 projection layer and probe-failure divergence in gitea_assess_work_issue_duplicate carry no assertions, and the AC6 gap leaves the recovery_sanctioned-to-expected_generation CAS wiring in gitea_lock_issue unguarded, so an inversion there would silently disable AC5 in the only case it was written for while the suite stayed green.
RELATED_PRS: PR #774 open for this issue, request_changes recorded at head ca76dacd. PR #771 merged, landed the #618 fix at master 0c2f45ab. PR #773 open for issue #765, foreign-owned and untouched by this review.
BLOCKERS: two reviewer findings, F1 (AC9 assessor/mutator parity regression absent) and F2 (AC6 MCP-level unpublished-claim recovery regression absent, leaving the AC5 CAS wiring unguarded). No implementation defect was found. The HTML-escaped co-author trailer is accepted as cosmetic and is explicitly not a blocker; no history rewrite is requested.
VALIDATION: Focused 33 passed. Affected 146 passed with 2 subtests passed. Full suite at PR head 3872 passed, 6 skipped, 11 failed. Independent baseline built in a pristine worktree at 0c2f45abb7 reproduced the identical 11 failures with 54 passed, then removed with a non-force git worktree remove. Zero failures attributable to this branch.
LAST_UPDATED_BY: reviewer (prgs-reviewer / sysadmin)

[THREAD STATE LEDGER]

What is true now:
  Server-side decision state: request_changes posted to gitea on PR #774 at head ca76dacd73f6793e80837afbb36a4926597f997c by sysadmin, verified visible through native read-after-write; merge not performed; this issue carries status:pr-open
  Local verdict/state: reviewer worktree branches/review-pr-774 clean and detached at the pinned head; merge-base against master equals the pinned base 0c2f45ab exactly; focused 33 pass, affected 146 pass, full suite 3872 pass with 11 pre-existing failures independently reproduced at pristine 0c2f45ab
  Runtime: server implementation in parity at ccfaa0ec0cb6c24159ae7795b51268289f04e9e5, control checkout clean on master
  Role: reviewer (prgs-reviewer / sysadmin), proven distinct from author jcwalker3

What changed:
  Server-side mutation ledger: 3 mutations — reviewer lease acquisition comment 13403; the request_changes review verdict on PR #774; this handoff comment
  Local-only changes: reviewer worktree branches/review-pr-774 created at the pinned head; a temporary baseline worktree created at 0c2f45ab and removed with a non-force git worktree remove
  Scope: review of PR #774 only; no code edited, no branch pushed, no PR created, merge not performed, no worktree belonging to another task touched

What is blocked:
  Blocker classification: test blocker
  Merge is withheld because AC9 and AC6 lack the explicit regressions this issue's validation expectations require. No implementation defect was found and no design change is requested.
  Accepted as cosmetic and explicitly not blocking: the HTML-escaped co-author trailer on the published commit. No history rewrite is requested.

Who/what acts next:
  Next actor: author
  Required action: add the F1 assessor/mutator parity regression and the F2 MCP-level unpublished-claim recovery regression, push, and request a fresh independent review at the new head
  Do not do: do not merge PR #774 on the strength of this review; do not force-push or rewrite the published head; do not modify issue #617, its branch fix/issue-617-mutation-budget-classifier, its worktree, its preserved commit b46f0f9f138d569edbc73e0d36df4b34239f9934, branches/baseline-master-617, or its durable lock, assignment, or lease; do not acquire a lease on, comment on, modify, or clean anything belonging to issue #765 or PR #773
## Reviewer Handoff — PR #774 reviewed at `ca76dacd73f6793e80837afbb36a4926597f997c` An independent reviewer (`sysadmin` / `prgs-reviewer`, distinct from author `jcwalker3`) reviewed PR #774 against this issue's AC1–AC10 and recorded a formal **REQUEST_CHANGES** verdict at the pinned head. Merge not performed, no code edited, no branch pushed. ### Outcome The implementation is sound. I found **no defect in the shipped logic** and I am not asking for any design change. AC1, AC2, AC3, AC4, AC5, AC7, AC8, and AC10 are independently proven. The verdict rests entirely on two acceptance criteria whose *explicitly named* validation requirements in this issue have no corresponding test. **F1 — AC9 assessor/mutator parity is untested.** `_evaluate_issue_lock_recovery` is referenced by no test, and the `lock_recovery` key now returned by `gitea_assess_work_issue_duplicate` has zero assertions. Sharing one callee is good design, but the diagnostic *projects* a five-field subset of the assessment and converts probe failures to `REFUSED` where the mutating path raises. That projection and that divergence are precisely what AC9 exists to pin, and a future change to either would go undetected with the suite green. **F2 — AC6 has no MCP-level regression.** All 33 new tests are unit-level. The PR offers its own publication as end-to-end proof, but PR #774 was published under a **fresh** lock on #772, not a recovered one — `recovery_mode = unpublished_claim` has never executed through `gitea_lock_issue`. This matters beyond bookkeeping: the `recovery_sanctioned ? lock_generation(...) : None` ternary that arms AC5's compare-and-swap is unguarded, and `bind_session_lock` skips CAS entirely on `None`. An inversion there silently disables AC5 in the only case it was written for, and `test_second_recoverer_loses_the_race` would not catch it because it exercises the store directly. Both remedies are additive tests. Neither requires reworking the mechanism. ### Evidence boundary honored Issue #617, its branch, worktree, preserved commit `b46f0f9f`, `branches/baseline-master-617`, and its durable lock, assignment, and lease were not modified, published, reset, or cleaned. Issue #765 and PR #773 were not touched; no lease was acquired against either. ## Canonical Issue State STATE: pr-open-changes-requested WHO_IS_NEXT: author NEXT_ACTION: Remediate reviewer findings F1 and F2 on PR #774, push to the same branch, and request a fresh independent review at the new head NEXT_PROMPT: ```text Remediate PR #774 review findings F1 and F2 at head ca76dacd73f6793e80837afbb36a4926597f997c. F1: add an explicit regression proving gitea_assess_work_issue_duplicate and the gitea_lock_issue path report the same decision and the same evidence for one durable lock state, covering the projected lock_recovery fields and the documented probe-failure behavior of each. F2: add an MCP-level regression driving gitea_lock_issue against a durable unpublished-claim lock with a dead PID, asserting recovery is sanctioned, branch and worktree are recorded unchanged, expected_generation is applied to the write, and a second replacement session loses the race through the tool. Do not modify issue #617, its branch fix/issue-617-mutation-budget-classifier, its worktree, its preserved commit b46f0f9f138d569edbc73e0d36df4b34239f9934, branches/baseline-master-617, or its durable lock, assignment, or lease. Do not touch anything belonging to issue #765 or PR #773. Do not force-push or rewrite the published head; the escaped co-author trailer is accepted as cosmetic. ``` WHAT_HAPPENED: Independent reviewer sysadmin reviewed PR #774 at pinned head ca76dacd in a clean detached reviewer worktree. Verified AC1, AC2, AC3, AC4, AC5, AC7, AC8, and AC10 against the implementation, including that read_recorded_base takes no caller-reachable base input, that mode selection requires a positive absence observation rather than a failed head lookup, and that the lock_generation compare-and-swap sits inside the flock critical section with a re-read. Independently reproduced the focused, affected, and full-suite runs, and reproduced the 11 pre-existing failures at pristine base 0c2f45ab. Confirmed the published head tree is identical to the local commit tree, so the escaped co-author trailer is the only difference. Recorded request_changes at the pinned head for two test-coverage gaps against AC9 and AC6. WHY: AC9 and AC6 are each backed by an explicitly named validation requirement in this issue that no test satisfies. The AC9 projection layer and probe-failure divergence in gitea_assess_work_issue_duplicate carry no assertions, and the AC6 gap leaves the recovery_sanctioned-to-expected_generation CAS wiring in gitea_lock_issue unguarded, so an inversion there would silently disable AC5 in the only case it was written for while the suite stayed green. RELATED_PRS: PR #774 open for this issue, request_changes recorded at head ca76dacd. PR #771 merged, landed the #618 fix at master 0c2f45ab. PR #773 open for issue #765, foreign-owned and untouched by this review. BLOCKERS: two reviewer findings, F1 (AC9 assessor/mutator parity regression absent) and F2 (AC6 MCP-level unpublished-claim recovery regression absent, leaving the AC5 CAS wiring unguarded). No implementation defect was found. The HTML-escaped co-author trailer is accepted as cosmetic and is explicitly not a blocker; no history rewrite is requested. VALIDATION: Focused 33 passed. Affected 146 passed with 2 subtests passed. Full suite at PR head 3872 passed, 6 skipped, 11 failed. Independent baseline built in a pristine worktree at 0c2f45abb7c0db96c477e9a6db35a6728e654311 reproduced the identical 11 failures with 54 passed, then removed with a non-force git worktree remove. Zero failures attributable to this branch. LAST_UPDATED_BY: reviewer (prgs-reviewer / sysadmin) ```text [THREAD STATE LEDGER] What is true now: Server-side decision state: request_changes posted to gitea on PR #774 at head ca76dacd73f6793e80837afbb36a4926597f997c by sysadmin, verified visible through native read-after-write; merge not performed; this issue carries status:pr-open Local verdict/state: reviewer worktree branches/review-pr-774 clean and detached at the pinned head; merge-base against master equals the pinned base 0c2f45ab exactly; focused 33 pass, affected 146 pass, full suite 3872 pass with 11 pre-existing failures independently reproduced at pristine 0c2f45ab Runtime: server implementation in parity at ccfaa0ec0cb6c24159ae7795b51268289f04e9e5, control checkout clean on master Role: reviewer (prgs-reviewer / sysadmin), proven distinct from author jcwalker3 What changed: Server-side mutation ledger: 3 mutations — reviewer lease acquisition comment 13403; the request_changes review verdict on PR #774; this handoff comment Local-only changes: reviewer worktree branches/review-pr-774 created at the pinned head; a temporary baseline worktree created at 0c2f45ab and removed with a non-force git worktree remove Scope: review of PR #774 only; no code edited, no branch pushed, no PR created, merge not performed, no worktree belonging to another task touched What is blocked: Blocker classification: test blocker Merge is withheld because AC9 and AC6 lack the explicit regressions this issue's validation expectations require. No implementation defect was found and no design change is requested. Accepted as cosmetic and explicitly not blocking: the HTML-escaped co-author trailer on the published commit. No history rewrite is requested. Who/what acts next: Next actor: author Required action: add the F1 assessor/mutator parity regression and the F2 MCP-level unpublished-claim recovery regression, push, and request a fresh independent review at the new head Do not do: do not merge PR #774 on the strength of this review; do not force-push or rewrite the published head; do not modify issue #617, its branch fix/issue-617-mutation-budget-classifier, its worktree, its preserved commit b46f0f9f138d569edbc73e0d36df4b34239f9934, branches/baseline-master-617, or its durable lock, assignment, or lease; do not acquire a lease on, comment on, modify, or clean anything belonging to issue #765 or PR #773 ```
jcwalker3 removed the status:pr-open label 2026-07-21 13:41:14 -05:00
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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