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
No Branch/Tag Specified
Labels
Clear labels
allocator
anti-stomp
architecture
bug
chore
codex
concurrency
contamination
control-plane
dashboard
database
design
documentation
enhancement
gitea
glitchtip
important
incident
incident-bridge
integration
jenkins
labels
leases
mcp
mcp-health
mcp-menu
multi-project
mutating
nice-to-have
observability
portability
preflight
protected-branch
queue
read-only
reconnect
recovery
refactor
release
reliability
resumable-review
reviewer
roadmap
safety
security
self-hosted
sentry
stale-runtime
status:blocked
status:in-progress
status:pr-open
status:ready
terminal-lock
testing
tracker
type:bug
type:feature
type:feature
type:guardrail
visibility
workflow
workflow-hardening
workflow-hardening
Controller-owned work allocator
Prevent concurrent LLM session stomping
Architecture / structural design
OpenAI Codex client / workflow session surface
Concurrent session safety
Workflow or session contamination incident
MCP control-plane coordination and allocation authority
MCP operational dashboard/queue view
Internal coordination storage (SQLite/Postgres)
Design / investigation, no implementation
Docs / runbooks
New feature or improvement
Gitea MCP workflow
GlitchTip integration
Operational or process incident requiring durable audit trail
Sentry-to-Gitea incident bridging
Integration testing
Jenkins integration
Label taxonomy management
Lease adopt/release/expire lifecycle
MCP server / tooling
MCP namespace and runtime health
MCP menu surface
Work spanning multiple monitoring projects or Gitea repos
Mutating action; requires gating
Observability, metrics, traces, error reporting
Cross-platform / portability
Shared preflight gates before mutation
Protected branch / stable-branch policy concern
Work queue visibility and allocation
Read-only, no mutation
MCP client reconnect/reload recovery path
Recovery paths for stale/foreign leases
Code refactor / restructure
Release / versioning
Reliability / failure handling
Persist and resume prepared review verdicts across sessions
Reviewer workflow tooling
Roadmap / umbrella issue
Safety rails and fail-closed mutation guards
Security / trust boundary
Self-hosted infrastructure integration
Sentry error monitoring integration
Stale backend daemon / runtime-vs-master parity failures
Issue is blocked
Issue is being worked on
Issue has an open pull request
Issue is ready for work
Terminal review lock (#332) path
Tests / test coverage
Issue tracker hygiene / meta
Bug or defect
Feature or enhancement
Feature or enhancement
Safety gate or guardrail
Workflow state visibility for LLMs/operators
Cross-tool workflow
LLM workflow coordination hardening
LLM workflow coordination hardening
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Scaled-Tech-Consulting/Gitea-Tools#772
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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
asn-3099d44fc0da4403lease-3ea53153ef714e5868614— dead (kill -0reports no such process)fix/issue-617-mutation-budget-classifierbranches/issue-617-mutation-budget-classifier, cleanb46f0f9f138d569edbc73e0d36df4b34239f99340568f44cb2d87e78fd394a27a670e33c84f7842fgit ls-remote --heads prgs 'fix/issue-617*'returns emptylinked_open_pr_count: 0,conflicting_branches: [],claim_status: not_claimedThe durable file-store lock record does exist and is internally consistent:
b46f0f9fis clean, locally reachable, and a strict descendant of its recorded base0568f44c—git merge-base master b46f0f9freturns0568f44c, which is also the commit's immediate parent. No history was rewritten. Master has since advanced to0c2f45ab, 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.
Fresh
gitea_lock_issueis refused by base-equivalence.assess_issue_lock_worktreerequires 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.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.
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 = nulland 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
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:
git pushto manufacture a remote head — bypasses the publication gates the lock exists to protect;pr_numberor head — fabricates ownership evidence;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
gitea_lock_issueand the downstream publication tools, not only against assessor functions.Linkage and ownership boundary
fix/issue-617-mutation-budget-classifier, worktreebranches/issue-617-mutation-budget-classifier, preserved commitb46f0f9f, and baseline worktreebranches/baseline-master-617must not be modified, reallocated, published, reset, or cleaned as part of this work.0c2f45ab.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_issuepath 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
Controller Handoff
Reviewer Handoff — PR #774 reviewed at
ca76dacd73f6793e80837afbb36a4926597f997cAn independent reviewer (
sysadmin/prgs-reviewer, distinct from authorjcwalker3) 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_recoveryis referenced by no test, and thelock_recoverykey now returned bygitea_assess_work_issue_duplicatehas zero assertions. Sharing one callee is good design, but the diagnostic projects a five-field subset of the assessment and converts probe failures toREFUSEDwhere 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_claimhas never executed throughgitea_lock_issue. This matters beyond bookkeeping: therecovery_sanctioned ? lock_generation(...) : Noneternary that arms AC5's compare-and-swap is unguarded, andbind_session_lockskips CAS entirely onNone. An inversion there silently disables AC5 in the only case it was written for, andtest_second_recoverer_loses_the_racewould 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:
WHAT_HAPPENED: Independent reviewer sysadmin reviewed PR #774 at pinned head
ca76dacdin 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 base0c2f45ab. 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 master0c2f45ab. 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
0c2f45abb7reproduced 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)