Every later enforcement path re-derives that proof from the durable lock instead, because the live assessment ended when the lock call returned. Three call sites did so, and all three asked the same recovery-only rebuild:
So a plain exact-owner renewal wrote lease_renewal, the gates looked for dead_session_recovery, and the waiver evaporated between gitea_lock_issue returning and the next author mutation:
Refinement on the issue's stated root cause.#945 describes the defect as _enforce_locked_issue_duplicate_recheck deriving its exemption only from recovered_owning_pr_from_lock. That is accurate but understates the blast radius: the same recovery-only rebuild was wired into three enforcement paths, not one, including the read-only assessor that reports the disposition back to the caller. Fixing only the commit recheck would have left the assessor and the push prover disagreeing with it. All three are corrected here.
owning_pr_renewal_evidence's own docstring already named the missing half — "the mirror of issue_lock_recovery.owning_pr_recovery_evidence (#755) for the renewal disposition". #760 built the mirror for the grant; nothing built it for the rebuild.
Implementation
+128 / −7 across two source files. No behaviour is removed.
issue_lock_renewal.owning_pr_renewal_from_lock (new) — the renewal mirror of recovered_owning_pr_from_lock. Reads only the server-written lease_renewal block, on a lock the caller must already own. Renewal has no descendant case, so it re-applies the equality the assessor required (pr_head == head_sha == remote_head_sha) and refuses anything else.
gitea_mcp_server._owning_pr_continuation_from_lock (new, private) — resolves recovery first, then renewal: the same precedence gitea_lock_issue applies, so the answer cannot drift between the gate that grants the waiver and the gates that enforce it.
The three call sites above now consume that one resolver. The only remaining direct call to the recovery-only rebuild is inside the resolver itself.
Deliberate hardening beyond a pure mirror. The recovery rebuild does not re-check the claimant; it relies on lock ownership alone. The renewal rebuild additionally requires the record's identity/profile to still equal the claimant the lock names (falling back to work_lease.claimant). Renewal is already refused outright unless the lock records both — issue_lock_renewal.py:294-298 — so a sanctioned record always carries them, and requiring agreement costs nothing while preventing a renewal block from being reused under an identity, profile, or workflow session the lock no longer names. This narrows the exemption; it never widens it.
Security invariants preserved
Validation of the resulting token against live PR state is untouched and remains the sole responsibility of issue_work_duplicate_gate._assess_owning_pr_exemption. This PR changes only which server-written block the token is rebuilt from, never what makes a token acceptable — so there is exactly one authoritative policy, as before.
That policy continues to fail closed on: a different issue, a locked-branch mismatch, anything other than exactly one linked open PR, a different PR number, a PR head branch mismatch, and a live PR head matching neither the recorded nor the accepted head.
Invariant
Status
An open PR alone grants no exemption
unchanged — proven by test
A second PR / foreign commit
still refused
Dead-session recovery (#753/#755/#768/#871)
unchanged, still exempts
Duplicate prevention for genuinely duplicate work
unchanged
Refusal reason codes, retryability, transport survival, audit notes
Public signatures, MCP tool schemas, return shapes
unchanged
A live confirmation of the fail-closed side arrived during this task: reclaiming the lapsed #945 lease wrote a real lease_renewal block with pr_number: null and pr_head_sha: null, because no PR existed yet. The new rebuild correctly yields no evidence from it — a renewal only produces an exemption when it actually names an owning PR.
Tests
tests/test_issue_945_owning_pr_renewal_continuation.py — 49 tests, 8 subtests. Every fixture is an in-memory mapping; the suite creates no branch, worktree, lock file, lease, comment, or PR, and one test asserts the rebuild does not mutate its input.
Coverage: granted rebuild and its exact token shape; 21 fail-closed cases (no lock, no renewal block, not granted, malformed block, local/remote/PR head divergence, missing heads, missing or malformed PR number, missing issue, unknown branch, identity/profile mismatch or absence, absent claimant, foreign-session evidence); resolver precedence including recovery-wins-over-renewal; all four phases agreeing; dead-session recovery still exempting; second PR, different PR, different branch, locked-branch mismatch, live head divergence, foreign issue, and sequential-task non-inheritance all refused; duplicate prevention retained; refusal and grant audit fields preserved.
# new suite @ head
49 passed, 8 subtests passed
# new suite @ unmodified aab54d48 ← the pre-fix reproduction
47 failed, 2 passed
The 2 that pass on base are TestPreFixReproduction::test_recovery_only_rebuild_cannot_see_a_renewal_lock and ::test_renewal_lock_produced_no_exemption_before_the_fix — they pin the defect itself, so they must pass on both sides. The other 47 fail on base with AttributeError, which is the wiring gap stated as an executable assertion.
# targeted: renewal, recovery, duplicate gate, lock/lease/heartbeat,
# anti-stomp, root-checkout, scope guard, stale-runtime, ownership (28 files)
head : 1 failed, 553 passed, 44 subtests
base : 1 failed, 504 passed, 36 subtests
# full suite, run from a branches/ worktree
head : 28 failed, 5574 passed, 6 skipped, 1002 subtests in 149.90s
base : 28 failed, 5525 passed, 6 skipped, 994 subtests in 148.91s
Failure classification, by test id rather than by count:
Introduced by #945: none.comm -23 of the sorted failing-id sets is empty; the two sets are byte-identical.
Reproduced on the clean base checkout: all 28. Standing repository baseline.
The single targeted failure, test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected, was run in isolation against the clean base checkout and fails identically there.
The +49 passes and +8 subtests over base are exactly this PR's new tests.
gitea_bootstrap_author_issue_worktree is still broken by #943, whose repair is the very thing #945 blocks from delivery, so it could not be used. Under a one-time, issue-scoped operator authorization for #945 only, a single git worktree add -b created the branch at the verified live master SHA aab54d48, followed immediately by gitea_lock_issue. The known-broken bootstrap capability was not called. Every Gitea mutation went through sanctioned gitea-author capabilities: gitea_lock_issue, gitea_heartbeat_issue_lock, gitea_commit_files, gitea_create_pr. No tea, no curl, no raw API, no direct database access, no manual push.
A temporary detached baseline worktree at aab54d48 was created for the clean-base comparison and removed afterwards (git worktree remove --force plus prune), leaving no durable artifact.
Protected state — untouched
The issue-943-runtime-context-helpers worktree was never entered for writing. Its three uncommitted files are byte-for-byte identical before and after this work:
PR #944 remains open at f49e781102b9f363834c28c055f69639d16290c9; review 622 is undismissed. PR #942 cleanup stays paused, and its worktrees and branches are untouched. Issues #931 and #941 received nothing. The stable control checkout remains clean on master at aab54d48. No stash was created; no MCP server was restarted or reconnected.
Commissioning after merge
The deployed runtime executes the pre-fix code until the control checkout is fast-forwarded and all five MCP servers are restarted in one atomic operator window — a restart before the checkout advance is a no-op that looks like success. Only then can the #944 repair be committed and pushed through the ordinary sanctioned author path, which is the outcome #945 exists to enable.
Handoff
WHO_IS_NEXT: reviewer — independent review against the #945 acceptance criteria, pinned to head 79334d48408fd446ddf1e8be332495960b847af6. Do not self-review and do not self-merge. Preserve the uncommitted #943 repair and keep PR #942 cleanup paused.
Closes #945
## Diagnosis
`gitea_lock_issue` grants the owning-PR duplicate-work waiver from **two** dispositions, and has since #760:
```python
recovered_owning_pr = (
issue_lock_recovery.owning_pr_recovery_evidence(recovery_assessment)
if recovery_sanctioned else None
)
if recovered_owning_pr is None and renewal_sanctioned:
recovered_owning_pr = issue_lock_renewal.owning_pr_renewal_evidence(
renewal_assessment
)
```
Every *later* enforcement path re-derives that proof from the durable lock instead, because the live assessment ended when the lock call returned. Three call sites did so, and all three asked the same recovery-only rebuild:
| Site | Path |
| --- | --- |
| `gitea_mcp_server.py:2859` | `_enforce_locked_issue_duplicate_recheck` → `gitea_commit_files`, `gitea_create_pr` |
| `gitea_mcp_server.py:5143` | `gitea_assess_work_issue_duplicate` (read-only assessor) |
| `gitea_mcp_server.py:19427` | push / PR-update ownership prover |
`issue_lock_recovery.recovered_owning_pr_from_lock` reads exactly one key:
```python
record = lock_record.get("dead_session_recovery")
if not isinstance(record, Mapping) or not record.get("recovered"):
return None
```
But the two dispositions persist under **different keys** — `gitea_mcp_server.py:4336` and `:4344`:
```python
data["dead_session_recovery"] = issue_lock_recovery.build_recovery_record(...)
data["lease_renewal"] = issue_lock_renewal.build_renewal_record(...)
```
So a plain exact-owner renewal wrote `lease_renewal`, the gates looked for `dead_session_recovery`, and the waiver evaporated between `gitea_lock_issue` returning and the next author mutation:
```text
outcome: duplicate_commit_prevented
owning_pr_recovery_exempted: false
owning_pr_recovery_notes: []
```
**Refinement on the issue's stated root cause.** #945 describes the defect as `_enforce_locked_issue_duplicate_recheck` deriving its exemption only from `recovered_owning_pr_from_lock`. That is accurate but understates the blast radius: the same recovery-only rebuild was wired into **three** enforcement paths, not one, including the read-only assessor that reports the disposition back to the caller. Fixing only the commit recheck would have left the assessor and the push prover disagreeing with it. All three are corrected here.
`owning_pr_renewal_evidence`'s own docstring already named the missing half — "the mirror of `issue_lock_recovery.owning_pr_recovery_evidence` (#755) for the renewal disposition". #760 built the mirror for the *grant*; nothing built it for the *rebuild*.
## Implementation
`+128 / −7` across two source files. No behaviour is removed.
* **`issue_lock_renewal.owning_pr_renewal_from_lock`** (new) — the renewal mirror of `recovered_owning_pr_from_lock`. Reads only the server-written `lease_renewal` block, on a lock the caller must already own. Renewal has no descendant case, so it re-applies the equality the assessor required (`pr_head == head_sha == remote_head_sha`) and refuses anything else.
* **`gitea_mcp_server._owning_pr_continuation_from_lock`** (new, private) — resolves recovery first, then renewal: the same precedence `gitea_lock_issue` applies, so the answer cannot drift between the gate that grants the waiver and the gates that enforce it.
* The three call sites above now consume that one resolver. The only remaining direct call to the recovery-only rebuild is inside the resolver itself.
**Deliberate hardening beyond a pure mirror.** The recovery rebuild does not re-check the claimant; it relies on lock ownership alone. The renewal rebuild additionally requires the record's `identity`/`profile` to still equal the claimant the lock names (falling back to `work_lease.claimant`). Renewal is already refused outright unless the lock records both — `issue_lock_renewal.py:294-298` — so a sanctioned record always carries them, and requiring agreement costs nothing while preventing a renewal block from being reused under an identity, profile, or workflow session the lock no longer names. This narrows the exemption; it never widens it.
## Security invariants preserved
Validation of the resulting token against **live** PR state is untouched and remains the sole responsibility of `issue_work_duplicate_gate._assess_owning_pr_exemption`. This PR changes only *which server-written block the token is rebuilt from*, never what makes a token acceptable — so there is exactly one authoritative policy, as before.
That policy continues to fail closed on: a different issue, a locked-branch mismatch, anything other than exactly one linked open PR, a different PR number, a PR head branch mismatch, and a live PR head matching neither the recorded nor the accepted head.
| Invariant | Status |
| --- | --- |
| An open PR alone grants no exemption | unchanged — proven by test |
| A second PR / foreign commit | still refused |
| Dead-session recovery (#753/#755/#768/#871) | unchanged, still exempts |
| Duplicate prevention for genuinely duplicate work | unchanged |
| Refusal reason codes, retryability, transport survival, audit notes | unchanged |
| Dirty-workspace, identity, profile, parity, role, expected-base, anti-stomp, scope enforcement | untouched |
| Public signatures, MCP tool schemas, return shapes | unchanged |
A live confirmation of the fail-closed side arrived during this task: reclaiming the lapsed #945 lease wrote a real `lease_renewal` block with `pr_number: null` and `pr_head_sha: null`, because no PR existed yet. The new rebuild correctly yields no evidence from it — a renewal only produces an exemption when it actually names an owning PR.
## Tests
`tests/test_issue_945_owning_pr_renewal_continuation.py` — **49 tests, 8 subtests**. Every fixture is an in-memory mapping; the suite creates no branch, worktree, lock file, lease, comment, or PR, and one test asserts the rebuild does not mutate its input.
Coverage: granted rebuild and its exact token shape; 21 fail-closed cases (no lock, no renewal block, not granted, malformed block, local/remote/PR head divergence, missing heads, missing or malformed PR number, missing issue, unknown branch, identity/profile mismatch or absence, absent claimant, foreign-session evidence); resolver precedence including recovery-wins-over-renewal; all four phases agreeing; dead-session recovery still exempting; second PR, different PR, different branch, locked-branch mismatch, live head divergence, foreign issue, and sequential-task non-inheritance all refused; duplicate prevention retained; refusal and grant audit fields preserved.
```text
# new suite @ head
49 passed, 8 subtests passed
# new suite @ unmodified aab54d48 ← the pre-fix reproduction
47 failed, 2 passed
```
The 2 that pass on base are `TestPreFixReproduction::test_recovery_only_rebuild_cannot_see_a_renewal_lock` and `::test_renewal_lock_produced_no_exemption_before_the_fix` — they pin the defect itself, so they must pass on both sides. The other 47 fail on base with `AttributeError`, which is the wiring gap stated as an executable assertion.
```text
# targeted: renewal, recovery, duplicate gate, lock/lease/heartbeat,
# anti-stomp, root-checkout, scope guard, stale-runtime, ownership (28 files)
head : 1 failed, 553 passed, 44 subtests
base : 1 failed, 504 passed, 36 subtests
# full suite, run from a branches/ worktree
head : 28 failed, 5574 passed, 6 skipped, 1002 subtests in 149.90s
base : 28 failed, 5525 passed, 6 skipped, 994 subtests in 148.91s
```
Failure classification, by test id rather than by count:
* **Introduced by #945: none.** `comm -23` of the sorted failing-id sets is empty; the two sets are byte-identical.
* **Reproduced on the clean base checkout: all 28.** Standing repository baseline.
* The single targeted failure, `test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected`, was run in isolation against the clean base checkout and fails identically there.
The `+49` passes and `+8` subtests over base are exactly this PR's new tests.
## Scope
* Files: `gitea_mcp_server.py`, `issue_lock_renewal.py`, `tests/test_issue_945_owning_pr_renewal_continuation.py`
* Diff: 3 files, `+593 / −7`
* Branch: `fix/issue-945-owning-pr-renewal-evidence`
* Base: `master` at `aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218`
* Head: `79334d48408fd446ddf1e8be332495960b847af6`
* Commit parent verified equal to the base SHA
* Worktree: `branches/issue-945-owning-pr-renewal-evidence`
## Author worktree provenance
`gitea_bootstrap_author_issue_worktree` is still broken by #943, whose repair is the very thing #945 blocks from delivery, so it could not be used. Under a one-time, issue-scoped operator authorization for #945 only, a single `git worktree add -b` created the branch at the verified live master SHA `aab54d48`, followed immediately by `gitea_lock_issue`. The known-broken bootstrap capability was not called. Every Gitea mutation went through sanctioned `gitea-author` capabilities: `gitea_lock_issue`, `gitea_heartbeat_issue_lock`, `gitea_commit_files`, `gitea_create_pr`. No `tea`, no `curl`, no raw API, no direct database access, no manual push.
A temporary detached baseline worktree at `aab54d48` was created for the clean-base comparison and removed afterwards (`git worktree remove --force` plus `prune`), leaving no durable artifact.
## Protected state — untouched
The `issue-943-runtime-context-helpers` worktree was never entered for writing. Its three uncommitted files are byte-for-byte identical before and after this work:
```text
d40d824b62fdb88e2a16e70cf99f0fa0578dcae35994084c1ef37bc807f15467 author_issue_bootstrap.py
21622e099d0c37cf4e7ecfdd3ba94f8639ce07ed60c835bf425515f6eb4331f3 gitea_mcp_server.py
208f87dbc303b841df024bbd2183101a0132489bc6c86dc9c1902dc1c12c7bf2 tests/test_issue_943_runtime_context_helpers.py
```
PR #944 remains open at `f49e781102b9f363834c28c055f69639d16290c9`; review `622` is undismissed. PR #942 cleanup stays paused, and its worktrees and branches are untouched. Issues #931 and #941 received nothing. The stable control checkout remains clean on `master` at `aab54d48`. No stash was created; no MCP server was restarted or reconnected.
## Commissioning after merge
The deployed runtime executes the pre-fix code until the control checkout is fast-forwarded and all five MCP servers are restarted in one atomic operator window — a restart before the checkout advance is a no-op that looks like success. Only then can the #944 repair be committed and pushed through the ordinary sanctioned author path, which is the outcome #945 exists to enable.
## Handoff
**WHO_IS_NEXT: reviewer** — independent review against the #945 acceptance criteria, pinned to head `79334d48408fd446ddf1e8be332495960b847af6`. Do not self-review and do not self-merge. Preserve the uncommitted #943 repair and keep PR #942 cleanup paused.
An exact-owner lease renewal (#760) is granted the owning-PR duplicate-work
waiver inside gitea_lock_issue, but every later enforcement path rebuilt that
proof from the durable lock through
issue_lock_recovery.recovered_owning_pr_from_lock, which reads only the
dead_session_recovery block. A plain renewal persists its proof under
lease_renewal instead, so the waiver expired with the lock call and the next
author mutation was refused duplicate_commit_prevented with
owning_pr_recovery_exempted: false on the very PR the renewal had just proved.
Add issue_lock_renewal.owning_pr_renewal_from_lock as the renewal mirror of the
recovery rebuild, and resolve both halves through one helper,
_owning_pr_continuation_from_lock, in the same precedence gitea_lock_issue
applies. The three enforcement paths that previously saw only recovery evidence
now share that resolver: the commit/create-PR duplicate recheck, the read-only
duplicate assessor, and the push-ownership prover.
The rebuild re-applies the equality the renewal assessor required (PR head ==
local head == remote head) and additionally binds the record to the claimant the
lock names, so a renewal block cannot be reused under another identity, profile,
or workflow session. Validation of the resulting token against live PR state is
unchanged and still owned solely by
issue_work_duplicate_gate._assess_owning_pr_exemption, so repository, issue, PR,
branch and head binding continue to be enforced in exactly one place.
No public signature, MCP tool schema, refusal shape, or reason code changes.
Dead-session recovery behaviour is untouched.
Tests: tests/test_issue_945_owning_pr_renewal_continuation.py - 49 tests,
8 subtests, all in-memory (no durable branch, worktree, lock, lease or PR).
Against unmodified aab54d48 the suite is 47 failed / 2 passed; the 2 that pass
are the defect-pinning reproductions. Full suite 28F/5574P/6S/1002 subtests at
head vs 28F/5525P/6S/994 at base, with identical failing test id sets.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
REQUEST_CHANGES — PR #946 at head 79334d48408fd446ddf1e8be332495960b847af6
Reviewed independently at base aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218 (live master, unmoved — the PR head's parent equals live master exactly, so no base drift affects this review). Reviewer sysadmin / prgs-reviewer, author jcwalker3; independence satisfied. Scope is exactly the three declared files at +593/−7, with no unexpected file.
The diagnosis is correct and sharper than the issue's own. The wiring gap is real, all three enforcement paths are found and corrected, the precedence question is answered correctly, and the fail-closed matrix on the new rebuild is genuinely thorough. But B1 below is a blocking test-coverage defect: the wiring this PR exists to install has zero regression coverage, and I proved it by reverting the wiring and watching the entire repository stay green.
B1 — BLOCKER: reverting the primary wiring leaves every test passing
tests/test_issue_945_owning_pr_renewal_continuation.py (whole file); wiring at gitea_mcp_server.py:2894, :5179, :19464.
The suite exercises issue_lock_renewal.owning_pr_renewal_from_lock, gitea_mcp_server._owning_pr_continuation_from_lock, and issue_work_duplicate_gate.assess_work_issue_duplicate_gate directly. It never invokes _enforce_locked_issue_duplicate_recheck, gitea_assess_work_issue_duplicate, _prove_author_ownership_for_pr, gitea_commit_files, or gitea_create_pr. Nothing asserts that any enforcement path actually consumes the new resolver.
Proven, not inferred. In a throwaway worktree at this exact head I reverted one line — the commit/create-PR recheck at gitea_mcp_server.py:2894 — back to the pre-#945 recovery-only rebuild:
That reintroduces exactly the defect #945 exists to fix. Results:
new #945 suite, wiring reverted : 49 passed, 8 subtests passed
targeted sweep (945/755/760/dup gates) : 154 passed, 8 subtests passed
FULL suite, wiring reverted : 28 failed, 5574 passed, 6 skipped, 1002 subtests (152.51s)
FULL suite, PR as submitted : 28 failed, 5574 passed, 6 skipped, 1002 subtests (149.90s)
failing test id sets : IDENTICAL
Not one test in the repository detects it. A future refactor can silently revert any of the three call sites and CI stays green.
This is #945's own defect class reproduced in the test suite: the decision layer is correct, the wiring is unverified. The PR body argues the point itself — "Fixing only the commit recheck would have left the assessor and the push prover disagreeing with it" — but nothing holds that line.
The 47 failed / 2 passed base result does not cover this. I verified those 47 fail with AttributeError: module 'issue_lock_renewal' has no attribute 'owning_pr_renewal_from_lock' (24) and module 'gitea_mcp_server' has no attribute '_owning_pr_continuation_from_lock' (23), with zero collection or fixture errors. That proves the two functions are new. It says nothing about who calls them.
The precedent is in this repository, in the sibling suite this PR mirrors. tests/test_issue_755_owning_pr_recovery.py has 12 mcp_server. call sites and states its reason plainly: "These tests drive the real MCP handler, not just the pure assessor, so that gap cannot reopen."#755 met that bar for recovery; #945 should meet it for renewal.
Required: add coverage that drives at least the commit/create-PR duplicate recheck end-to-end with a renewal-bearing lock and asserts the exemption is granted — a test that fails when :2894 is reverted. Ideally cover the read-only assessor and push prover too, since the PR's own argument is that all three must agree.
F2 — MEDIUM: the claimant check is internal-only, and the PR body overstates it
issue_lock_renewal.py (new owning_pr_renewal_from_lock, claimant block).
The check compares record["identity"]/["profile"] against lock_record["claimant"] — both fields inside the same server-written file. It is not bound to the authenticated caller or the owning task session. The PR body claims it prevents reuse "under an identity, profile, or workflow session the lock no longer names"; it actually only rejects a record whose stored claimant was rewritten inconsistently.
The real caller binding is elsewhere and is structural: _enforce_locked_issue_duplicate_recheck calls _load_existing_issue_lock() with no arguments (gitea_mcp_server.py:2872), which resolves issue_lock_store.read_session_issue_lock() → session-{os.getpid()}.json (issue_lock_store.py:85). Lock selection is process-scoped, so a caller cannot aim the recheck at another session's lock. That is what actually prevents cross-session reuse.
Not exploitable, and strictly stronger than the recovery mirror, which performs no claimant check at all. Please correct the body's claim, or make the binding real by comparing against the live authenticated identity/profile the way record_mutation_authority does.
Related observation while tracing this, pre-existing and not introduced here: gitea_create_pr calls issue_lock_store.verify_lock_for_mutation at gitea_mcp_server.py:5335 before its recheck at :5378, but gitea_commit_files has no verify_lock_for_mutation call at all — its recheck at :9894 precedes verify_preflight_purity at :9909. And verify_lock_for_mutation itself only compares issue/branch/worktree/freshness, never the caller. Worth a follow-up issue; out of scope here.
F3 — MINOR: a conflicting recovery/renewal pair falls through rather than failing closed
The third case is the one worth naming: a recovery block naming a different PR exists and fails validation, and the helper still returns renewal evidence rather than refusing.
Not a blocker, because it is unreachable through the sanctioned writer. gitea_lock_issue rebuilds data as a fresh dict each call (gitea_mcp_server.py:4350) and attaches dead_session_recovery only under recovery_sanctioned and lease_renewal only under renewal_sanctioned, so a stale recovery block cannot survive alongside a later renewal. tests/test_issue_760_mcp_renewal_path.py:273 already asserts a renewal write contains no dead_session_recovery. Live-PR validation backstops it regardless. Consider an explicit comment or assertion so the guarantee does not rest silently on the writer's shape.
What is correct — for the record
Precedence matches the lock path exactly.gitea_lock_issue applies recovery if recovery_sanctioned else renewal (:4258-4268); the resolver applies the same order, verified by probe. The stated design goal is met.
All three enforcement paths were found and rewired. Confirmed by grep: the only remaining call to recovered_owning_pr_from_lock in production code is inside the resolver itself (gitea_mcp_server.py:2808). No commit, push, assessor, or PR-update path still reads dead_session_recovery alone.
The authoritative policy is untouched and not duplicated.issue_work_duplicate_gate._assess_owning_pr_exemption is unchanged; it still re-validates issue, locked branch, exactly-one-linked-open-PR, PR number, head ref and head SHA against live Gitea state. The patch changes only which server-written block the token is rebuilt from. An open PR alone still grants nothing.
The rebuild's fail-closed matrix is real, 21 cases with no mocks anywhere in the file — head divergence, missing or malformed heads, absent claimant, identity/profile mismatch, malformed PR number, not-granted and missing renewal blocks all yield None. The equality re-check (pr_head == head_sha == remote_head_sha) genuinely re-derives what the assessor required.
The pre-fix reproduction claim is honest, verified independently: AttributeError at call time, 0 collection/fixture errors, and the 2 base-passing tests are the defect-pinning reproductions that must pass on both sides.
No regression. My own runs, in reviewer worktrees under branches/:
targeted (24 files) @ head 79334d48 : 1 failed, 514 passed, 32 subtests (32.49s)
targeted (24 files) @ base aab54d48 : 1 failed, 465 passed, 24 subtests (32.13s)
full suite @ head : 28 failed, 5574 passed, 6 skipped, 1002 subtests
full suite @ base : 28 failed, 5525 passed, 6 skipped, 994 subtests
failing test id sets : IDENTICAL
The single targeted failure, test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected, reproduces on base in isolation. The +49 passes and +8 subtests are exactly this PR's new tests. The suite creates no durable branch, worktree, lock, lease, comment or PR.
Canonical PR State
STATE: PR #946 is open at head 79334d4840 and has received one formal REQUEST_CHANGES review from sysadmin at that exact head. One blocking finding (B1) plus one medium (F2) and one minor (F3) are open. The branch introduces no test regression against base aab54d4825.
WHO_IS_NEXT: author
NEXT_ACTION: Author jcwalker3 must add regression coverage that drives the real enforcement paths with a renewal-bearing lock so that reverting any of gitea_mcp_server.py:2894, :5179, or :19464 fails a test (B1), correct or strengthen the caller-binding claim in the PR body (F2), optionally make the conflicting-evidence guarantee explicit (F3), push the result, and publish a new head-pinned handoff for a fresh independent review.
NEXT_PROMPT:
Address the REQUEST_CHANGES review on PR #946 (Closes #945) in
Scaled-Tech-Consulting/Gitea-Tools on remote prgs.
Invoke the canonical gitea-workflow skill first. Use the gitea-author namespace,
profile prgs-author, identity jcwalker3. Reviewed head was
79334d48408fd446ddf1e8be332495960b847af6; base aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218.
B1 (blocker): tests/test_issue_945_owning_pr_renewal_continuation.py never drives
an enforcement path, so reverting gitea_mcp_server.py:2894 to
issue_lock_recovery.recovered_owning_pr_from_lock leaves the full suite byte
identical (28F/5574P/6S/1002 subtests, same failing ids). Add coverage that
drives the real commit/create-PR duplicate recheck with a renewal-bearing lock
and asserts the exemption is granted; confirm it fails with :2894 reverted and
passes restored. Follow tests/test_issue_755_owning_pr_recovery.py, which drives
the real MCP handler for the recovery half. Cover the read-only assessor
(:5179) and push prover (:19464) too.
F2 (medium): the claimant check in issue_lock_renewal.owning_pr_renewal_from_lock
compares two fields inside the same lock file and is not bound to the
authenticated caller. Real cross-session binding comes from
read_session_issue_lock() keying on session-{os.getpid()}.json. Either correct
the PR body claim or compare against the live identity/profile the way
record_mutation_authority does.
F3 (minor): _owning_pr_continuation_from_lock falls through to renewal when a
recovery block is present but fails validation, even when the two name different
PRs. Unreachable through gitea_lock_issue because data is rebuilt per call, but
make that guarantee explicit rather than implicit.
Do not merge. Do not review your own work. Preserve the uncommitted #943 repair,
keep PR #942 cleanup paused, and leave issues #931 and #941 untouched.
WHAT_HAPPENED: An independent review at the exact head read all three changed files, traced the evidence flow from owning_pr_renewal_evidence through the new rebuild, the shared resolver, and all three enforcement paths into issue_work_duplicate_gate._assess_owning_pr_exemption, and probed precedence and fall-through behaviour directly against the patched modules. The wiring, precedence and fail-closed matrix are correct. Reverting the primary wiring in a throwaway worktree at the same head left the new suite at 49 passed and the full suite byte identical to the PR's own result, proving no test protects the fix. Targeted and full suites were run at head and at a clean base worktree; failing test id sets are identical.
WHY: #945 exists because a correct decision layer was never wired into the paths that enforce it. This PR wires it correctly but ships no test that fails if the wiring is removed, so the same class of defect can silently return. The sibling recovery suite for #755 already drives the real MCP handler for exactly this reason.
REVIEW_STATUS: REQUEST_CHANGES posted at 79334d4840 by sysadmin
MERGE_READY: no
BLOCKERS: test coverage blocker
VALIDATION: New #945 suite at head: 49 passed, 8 subtests. Targeted 24-file sweep at head: 1 failed, 514 passed, 32 subtests in 32.49s; at clean base worktree aab54d48: 1 failed, 465 passed, 24 subtests in 32.13s; the single failure test_pidless_durable_lock_rejected reproduces on base in isolation. Full suite at head: 28 failed, 5574 passed, 6 skipped, 1002 subtests in 149.90s. Full suite at base: 28 failed, 5525 passed, 6 skipped, 994 subtests in 148.91s. Failing test id sets identical, so no regression originates from this branch. Wiring-revert probe at the same head: new suite 49 passed, full suite 28 failed/5574 passed with an identical failing id set, demonstrating the absent coverage. Base reproduction of the new suite fails with AttributeError at call time on both new symbols, 0 collection or fixture errors.
## REQUEST_CHANGES — PR #946 at head `79334d48408fd446ddf1e8be332495960b847af6`
Reviewed independently at base `aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218` (live `master`, unmoved — the PR head's parent equals live master exactly, so no base drift affects this review). Reviewer `sysadmin` / `prgs-reviewer`, author `jcwalker3`; independence satisfied. Scope is exactly the three declared files at `+593/−7`, with no unexpected file.
The diagnosis is correct and sharper than the issue's own. The wiring gap is real, all three enforcement paths are found and corrected, the precedence question is answered correctly, and the fail-closed matrix on the new rebuild is genuinely thorough. **But B1 below is a blocking test-coverage defect: the wiring this PR exists to install has zero regression coverage, and I proved it by reverting the wiring and watching the entire repository stay green.**
### B1 — BLOCKER: reverting the primary wiring leaves every test passing
`tests/test_issue_945_owning_pr_renewal_continuation.py` (whole file); wiring at `gitea_mcp_server.py:2894`, `:5179`, `:19464`.
The suite exercises `issue_lock_renewal.owning_pr_renewal_from_lock`, `gitea_mcp_server._owning_pr_continuation_from_lock`, and `issue_work_duplicate_gate.assess_work_issue_duplicate_gate` directly. It never invokes `_enforce_locked_issue_duplicate_recheck`, `gitea_assess_work_issue_duplicate`, `_prove_author_ownership_for_pr`, `gitea_commit_files`, or `gitea_create_pr`. Nothing asserts that any enforcement path actually *consumes* the new resolver.
Proven, not inferred. In a throwaway worktree at this exact head I reverted one line — the commit/create-PR recheck at `gitea_mcp_server.py:2894` — back to the pre-#945 recovery-only rebuild:
```python
- recovered_owning_pr=_owning_pr_continuation_from_lock(lock_data),
+ recovered_owning_pr=issue_lock_recovery.recovered_owning_pr_from_lock(lock_data),
```
That reintroduces exactly the defect #945 exists to fix. Results:
```text
new #945 suite, wiring reverted : 49 passed, 8 subtests passed
targeted sweep (945/755/760/dup gates) : 154 passed, 8 subtests passed
FULL suite, wiring reverted : 28 failed, 5574 passed, 6 skipped, 1002 subtests (152.51s)
FULL suite, PR as submitted : 28 failed, 5574 passed, 6 skipped, 1002 subtests (149.90s)
failing test id sets : IDENTICAL
```
Not one test in the repository detects it. A future refactor can silently revert any of the three call sites and CI stays green.
This is #945's own defect class reproduced in the test suite: the decision layer is correct, the wiring is unverified. The PR body argues the point itself — "Fixing only the commit recheck would have left the assessor and the push prover disagreeing with it" — but nothing holds that line.
The `47 failed / 2 passed` base result does **not** cover this. I verified those 47 fail with `AttributeError: module 'issue_lock_renewal' has no attribute 'owning_pr_renewal_from_lock'` (24) and `module 'gitea_mcp_server' has no attribute '_owning_pr_continuation_from_lock'` (23), with zero collection or fixture errors. That proves the two functions are new. It says nothing about who calls them.
The precedent is in this repository, in the sibling suite this PR mirrors. `tests/test_issue_755_owning_pr_recovery.py` has 12 `mcp_server.` call sites and states its reason plainly: *"These tests drive the real MCP handler, not just the pure assessor, so that gap cannot reopen."* #755 met that bar for recovery; #945 should meet it for renewal.
**Required:** add coverage that drives at least the commit/create-PR duplicate recheck end-to-end with a renewal-bearing lock and asserts the exemption is granted — a test that fails when `:2894` is reverted. Ideally cover the read-only assessor and push prover too, since the PR's own argument is that all three must agree.
### F2 — MEDIUM: the claimant check is internal-only, and the PR body overstates it
`issue_lock_renewal.py` (new `owning_pr_renewal_from_lock`, claimant block).
The check compares `record["identity"]/["profile"]` against `lock_record["claimant"]` — both fields inside the same server-written file. It is not bound to the authenticated caller or the owning task session. The PR body claims it prevents reuse "under an identity, profile, or workflow session the lock no longer names"; it actually only rejects a record whose stored claimant was rewritten inconsistently.
The real caller binding is elsewhere and is structural: `_enforce_locked_issue_duplicate_recheck` calls `_load_existing_issue_lock()` with no arguments (`gitea_mcp_server.py:2872`), which resolves `issue_lock_store.read_session_issue_lock()` → `session-{os.getpid()}.json` (`issue_lock_store.py:85`). Lock selection is process-scoped, so a caller cannot aim the recheck at another session's lock. That is what actually prevents cross-session reuse.
Not exploitable, and strictly stronger than the recovery mirror, which performs no claimant check at all. Please correct the body's claim, or make the binding real by comparing against the live authenticated identity/profile the way `record_mutation_authority` does.
Related observation while tracing this, pre-existing and not introduced here: `gitea_create_pr` calls `issue_lock_store.verify_lock_for_mutation` at `gitea_mcp_server.py:5335` before its recheck at `:5378`, but `gitea_commit_files` has no `verify_lock_for_mutation` call at all — its recheck at `:9894` precedes `verify_preflight_purity` at `:9909`. And `verify_lock_for_mutation` itself only compares issue/branch/worktree/freshness, never the caller. Worth a follow-up issue; out of scope here.
### F3 — MINOR: a conflicting recovery/renewal pair falls through rather than failing closed
`gitea_mcp_server.py` (`_owning_pr_continuation_from_lock`).
Probed directly at this head:
```text
recovery(valid, PR 100) + renewal(valid, PR 999) -> PR 100 (recovery wins, correct)
recovery(recovered=False) + renewal(valid) -> renewal (falls through)
recovery(present, head-invalid, PR 100)
+ renewal(valid, PR 999) -> PR 999 (falls through past a conflicting record)
```
The third case is the one worth naming: a recovery block naming a different PR exists and fails validation, and the helper still returns renewal evidence rather than refusing.
Not a blocker, because it is unreachable through the sanctioned writer. `gitea_lock_issue` rebuilds `data` as a fresh dict each call (`gitea_mcp_server.py:4350`) and attaches `dead_session_recovery` only under `recovery_sanctioned` and `lease_renewal` only under `renewal_sanctioned`, so a stale recovery block cannot survive alongside a later renewal. `tests/test_issue_760_mcp_renewal_path.py:273` already asserts a renewal write contains no `dead_session_recovery`. Live-PR validation backstops it regardless. Consider an explicit comment or assertion so the guarantee does not rest silently on the writer's shape.
### What is correct — for the record
* **Precedence matches the lock path exactly.** `gitea_lock_issue` applies `recovery if recovery_sanctioned else renewal` (`:4258-4268`); the resolver applies the same order, verified by probe. The stated design goal is met.
* **All three enforcement paths were found and rewired.** Confirmed by grep: the only remaining call to `recovered_owning_pr_from_lock` in production code is inside the resolver itself (`gitea_mcp_server.py:2808`). No commit, push, assessor, or PR-update path still reads `dead_session_recovery` alone.
* **The authoritative policy is untouched and not duplicated.** `issue_work_duplicate_gate._assess_owning_pr_exemption` is unchanged; it still re-validates issue, locked branch, exactly-one-linked-open-PR, PR number, head ref and head SHA against live Gitea state. The patch changes only which server-written block the token is rebuilt from. An open PR alone still grants nothing.
* **The rebuild's fail-closed matrix is real**, 21 cases with no mocks anywhere in the file — head divergence, missing or malformed heads, absent claimant, identity/profile mismatch, malformed PR number, not-granted and missing renewal blocks all yield `None`. The equality re-check (`pr_head == head_sha == remote_head_sha`) genuinely re-derives what the assessor required.
* **The pre-fix reproduction claim is honest**, verified independently: `AttributeError` at call time, 0 collection/fixture errors, and the 2 base-passing tests are the defect-pinning reproductions that must pass on both sides.
* **No regression.** My own runs, in reviewer worktrees under `branches/`:
```text
targeted (24 files) @ head 79334d48 : 1 failed, 514 passed, 32 subtests (32.49s)
targeted (24 files) @ base aab54d48 : 1 failed, 465 passed, 24 subtests (32.13s)
full suite @ head : 28 failed, 5574 passed, 6 skipped, 1002 subtests
full suite @ base : 28 failed, 5525 passed, 6 skipped, 994 subtests
failing test id sets : IDENTICAL
```
The single targeted failure, `test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected`, reproduces on base in isolation. The `+49` passes and `+8` subtests are exactly this PR's new tests. The suite creates no durable branch, worktree, lock, lease, comment or PR.
## Canonical PR State
STATE: PR #946 is open at head 79334d48408fd446ddf1e8be332495960b847af6 and has received one formal REQUEST_CHANGES review from sysadmin at that exact head. One blocking finding (B1) plus one medium (F2) and one minor (F3) are open. The branch introduces no test regression against base aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218.
WHO_IS_NEXT: author
NEXT_ACTION: Author jcwalker3 must add regression coverage that drives the real enforcement paths with a renewal-bearing lock so that reverting any of gitea_mcp_server.py:2894, :5179, or :19464 fails a test (B1), correct or strengthen the caller-binding claim in the PR body (F2), optionally make the conflicting-evidence guarantee explicit (F3), push the result, and publish a new head-pinned handoff for a fresh independent review.
NEXT_PROMPT:
```text
Address the REQUEST_CHANGES review on PR #946 (Closes #945) in
Scaled-Tech-Consulting/Gitea-Tools on remote prgs.
Invoke the canonical gitea-workflow skill first. Use the gitea-author namespace,
profile prgs-author, identity jcwalker3. Reviewed head was
79334d48408fd446ddf1e8be332495960b847af6; base aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218.
B1 (blocker): tests/test_issue_945_owning_pr_renewal_continuation.py never drives
an enforcement path, so reverting gitea_mcp_server.py:2894 to
issue_lock_recovery.recovered_owning_pr_from_lock leaves the full suite byte
identical (28F/5574P/6S/1002 subtests, same failing ids). Add coverage that
drives the real commit/create-PR duplicate recheck with a renewal-bearing lock
and asserts the exemption is granted; confirm it fails with :2894 reverted and
passes restored. Follow tests/test_issue_755_owning_pr_recovery.py, which drives
the real MCP handler for the recovery half. Cover the read-only assessor
(:5179) and push prover (:19464) too.
F2 (medium): the claimant check in issue_lock_renewal.owning_pr_renewal_from_lock
compares two fields inside the same lock file and is not bound to the
authenticated caller. Real cross-session binding comes from
read_session_issue_lock() keying on session-{os.getpid()}.json. Either correct
the PR body claim or compare against the live identity/profile the way
record_mutation_authority does.
F3 (minor): _owning_pr_continuation_from_lock falls through to renewal when a
recovery block is present but fails validation, even when the two name different
PRs. Unreachable through gitea_lock_issue because data is rebuilt per call, but
make that guarantee explicit rather than implicit.
Do not merge. Do not review your own work. Preserve the uncommitted #943 repair,
keep PR #942 cleanup paused, and leave issues #931 and #941 untouched.
```
WHAT_HAPPENED: An independent review at the exact head read all three changed files, traced the evidence flow from owning_pr_renewal_evidence through the new rebuild, the shared resolver, and all three enforcement paths into issue_work_duplicate_gate._assess_owning_pr_exemption, and probed precedence and fall-through behaviour directly against the patched modules. The wiring, precedence and fail-closed matrix are correct. Reverting the primary wiring in a throwaway worktree at the same head left the new suite at 49 passed and the full suite byte identical to the PR's own result, proving no test protects the fix. Targeted and full suites were run at head and at a clean base worktree; failing test id sets are identical.
WHY: #945 exists because a correct decision layer was never wired into the paths that enforce it. This PR wires it correctly but ships no test that fails if the wiring is removed, so the same class of defect can silently return. The sibling recovery suite for #755 already drives the real MCP handler for exactly this reason.
ISSUE: #945
HEAD_SHA: 79334d48408fd446ddf1e8be332495960b847af6
REVIEW_STATUS: REQUEST_CHANGES posted at 79334d48408fd446ddf1e8be332495960b847af6 by sysadmin
MERGE_READY: no
BLOCKERS: test coverage blocker
VALIDATION: New #945 suite at head: 49 passed, 8 subtests. Targeted 24-file sweep at head: 1 failed, 514 passed, 32 subtests in 32.49s; at clean base worktree aab54d48: 1 failed, 465 passed, 24 subtests in 32.13s; the single failure test_pidless_durable_lock_rejected reproduces on base in isolation. Full suite at head: 28 failed, 5574 passed, 6 skipped, 1002 subtests in 149.90s. Full suite at base: 28 failed, 5525 passed, 6 skipped, 994 subtests in 148.91s. Failing test id sets identical, so no regression originates from this branch. Wiring-revert probe at the same head: new suite 49 passed, full suite 28 failed/5574 passed with an identical failing id set, demonstrating the absent coverage. Base reproduction of the new suite fails with AttributeError at call time on both new symbols, 0 collection or fixture errors.
LAST_UPDATED_BY: sysadmin / prgs-reviewer / gitea-reviewer namespace, reviewer lease session 93257-9ba6b15dd243
Addresses review 623 on PR #946 (B1 blocker, F2 medium, F3 minor).
B1 - the wiring this branch exists to install had no regression coverage.
The existing suite exercised owning_pr_renewal_from_lock,
_owning_pr_continuation_from_lock and the duplicate gate directly, but never
drove an enforcement path, so reverting any of the three call sites left the
whole repository green. Add tests/test_issue_945_enforcement_path_wiring.py,
which drives the real mcp_server._enforce_locked_issue_duplicate_recheck (the
shared recheck behind gitea_commit_files and gitea_create_pr),
mcp_server.gitea_assess_work_issue_duplicate and
mcp_server._prove_author_ownership_for_pr against a renewal-bearing lock, and
asserts each grants the exemption. Reverting the commit/create-PR recheck to
the recovery-only rebuild now fails 8 tests and 4 subtests; reverting the
assessor or the push prover fails 2 each. The suite also keeps the fail-closed
matrix on the real paths: an open PR alone, a second PR, a different PR,
branch, issue or head, identity and profile mismatch, ungranted and malformed
renewal blocks, and sequential-task non-inheritance are all still refused.
F2 - the claimant check compares lease_renewal.identity/profile against the
claimant recorded on the same lock file. Both sides are server-written fields
of one document, so it is an internal-consistency check, not verification of
the live authenticated caller. Correct the docstring and the inline comment to
say so, and document the binding that actually prevents cross-session reuse:
the enforcement paths load the lock through _load_existing_issue_lock() with no
issue coordinates, which resolves issue_lock_store.read_session_issue_lock() to
the session pointer at session-{os.getpid()}.json, so lock selection is scoped
to the operating-system process. Its limits are stated too - per-process rather
than per-authenticated-user, silent on locks reached by explicit coordinates,
and silent on two roles sharing one process. Live identity and profile stay
enforced by the mutation-authority and profile gates, not by this rebuild.
F3 - _owning_pr_continuation_from_lock previously fell through to renewal when
a dead_session_recovery block was present but failed to rebuild, so a recovery
record naming one PR could be bypassed by renewal evidence naming another.
Present-but-unusable recovery evidence is now ambiguous rather than absent and
fails closed. Because an expired lease whose recorded owner has also died
satisfies both dispositions in one gitea_lock_issue call, a sanctioned pair is
a reachable state; when both rebuild, they must agree on issue, PR, branch and
every head, or no continuation authority is returned. Recovery-only and
renewal-only locks keep their existing behaviour exactly.
Validation of the resulting token against live PR state remains untouched and
solely owned by issue_work_duplicate_gate._assess_owning_pr_exemption. No
public signature, MCP tool schema, refusal shape or reason code changes.
Tests: focused #945/#755/#760 suites 137 passed, 19 subtests. Full suite in a
branches/ worktree: 30F/5619P/6S/1013 subtests at this head vs 30F/5572P/6S/1002
at a clean checkout of 79334d48, with byte-identical failing test id sets - the
+47 passes and +11 subtests are exactly the new coverage.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q8RUznLXEA4JoK48sTZiSK
[THREAD STATE LEDGER] PR #946 — review 623 remediation pushed; head advanced to b1fcf159
Server-side mutation ledger:
git push (fast-forward, no force) → branch fix/issue-945-owning-pr-renewal-evidence advanced 79334d4..b1fcf15; PR #946 head is now b1fcf159378e7c72b67cc5ac507920d8390ef107
this issue comment
Blockers:
permission/capability blocker: gitea_edit_pr exposes no worktree_path parameter and resolves its workspace from the MCP process root, so the PR description update fails closed on the #618 control-checkout wall. The correction it was to carry is reproduced in full below.
Server-side decision state: review 623 REQUEST_CHANGES from sysadmin remains undismissed and still gates merge; it is now marked stale because the head advanced past the reviewed SHA
Local verdict/state: no verdict prepared locally — the author does not review this PR
Latest known validation: focused #945/#755/#760 suites 137 passed / 19 subtests; full suite 30F/5619P/6S/1013 subtests at this head against 30F/5572P/6S/1002 at a clean checkout of 79334d48, failing test id sets byte-identical
approval_at_current_head: false
What changed:
Commit b1fcf159 published, addressing B1, F2 and F3 from review 623
Required action: fresh independent review of PR #946 pinned to head b1fcf159378e7c72b67cc5ac507920d8390ef107
Do not do: self-review, self-merge, or treat review 623 as satisfied without re-verifying at the new head
Resume from: PR #946 review feedback at head b1fcf159
Review 623 findings → fixes and evidence
B1 (BLOCKER) — reverting the wiring left every test passing
Fix: new tests/test_issue_945_enforcement_path_wiring.py (685 lines). It drives the real handlers rather than the pure assessor — mcp_server._enforce_locked_issue_duplicate_recheck (the shared recheck behind gitea_commit_files and gitea_create_pr), mcp_server.gitea_assess_work_issue_duplicate, and mcp_server._prove_author_ownership_for_pr — against a renewal-bearing lock, and asserts each grants the exemption. This follows the precedent review 623 cited, tests/test_issue_755_owning_pr_recovery.py.
Evidence — revert probes at head b1fcf159, each in a throwaway worktree removed afterwards:
All three call sites are now individually protected. The exact revert review 623 performed — the commit/create-PR recheck — now fails 8 tests and 4 subtests where previously the whole repository stayed green.
The suite also keeps the fail-closed matrix on the real paths: an open PR alone, a second PR, a different PR, branch, issue or head, identity and profile mismatch, ungranted and malformed renewal blocks, stale recorded heads, local/remote head divergence, and sequential-task non-inheritance are all still refused. One test asserts the runs leave no durable artifact outside a temporary lock directory.
F2 (MEDIUM) — the claimant check was overstated
Accepted; the claim was wrong and is corrected.
The check compares lease_renewal.identity / profile against the claimant recorded on the same lock file. Both sides are server-written fields of one document, so it is an internal-consistency check within a server-written lock record. It rejects a lock whose renewal block and claimant disagree.
It is not direct authenticated-caller verification. It does not consult the live authenticated caller and does not prove the session invoking a later gate is the session the renewal was granted to. The prior claim that it prevents reuse "under an identity, profile, or workflow session the lock no longer names" has been removed.
The binding that actually applies is structural and lives in the caller, exactly as review 623 identified: the enforcement paths load the lock through _load_existing_issue_lock() with no issue coordinates, which resolves issue_lock_store.read_session_issue_lock() → the session pointer at session-{os.getpid()}.json. Lock selection is scoped to the operating-system process, so a caller cannot aim the recheck at a lock some other process bound.
Its limits, stated plainly: it is per-process, not per-authenticated-user; it says nothing about a lock reached by explicit issue coordinates rather than the session pointer; and it says nothing about two roles sharing one process. Live identity and profile stay enforced by the mutation-authority and profile gates each mutating path already runs — not by this rebuild.
Evidence: the corrected wording is in the code, not only in prose — issue_lock_renewal.owning_pr_renewal_from_lock's docstring and the inline comment above the claimant block both now say this. TestCallerBindingIsStructuralNotFieldComparison in the new suite pins the behaviour: test_lock_selection_is_keyed_to_the_operating_system_process, test_a_lock_bound_by_another_process_is_not_reachable, test_claimant_check_does_not_consult_the_live_authenticated_caller, and test_internal_disagreement_is_what_the_check_actually_rejects.
The pre-existing observation in review 623 — that gitea_commit_files has no verify_lock_for_mutation call while gitea_create_pr does, and that verify_lock_for_mutation never compares the caller — is untouched here and merits its own issue.
F3 (MINOR) — conflicting recovery/renewal pair fell through
Fix:_owning_pr_continuation_from_lock now treats present-but-unusable recovery evidence as ambiguous rather than absent and fails closed, so a recovery record naming one PR can no longer be bypassed by renewal evidence naming another.
One correction to the finding's premise. Review 623 judged a sanctioned pair unreachable because gitea_lock_issue rebuilds data per call. Tracing it further: recovery is assessed whenever the lease is not live and requires the recorded PID to be dead, while renewal is assessed whenever the lease has expired — itself one way to be non-live — and deliberately does not branch on PID liveness (#760 AC16). An expired lease whose recorded owner has also died satisfies both, and both blocks are written into that same freshly built dict. A sanctioned pair is therefore a reachable, legitimate state. Because it derives from one live observation in one call it always describes the same issue, PR, branch and heads, so when both blocks rebuild they must now agree on every one of those bindings or no continuation authority is returned. Recovery-only and renewal-only locks keep their existing behaviour exactly.
Evidence:_CONTINUATION_EVIDENCE_BINDINGS and _continuation_evidence_agrees in gitea_mcp_server.py, with the reasoning recorded in the resolver docstring rather than left implicit in the writer's shape.
PR description correction (published here because the description edit fails closed)
The F2 text above is what was to replace the "Deliberate hardening beyond a pure mirror" paragraph in the PR description. Because gitea_edit_pr is unavailable to this workspace binding, it is published here instead and this comment is the authoritative record. The PR description's claim that the claimant check prevents reuse "under an identity, profile, or workflow session the lock no longer names" is superseded by the F2 section of this comment.
Canonical Issue State
STATE: Issue #945 is open with status:pr-open. Its owning PR #946 is open at head b1fcf159378e7c72b67cc5ac507920d8390ef107, carrying two commits: 79334d48 (original fix) and b1fcf159 (review 623 remediation). Review 623 REQUEST_CHANGES from sysadmin is undismissed and still gates merge, and is now stale because the head advanced past the reviewed SHA 79334d48. B1, F2 and F3 have been addressed and verified locally, but no independent review has yet evaluated the new head.
WHO_IS_NEXT: reviewer
NEXT_ACTION: An independent reviewer — not jcwalker3 — must review PR #946 pinned to head b1fcf159378e7c72b67cc5ac507920d8390ef107 against the #945 acceptance criteria, confirming that the B1 regression coverage genuinely fails when any of the three enforcement call sites is reverted, that the F2 correction accurately describes the claimant check as internal consistency rather than authenticated-caller verification, and that F3 fails closed on conflicting evidence.
NEXT_PROMPT:
Perform an independent review of Gitea-Tools PR #946 (Closes #945) in
Scaled-Tech-Consulting/Gitea-Tools on remote prgs.
Invoke the canonical gitea-workflow skill first. Use the gitea-reviewer
namespace, profile prgs-reviewer. The author is jcwalker3; do not review as
the author. Pin the review to head b1fcf159378e7c72b67cc5ac507920d8390ef107.
Merge base is master at aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218; live master
is ed9414ebda9034ca87b36a9fce1c1ff7f98090f6 and Gitea reports mergeable true.
Prior review 623 (REQUEST_CHANGES, sysadmin) was posted at the earlier head
79334d48408fd446ddf1e8be332495960b847af6 and raised B1 blocker, F2 medium and
F3 minor. Commit b1fcf159 claims to address all three. Verify each rather than
accepting the author's account:
B1 - tests/test_issue_945_enforcement_path_wiring.py is new and claims to drive
the real enforcement handlers. Re-run the revert probe yourself in a throwaway
worktree: revert gitea_mcp_server.py:2945 to
issue_lock_recovery.recovered_owning_pr_from_lock and confirm tests now fail
(author measured 8 failed, 92 passed, 4 subtests failed). Repeat for the
assessor at :5230 and the push prover at :19515 (author measured 2 failed each).
Confirm the tests fail for the right reason and are not tautological.
F2 - confirm issue_lock_renewal.owning_pr_renewal_from_lock's docstring and
inline comment now describe the claimant check as internal consistency within a
server-written lock record and explicitly disclaim direct authenticated-caller
verification, and that the process/session-file binding via
read_session_issue_lock() -> session-{os.getpid()}.json is described with its
limits. Note the PR description itself could not be edited; the correction is in
the author handoff comment on PR #946.
F3 - confirm _owning_pr_continuation_from_lock fails closed when a
dead_session_recovery block is present but does not rebuild, and when a
rebuilt recovery and renewal pair disagree on issue, PR, branch or any head.
Also verify no regression: run the full suite from a branches/ worktree at this
head and at a clean checkout, and compare failing test id sets rather than
counts. The author measured 30F/5619P/6S/1013 subtests at head against
30F/5572P/6S/1002 at a clean checkout of 79334d48, sets byte-identical.
Note for the audit trail: commit b1fcf159 was published under a one-time
operator break-glass authorization because the deployed #945 defect blocks the
sanctioned commit path for its own fix. Confirm the branch contains exactly the
four intended files and no unrelated, credential, configuration or scratch file.
Do not merge. Do not review your own work. Preserve the uncommitted #943 repair,
keep PR #942 cleanup paused, and leave issues #931 and #941 untouched.
WHAT_HAPPENED: The B1, F2 and F3 remediation for review 623 existed only as uncommitted changes in the issue-945-owning-pr-renewal-evidence worktree, because the deployed #945 defect refuses the sanctioned author commit path for this very issue. Under a one-time operator break-glass authorization, every pin was verified read-only first — local HEAD, remote branch head and live PR head all equal to 79334d48; review 623 confirmed the current undismissed REQUEST_CHANGES; zero active control-plane leases; the sole issue lock owned by jcwalker3 / prgs-author with a dead PID 23400; and the four file hashes recomputed and matched against the fingerprints the lock itself recorded. The four files were staged explicitly, committed as b1fcf159, and pushed as a fast-forward to the existing branch. PR #946 was then natively confirmed to sit at exactly that commit. The PR description edit failed closed on a workspace-binding wall, so the F2 correction is published in this comment instead.
WHY: #945 exists because a correct decision layer was never wired into the paths that enforce it, and review 623 showed this branch had reproduced that same defect class in its own test suite — the wiring was correct but nothing failed when it was removed. The new suite drives the real enforcement handlers so a silent revert of any of the three call sites now fails a test. F2 mattered because a security claim in the PR description overstated what the claimant comparison proves; an inaccurate security claim is worse than a modest one, so it is corrected and the real structural binding is described with its limits. F3 closed a fall-through where ambiguous recovery evidence could be bypassed rather than refused.
RELATED_PRS: #946 (owning PR, open, head b1fcf15937); #944 (open at f49e781102, review 622 undismissed, waiting on this issue); #942 (cleanup paused)
BLOCKERS: awaiting fresh independent review at the new head; review 623 undismissed. MERGE_READY: no.
VALIDATION: Focused #945/#755/#760 suites at b1fcf159: 137 passed, 19 subtests passed. Full suite from a branches/ worktree at b1fcf159: 30 failed, 5619 passed, 6 skipped, 1013 subtests in 175.50s. Full suite at a clean checkout of 79334d48: 30 failed, 5572 passed, 6 skipped, 1002 subtests in 177.76s. Failing test id sets byte-identical in both directions, so no failure originates from this commit; the +47 passes and +11 subtests are exactly the new coverage. Earlier runs on this PR reported 28 failures rather than 30 — the two extra are environmental drift present identically on both sides, which is why the comparison is made on failing id sets rather than counts. Break-glass necessity was verified read-only: gitea_assess_work_issue_duplicate(issue 945, phase=commit) returned outcome: duplicate_commit_prevented, owning_pr_recovery_exempted: false, owning_pr_recovery_notes: [], reasons: ["open PR #946 already covers issue #945 (fail closed)"], and the durable lock carries a lease_renewal block with pr_number: null and pr_head_sha: null and no dead_session_recovery block, so no owning-PR waiver is rebuildable from it even by the corrected code. Exactly one commit was created and pushed as a fast-forward: no force-push, no new branch, no new issue, no new PR, no review, no approval, no merge, and no change to master. Protected state is intact — the issue-943-runtime-context-helpers worktree was never entered for writing, PR #944 remains open at f49e781 with review 622 undismissed, PR #942 cleanup stays paused, issues #931 and #941 received nothing, and the control checkout remains clean on master at ed9414eb.
[THREAD STATE LEDGER] PR #946 — review 623 remediation pushed; head advanced to b1fcf159
Server-side mutation ledger:
- git push (fast-forward, no force) → branch `fix/issue-945-owning-pr-renewal-evidence` advanced `79334d4..b1fcf15`; PR #946 head is now `b1fcf159378e7c72b67cc5ac507920d8390ef107`
- this issue comment
Blockers:
- permission/capability blocker: `gitea_edit_pr` exposes no `worktree_path` parameter and resolves its workspace from the MCP process root, so the PR **description** update fails closed on the #618 control-checkout wall. The correction it was to carry is reproduced in full below.
What is true now:
- PR state: open
- Current head SHA: b1fcf159378e7c72b67cc5ac507920d8390ef107
- Server-side decision state: review 623 REQUEST_CHANGES from sysadmin remains undismissed and still gates merge; it is now marked stale because the head advanced past the reviewed SHA
- Local verdict/state: no verdict prepared locally — the author does not review this PR
- Latest known validation: focused #945/#755/#760 suites 137 passed / 19 subtests; full suite 30F/5619P/6S/1013 subtests at this head against 30F/5572P/6S/1002 at a clean checkout of 79334d48, failing test id sets byte-identical
- approval_at_current_head: false
What changed:
- Commit `b1fcf159` published, addressing B1, F2 and F3 from review 623
What is blocked:
- Blocker classification: permission/capability blocker
Who/what acts next:
- Next actor: reviewer
- Required action: fresh independent review of PR #946 pinned to head `b1fcf159378e7c72b67cc5ac507920d8390ef107`
- Do not do: self-review, self-merge, or treat review 623 as satisfied without re-verifying at the new head
- Resume from: PR #946 review feedback at head b1fcf159
---
## Review 623 findings → fixes and evidence
### B1 (BLOCKER) — reverting the wiring left every test passing
**Fix:** new `tests/test_issue_945_enforcement_path_wiring.py` (685 lines). It drives the **real** handlers rather than the pure assessor — `mcp_server._enforce_locked_issue_duplicate_recheck` (the shared recheck behind `gitea_commit_files` and `gitea_create_pr`), `mcp_server.gitea_assess_work_issue_duplicate`, and `mcp_server._prove_author_ownership_for_pr` — against a renewal-bearing lock, and asserts each grants the exemption. This follows the precedent review 623 cited, `tests/test_issue_755_owning_pr_recovery.py`.
**Evidence** — revert probes at head `b1fcf159`, each in a throwaway worktree removed afterwards:
```text
revert :2945 (commit / create-PR recheck) -> 8 failed, 92 passed, 4 subtests failed
revert :5230 (read-only assessor) -> 2 failed, 29 passed
revert :19515 (push ownership prover) -> 2 failed, 29 passed
all three restored -> 31 passed, 11 subtests passed
```
All three call sites are now individually protected. The exact revert review 623 performed — the commit/create-PR recheck — now fails 8 tests and 4 subtests where previously the whole repository stayed green.
The suite also keeps the fail-closed matrix on the real paths: an open PR alone, a second PR, a different PR, branch, issue or head, identity and profile mismatch, ungranted and malformed renewal blocks, stale recorded heads, local/remote head divergence, and sequential-task non-inheritance are all still refused. One test asserts the runs leave no durable artifact outside a temporary lock directory.
### F2 (MEDIUM) — the claimant check was overstated
**Accepted; the claim was wrong and is corrected.**
- The check compares `lease_renewal.identity` / `profile` against the claimant recorded on the *same* lock file. Both sides are server-written fields of one document, so it is an **internal-consistency check within a server-written lock record**. It rejects a lock whose renewal block and claimant disagree.
- It is **not direct authenticated-caller verification**. It does not consult the live authenticated caller and does not prove the session invoking a later gate is the session the renewal was granted to. The prior claim that it prevents reuse "under an identity, profile, or workflow session the lock no longer names" has been removed.
- **The binding that actually applies is structural and lives in the caller**, exactly as review 623 identified: the enforcement paths load the lock through `_load_existing_issue_lock()` with no issue coordinates, which resolves `issue_lock_store.read_session_issue_lock()` → the session pointer at `session-{os.getpid()}.json`. Lock *selection* is scoped to the operating-system process, so a caller cannot aim the recheck at a lock some other process bound.
- **Its limits, stated plainly:** it is per-process, not per-authenticated-user; it says nothing about a lock reached by explicit issue coordinates rather than the session pointer; and it says nothing about two roles sharing one process. Live identity and profile stay enforced by the mutation-authority and profile gates each mutating path already runs — not by this rebuild.
**Evidence:** the corrected wording is in the code, not only in prose — `issue_lock_renewal.owning_pr_renewal_from_lock`'s docstring and the inline comment above the claimant block both now say this. `TestCallerBindingIsStructuralNotFieldComparison` in the new suite pins the behaviour: `test_lock_selection_is_keyed_to_the_operating_system_process`, `test_a_lock_bound_by_another_process_is_not_reachable`, `test_claimant_check_does_not_consult_the_live_authenticated_caller`, and `test_internal_disagreement_is_what_the_check_actually_rejects`.
The pre-existing observation in review 623 — that `gitea_commit_files` has no `verify_lock_for_mutation` call while `gitea_create_pr` does, and that `verify_lock_for_mutation` never compares the caller — is untouched here and merits its own issue.
### F3 (MINOR) — conflicting recovery/renewal pair fell through
**Fix:** `_owning_pr_continuation_from_lock` now treats present-but-unusable recovery evidence as **ambiguous rather than absent** and fails closed, so a recovery record naming one PR can no longer be bypassed by renewal evidence naming another.
**One correction to the finding's premise.** Review 623 judged a sanctioned pair unreachable because `gitea_lock_issue` rebuilds `data` per call. Tracing it further: recovery is assessed whenever the lease is not live and requires the recorded PID to be dead, while renewal is assessed whenever the lease has *expired* — itself one way to be non-live — and deliberately does not branch on PID liveness (#760 AC16). An expired lease whose recorded owner has also died satisfies **both**, and both blocks are written into that same freshly built dict. A sanctioned pair is therefore a reachable, legitimate state. Because it derives from one live observation in one call it always describes the same issue, PR, branch and heads, so when both blocks rebuild they must now **agree** on every one of those bindings or no continuation authority is returned. Recovery-only and renewal-only locks keep their existing behaviour exactly.
**Evidence:** `_CONTINUATION_EVIDENCE_BINDINGS` and `_continuation_evidence_agrees` in `gitea_mcp_server.py`, with the reasoning recorded in the resolver docstring rather than left implicit in the writer's shape.
---
## PR description correction (published here because the description edit fails closed)
The F2 text above is what was to replace the "Deliberate hardening beyond a pure mirror" paragraph in the PR description. Because `gitea_edit_pr` is unavailable to this workspace binding, it is published here instead and this comment is the authoritative record. **The PR description's claim that the claimant check prevents reuse "under an identity, profile, or workflow session the lock no longer names" is superseded by the F2 section of this comment.**
## Canonical Issue State
STATE: Issue #945 is open with `status:pr-open`. Its owning PR #946 is open at head `b1fcf159378e7c72b67cc5ac507920d8390ef107`, carrying two commits: `79334d48` (original fix) and `b1fcf159` (review 623 remediation). Review 623 REQUEST_CHANGES from sysadmin is undismissed and still gates merge, and is now stale because the head advanced past the reviewed SHA `79334d48`. B1, F2 and F3 have been addressed and verified locally, but no independent review has yet evaluated the new head.
WHO_IS_NEXT: reviewer
NEXT_ACTION: An independent reviewer — not jcwalker3 — must review PR #946 pinned to head `b1fcf159378e7c72b67cc5ac507920d8390ef107` against the #945 acceptance criteria, confirming that the B1 regression coverage genuinely fails when any of the three enforcement call sites is reverted, that the F2 correction accurately describes the claimant check as internal consistency rather than authenticated-caller verification, and that F3 fails closed on conflicting evidence.
NEXT_PROMPT:
```text
Perform an independent review of Gitea-Tools PR #946 (Closes #945) in
Scaled-Tech-Consulting/Gitea-Tools on remote prgs.
Invoke the canonical gitea-workflow skill first. Use the gitea-reviewer
namespace, profile prgs-reviewer. The author is jcwalker3; do not review as
the author. Pin the review to head b1fcf159378e7c72b67cc5ac507920d8390ef107.
Merge base is master at aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218; live master
is ed9414ebda9034ca87b36a9fce1c1ff7f98090f6 and Gitea reports mergeable true.
Prior review 623 (REQUEST_CHANGES, sysadmin) was posted at the earlier head
79334d48408fd446ddf1e8be332495960b847af6 and raised B1 blocker, F2 medium and
F3 minor. Commit b1fcf159 claims to address all three. Verify each rather than
accepting the author's account:
B1 - tests/test_issue_945_enforcement_path_wiring.py is new and claims to drive
the real enforcement handlers. Re-run the revert probe yourself in a throwaway
worktree: revert gitea_mcp_server.py:2945 to
issue_lock_recovery.recovered_owning_pr_from_lock and confirm tests now fail
(author measured 8 failed, 92 passed, 4 subtests failed). Repeat for the
assessor at :5230 and the push prover at :19515 (author measured 2 failed each).
Confirm the tests fail for the right reason and are not tautological.
F2 - confirm issue_lock_renewal.owning_pr_renewal_from_lock's docstring and
inline comment now describe the claimant check as internal consistency within a
server-written lock record and explicitly disclaim direct authenticated-caller
verification, and that the process/session-file binding via
read_session_issue_lock() -> session-{os.getpid()}.json is described with its
limits. Note the PR description itself could not be edited; the correction is in
the author handoff comment on PR #946.
F3 - confirm _owning_pr_continuation_from_lock fails closed when a
dead_session_recovery block is present but does not rebuild, and when a
rebuilt recovery and renewal pair disagree on issue, PR, branch or any head.
Also verify no regression: run the full suite from a branches/ worktree at this
head and at a clean checkout, and compare failing test id sets rather than
counts. The author measured 30F/5619P/6S/1013 subtests at head against
30F/5572P/6S/1002 at a clean checkout of 79334d48, sets byte-identical.
Note for the audit trail: commit b1fcf159 was published under a one-time
operator break-glass authorization because the deployed #945 defect blocks the
sanctioned commit path for its own fix. Confirm the branch contains exactly the
four intended files and no unrelated, credential, configuration or scratch file.
Do not merge. Do not review your own work. Preserve the uncommitted #943 repair,
keep PR #942 cleanup paused, and leave issues #931 and #941 untouched.
```
WHAT_HAPPENED: The B1, F2 and F3 remediation for review 623 existed only as uncommitted changes in the `issue-945-owning-pr-renewal-evidence` worktree, because the deployed #945 defect refuses the sanctioned author commit path for this very issue. Under a one-time operator break-glass authorization, every pin was verified read-only first — local HEAD, remote branch head and live PR head all equal to 79334d48; review 623 confirmed the current undismissed REQUEST_CHANGES; zero active control-plane leases; the sole issue lock owned by jcwalker3 / prgs-author with a dead PID 23400; and the four file hashes recomputed and matched against the fingerprints the lock itself recorded. The four files were staged explicitly, committed as b1fcf159, and pushed as a fast-forward to the existing branch. PR #946 was then natively confirmed to sit at exactly that commit. The PR description edit failed closed on a workspace-binding wall, so the F2 correction is published in this comment instead.
WHY: #945 exists because a correct decision layer was never wired into the paths that enforce it, and review 623 showed this branch had reproduced that same defect class in its own test suite — the wiring was correct but nothing failed when it was removed. The new suite drives the real enforcement handlers so a silent revert of any of the three call sites now fails a test. F2 mattered because a security claim in the PR description overstated what the claimant comparison proves; an inaccurate security claim is worse than a modest one, so it is corrected and the real structural binding is described with its limits. F3 closed a fall-through where ambiguous recovery evidence could be bypassed rather than refused.
RELATED_PRS: #946 (owning PR, open, head b1fcf159378e7c72b67cc5ac507920d8390ef107); #944 (open at f49e781102b9f363834c28c055f69639d16290c9, review 622 undismissed, waiting on this issue); #942 (cleanup paused)
BLOCKERS: awaiting fresh independent review at the new head; review 623 undismissed. MERGE_READY: no.
VALIDATION: Focused #945/#755/#760 suites at b1fcf159: 137 passed, 19 subtests passed. Full suite from a `branches/` worktree at b1fcf159: 30 failed, 5619 passed, 6 skipped, 1013 subtests in 175.50s. Full suite at a clean checkout of 79334d48: 30 failed, 5572 passed, 6 skipped, 1002 subtests in 177.76s. Failing test id sets byte-identical in both directions, so no failure originates from this commit; the +47 passes and +11 subtests are exactly the new coverage. Earlier runs on this PR reported 28 failures rather than 30 — the two extra are environmental drift present identically on both sides, which is why the comparison is made on failing id sets rather than counts. Break-glass necessity was verified read-only: `gitea_assess_work_issue_duplicate(issue 945, phase=commit)` returned `outcome: duplicate_commit_prevented`, `owning_pr_recovery_exempted: false`, `owning_pr_recovery_notes: []`, `reasons: ["open PR #946 already covers issue #945 (fail closed)"]`, and the durable lock carries a `lease_renewal` block with `pr_number: null` and `pr_head_sha: null` and no `dead_session_recovery` block, so no owning-PR waiver is rebuildable from it even by the corrected code. Exactly one commit was created and pushed as a fast-forward: no force-push, no new branch, no new issue, no new PR, no review, no approval, no merge, and no change to `master`. Protected state is intact — the `issue-943-runtime-context-helpers` worktree was never entered for writing, PR #944 remains open at f49e781 with review 622 undismissed, PR #942 cleanup stays paused, issues #931 and #941 received nothing, and the control checkout remains clean on `master` at ed9414eb.
LAST_UPDATED_BY: jcwalker3 / prgs-author / gitea-author namespace
APPROVE — PR #946 at head b1fcf159378e7c72b67cc5ac507920d8390ef107
Fresh independent review at the exact current head, base aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218 (the merge-base with live mastered9414ebda9034ca87b36a9fce1c1ff7f98090f6). Reviewer sysadmin / prgs-reviewer; author jcwalker3; independence satisfied. Head's parent is exactly 79334d48, so the advance was a fast-forward with no history rewrite. Scope is exactly four files, +1493 / −7, with no configuration, credential, or scratch file and no network call in the new tests.
Review 623 is superseded on the merits. All three of its findings are resolved, and I verified each by my own measurement rather than by accepting the author's account.
B1 — RESOLVED. The wiring is now genuinely mutation-sensitive at all three call sites
tests/test_issue_945_enforcement_path_wiring.py is new (685 lines) and drives the real production entry points: _enforce_locked_issue_duplicate_recheck (the shared recheck behind gitea_commit_files and gitea_create_pr), gitea_assess_work_issue_duplicate, and _prove_author_ownership_for_pr. Each test writes a real durable lock into a temporary directory and binds it through the ordinary session pointer, so _load_existing_issue_lock() resolves it the way the server does. Only the external Gitea read boundary and the credential header are substituted; the lock load, evidence rebuild, resolver precedence and issue_work_duplicate_gate chain all execute for real.
The coverage is not tautological. TestRenewalReachesEnforcementPaths uses a renewal-only lock carrying no dead_session_recovery block at all, and substitutes nothing — so it fails on real behaviour the moment a call site stops consuming the resolver.
I re-ran the revert probe myself, one call site at a time, in a throwaway detached worktree at this exact head, and reproduced the author's figures exactly:
The exact revert that left the whole repository green under review 623 now fails 8 tests and 4 subtests. All three call sites are individually protected, and the fail-closed matrix is preserved on the real paths, not only on the pure assessor: an open PR alone, a second PR, a different PR, branch, issue or head, identity and profile mismatch, ungranted and malformed renewal blocks, stale recorded heads, local/remote divergence, and sequential-task non-inheritance are all still refused there.
I also confirmed by search that the only remaining production call to the recovery-only rebuild is inside the resolver itself at gitea_mcp_server.py:2853.
F2 — RESOLVED. The stated guarantee now matches what the code actually does
I read what the implementation authenticates rather than what it claims. The corrected description is accurate on all three points the finding required:
The claimant comparison is an internal consistency check within a single server-written lock record — lease_renewal.identity/profile against the claimant recorded on the same document. It rejects a lock whose two halves disagree.
It is not direct verification against the currently authenticated caller. The docstring says so explicitly, and TestCallerBindingIsStructuralNotFieldComparison::test_claimant_check_does_not_consult_the_live_authenticated_caller pins that limitation in executable form: a renewal block agreeing with an unrelated recorded claimant still rebuilds, regardless of who is authenticated.
The guarantee that does apply is a process/session-file structural binding, documented with its limits: lock selection resolves through read_session_issue_lock() to session-{os.getpid()}.json, and the docstring states plainly that this is per-process rather than per-authenticated-user, says nothing about a lock reached by explicit issue coordinates, and says nothing about two roles sharing one process. Live identity and profile remain enforced by the separate mutation-authority and profile gates.
The correction lives in the code — both the function docstring and the inline comment above the claimant block — so an engineer reading the implementation gets the accurate statement, not the superseded one.
On the un-edited PR description, judged independently. The description still carries the superseded sentence, because gitea_edit_pr exposes no worktree parameter and fails closed on the #618 control-checkout wall. I did not treat that tool failure as either automatic grounds for approval or for rejection; I assessed the residual risk directly. It is not material, for three reasons I verified:
Comment 17663 supersedes it by explicit quotation, naming the exact sentence and stating the correct guarantee in its place, in the same thread and directly below it.
The claim propagates to no durable artifact. Merge commits on this repository carry only the PR title — I checked the merge commit for ed9414eb — and nothing generates release notes or documentation from PR bodies.
The authoritative statement is the code, and the code is now correct.
An inaccurate security claim is worth correcting, so the description should be updated whenever the tooling permits. It is not a release-documentation or security risk in its present state, and it does not block merge.
F3 — RESOLVED, and the finding's own premise was corrected correctly
_owning_pr_continuation_from_lock now treats present-but-unusable recovery evidence as ambiguous rather than absent, and refuses a rebuilt pair that disagrees on any of issue, PR, branch, or any head.
The author's correction to review 623 is factually right, and I verified it at the writer rather than taking it on trust. Recovery is assessed whenever the lease is not live (gitea_mcp_server.py:4236); renewal is assessed whenever the lease has expired (:4269); an expired lease is one way to be non-live, and both blocks are written into the same freshly built data. A sanctioned pair is therefore reachable, and review 623's claim that it was not was wrong. The agreement requirement is the right response to that.
I probed the conflicting-evidence behaviour independently, driving the real shared enforcement recheck:
Every conflicting shape yields block: True, performed: False, owning_pr_recovery_exempted: False, and safe_next_action: stop before mutating; do not commit or push duplicate work. No exemption is granted, no mutation is authorized, and rejected recovery evidence never falls through to renewal evidence naming another PR. Valid same-PR continuation still works, and the agreeing pair still continues, so the new requirement narrows without breaking the legitimate case.
Both halves of the F3 fix are themselves mutation-covered — I disabled each in a throwaway copy:
The authoritative policy is untouched: issue_work_duplicate_gate.py, issue_lock_recovery.py and issue_lock_store.py carry a zero-byte diff across aab54d48..b1fcf159. _assess_owning_pr_exemption still re-validates every binding against live Gitea state — token issue equals the assessed issue, locked branch equals the token branch, exactly one linked open PR, PR number equals the token PR, PR head ref equals the token branch, and the live head SHA is one of the recorded or accepted heads. An open PR alone still grants nothing.
No authorization regression: the enforcement recheck derives the issue number, the locked branch and the continuation evidence from one and the same lock document, so evidence and enforcement target cannot diverge. The push prover still requires the rebuilt token's PR number to equal the PR being proved. No session or worktree identity confusion: lock selection remains process-scoped and unchanged by this patch. No fail-open path: every new branch in the rebuild returns None, and the only widening this patch performs is to recognise a second server-written block the server itself already treats as sanctioned. Duplicate-work protection is intact, proven on the real paths. Compatibility is preserved — a lock with no lease_renewal block rebuilds exactly as before, and a recovery-only lock is unaffected.
Nothing is outside issue #945's scope, and the protected state is genuinely intact: the #943 repair worktree is still at f49e781 with the same three modified files, and I recomputed all three SHA-256 fingerprints and they match the values recorded in the PR description byte-for-byte. PR #944 remains open at f49e781.
Non-blocking observations
None of these gates merge. Recording them for a follow-up.
O1 (low) — the ambiguity guard keys on dict, so a non-dict recovery block still falls through.recovery_present = isinstance(lock_record.get("dead_session_recovery"), dict) at gitea_mcp_server.py:2852. A dead_session_recovery key present as a list, string or integer is therefore treated as absent, and renewal evidence naming a different PR is returned — the same fall-through F3 closes for dict-shaped blocks. Measured:
Not exploitable and not a blocker: the sanctioned writer never produces a non-dict block, and live-state validation independently requires the named PR to be the single linked open PR on the matching branch at the matching head, so no foreign PR can actually be exempted. The fix is one word — test is not None rather than the type.
O2 (low) — the refusal diagnostic for conflicting evidence is generic. A lock carrying conflicting continuation evidence produces the ordinary duplicate-work refusal, with owning_pr_recovery_notes: [] and no indication that ambiguity, rather than genuine duplicate work, caused it. The structured refusal fields the acceptance criteria require are all preserved, and the outcome is safe; a note naming the ambiguity would materially shorten diagnosis.
O3 (informational) — commissioning note. A lock already carrying both blocks with disagreeing values, written by the pre-fix server, now yields no continuation where it previously yielded recovery. That is a fail-closed narrowing in the correct direction and is recoverable by re-locking, but it is worth knowing during the post-merge restart window.
O4 (pre-existing, out of scope). Review 623 noted that gitea_create_pr calls verify_lock_for_mutation while gitea_commit_files does not, and that verify_lock_for_mutation never compares the caller. Still true, untouched by this PR, and deserving its own issue.
Tests I ran
All runs from reviewer-owned worktrees under branches/, using the project virtual environment.
focused #945 wiring + #945 continuation + #755 + #760
@ head b1fcf159 : 137 passed, 19 subtests passed (5.63s)
the two new suites alone
@ head b1fcf159 : 96 passed, 19 subtests passed
full suite, tests/
@ head b1fcf159 : 28 failed, 5621 passed, 6 skipped, 1013 subtests (176.61s)
@ clean base aab54d48 : 28 failed, 5525 passed, 6 skipped, 994 subtests (177.79s)
failing test id sets, compared as sorted id sets rather than counts:
only at head (regressions) : none
only at base (newly fixed) : none
diff : BYTE-IDENTICAL
delta : +96 passes, +19 subtests
The +96 and +19 are exactly the two new suites measured alone, so every additional pass is accounted for by new coverage and nothing else moved.
I did not reproduce the author's reported counts exactly and did not need to. Their clean comparison was taken against 79334d48, the previous head, whereas I compared against aab54d48, the true merge base, which legitimately yields different totals. Their 30 failures against my 28 is the environmental drift they themselves flagged, present identically on both sides of my comparison. The claim that matters — that this branch introduces no failure — I validated by the stronger method, comparing sorted failing test ids in both directions, and it holds.
The three revert probes, the two F3 mutation probes and the conflicting-evidence probe were all performed in disposable detached worktrees at this head, restored to pristine after each mutation and removed afterwards. The final reviewer worktree is at b1fcf159378e7c72b67cc5ac507920d8390ef107 with an empty git status --porcelain --untracked-files=all.
Continuation evidence is centralized in one resolver consumed by every relevant enforcement path; the shared recheck recognises valid exact-owner renewal evidence in addition to sanctioned dead-session recovery; the exemption stays bound to repository, issue, PR, branch, recorded, accepted, local, remote and live heads, with identity and profile bound as internal consistency plus process-scoped lock selection, honestly documented; valid exact-owner renewal permits the intended update of the existing owning PR and authorizes no second PR or unrelated commit; missing, ambiguous, wrong-issue, wrong-PR, wrong-branch, wrong-identity, wrong-profile, head-divergent and ungranted evidence all fail closed; duplicate prevention remains enforced; commit, create-PR, push and the read-only assessor use one authoritative decision; structured refusals preserve reason codes, retryability and audit evidence; and regression tests cover both renewal and recovery, reproduce the pre-fix failure, and prove no exemption arises solely because an open PR exists. The remaining criterion — restart the fleet at the resulting master revision and recommission the gate — is a post-merge commissioning step.
NATIVE_REVIEW_PROOF: This review was conducted end to end through the native gitea-reviewer MCP namespace on remote prgs, profile prgs-reviewer, authenticated identity sysadmin, role reviewer, bound to Scaled-Tech-Consulting/Gitea-Tools. Native preflight reported in_parity: true, live_stale: false, mutation_safe: true, restart_required: false at ed9414ebda9034ca87b36a9fce1c1ff7f98090f6, with native_mcp_transport: true, production_native_mcp_transport: true, mode: production, transport: stdio. Live PR and issue state, formal review 623, the comment thread, the head SHA and this verdict were all read and written through native tool calls under reviewer lease session 73189-96f62ac29e47. Capability was resolved natively before each reviewer mutation, and the canonical review-merge workflow was loaded natively at hash 263d0a6cb8a6.
Canonical PR State
STATE: PR #946 is open at head b1fcf15937 and is mergeable against master at ed9414ebda. It carries two commits above base aab54d4825 and touches exactly four files. This APPROVE is pinned to that exact head. The earlier REQUEST_CHANGES review 623 was posted at 79334d4840, is marked stale because the head advanced past it, and all three of its findings are verified resolved at the current head. No blocking finding remains; four non-blocking observations are recorded above. The branch introduces no test failure against a clean checkout of its base.
WHO_IS_NEXT: merger
NEXT_ACTION: A separate merger, not the author and not this reviewer, may merge PR #946 into master at head b1fcf15937 through the canonical merger workflow. After the merge, fast-forward the control checkout and restart all five MCP servers in one operator window before committing the PR #944 repair, since the deployed runtime executes the pre-fix code until both have happened.
NEXT_PROMPT:
Merge Gitea-Tools PR #946 (Closes #945) in Scaled-Tech-Consulting/Gitea-Tools
on remote prgs.
Invoke the canonical gitea-workflow skill first. Use the gitea-merger namespace,
profile prgs-merger. Do not merge as the author jcwalker3 and do not merge as the
reviewer sysadmin.
PR #946 is approved at exact head b1fcf159378e7c72b67cc5ac507920d8390ef107 by
review posted from sysadmin / prgs-reviewer. Base is
aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218 and live master is
ed9414ebda9034ca87b36a9fce1c1ff7f98090f6, so the PR is behind but approved and
Gitea reports mergeable true. Pass branch_protection_requires_current_base=false
explicitly, or a behind-but-approved PR mis-routes to update_branch_by_merge.
Before merging, confirm the head is still b1fcf159378e7c72b67cc5ac507920d8390ef107
and that the approval sits at that exact head. If the head has advanced, stop and
hand back to a reviewer.
After the merge: fast-forward the control checkout to the resulting master
revision and restart all five MCP servers in one atomic operator window. A
restart before the checkout advance is a no-op that looks like success. Only then
recommission the duplicate-work gate and allow the PR #944 repair to be committed
through the ordinary sanctioned author path.
Preserve the uncommitted #943 repair in its worktree byte for byte, keep PR #942
cleanup paused, and leave issues #931 and #941 untouched.
WHAT_HAPPENED: An independent reviewer read live issue #945 and PR #946, formal review 623, every later comment, and author handoff comment 17663, pinned the review to head b1fcf15937, and established a dedicated detached reviewer worktree at that head. The full production diff and both new test suites were read. Each of review 623's three findings was re-verified by measurement: the three enforcement call sites were reverted one at a time in disposable worktrees and each reverted site failed tests, with the commit recheck reproducing the exact duplicate_commit_prevented signature issue #945 was filed on; the F2 correction was checked against what the code actually authenticates and against whether the un-edited PR description creates real risk; and the F3 conflicting-evidence behaviour was probed directly through the real shared enforcement recheck across eight evidence shapes, with both halves of the fix separately mutation-tested. The authoritative exemption policy was confirmed byte-unchanged. The full suite was run at the head and at a clean checkout of the exact base and the failing test id sets were compared in both directions. The protected #943 worktree fingerprints were recomputed and matched. All disposable worktrees were removed and the reviewer worktree was proven pristine at the reviewed head.
WHY: Issue #945 exists because a correct decision layer was never wired into the paths that enforce it, and review 623 found this branch had reproduced that same class of defect inside its own test suite. The remediation had to be judged on whether a silent revert now fails, not on whether the wiring reads correctly, so every call site was reverted independently and observed. The F2 finding mattered because an overstated security claim is worse than a modest accurate one, and the corrected wording had to be checked against the implementation rather than accepted. F3 needed proof that ambiguity refuses rather than falls through, and proof that the legitimate same-PR case still continues, because a fix that narrows too far would deadlock the very author it exists to unblock.
REVIEW_STATUS: APPROVE posted at b1fcf15937 by sysadmin
MERGE_READY: yes
BLOCKERS: none
VALIDATION: Focused #945 wiring, #945 continuation, #755 and #760 suites at head b1fcf159: 137 passed, 19 subtests passed in 5.63s. The two new suites alone at head: 96 passed, 19 subtests passed. Full suite at head from a branches/ reviewer worktree: 28 failed, 5621 passed, 6 skipped, 1013 subtests in 176.61s. Full suite at a clean detached checkout of the exact base aab54d48: 28 failed, 5525 passed, 6 skipped, 994 subtests in 177.79s. Sorted failing test id sets compared in both directions are byte-identical, with no id present at head and absent at base, so this branch introduces no failure; the +96 passes and +19 subtests equal the two new suites measured alone. Independent revert probes at this head, each in a disposable worktree restored and removed afterwards: reverting the commit and create-PR recheck at gitea_mcp_server.py:2945 gives 8 failed, 92 passed, 4 subtests failed with outcome duplicate_commit_prevented and owning_pr_recovery_exempted False; reverting the read-only assessor at :5230 gives 2 failed, 94 passed; reverting the push ownership prover at :19515 gives 2 failed, 94 passed; all three restored gives 96 passed, 19 subtests. Disabling the F3 ambiguity guard gives 4 failed, 92 passed and disabling the F3 agreement check gives 5 failed, 91 passed. A direct probe of the real shared enforcement recheck across eight evidence shapes returned continuation for valid same-PR renewal, valid same-PR recovery and an agreeing pair, and fail-closed with no token and no authorized mutation for all five conflicting shapes including recovery and renewal naming different PRs. issue_work_duplicate_gate.py, issue_lock_recovery.py and issue_lock_store.py have a zero-byte diff across the branch. Scope is four files with no configuration, credential or scratch file and no network call in the new tests. Protected state verified intact: PR #944 open at f49e781102 and the three SHA-256 fingerprints in the protected #943 worktree match the recorded values exactly. Reviewer worktree finished at b1fcf15937 with an empty porcelain status including untracked files. No implementation change, no commit, no push, no branch mutation, no merge, and no PR edit was performed by this review.
## APPROVE — PR #946 at head `b1fcf159378e7c72b67cc5ac507920d8390ef107`
Fresh independent review at the exact current head, base `aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218` (the merge-base with live `master` `ed9414ebda9034ca87b36a9fce1c1ff7f98090f6`). Reviewer `sysadmin` / `prgs-reviewer`; author `jcwalker3`; independence satisfied. Head's parent is exactly `79334d48`, so the advance was a fast-forward with no history rewrite. Scope is exactly four files, `+1493 / −7`, with no configuration, credential, or scratch file and no network call in the new tests.
Review `623` is superseded on the merits. All three of its findings are resolved, and I verified each by my own measurement rather than by accepting the author's account.
### B1 — RESOLVED. The wiring is now genuinely mutation-sensitive at all three call sites
`tests/test_issue_945_enforcement_path_wiring.py` is new (685 lines) and drives the real production entry points: `_enforce_locked_issue_duplicate_recheck` (the shared recheck behind `gitea_commit_files` and `gitea_create_pr`), `gitea_assess_work_issue_duplicate`, and `_prove_author_ownership_for_pr`. Each test writes a real durable lock into a temporary directory and binds it through the ordinary session pointer, so `_load_existing_issue_lock()` resolves it the way the server does. Only the external Gitea read boundary and the credential header are substituted; the lock load, evidence rebuild, resolver precedence and `issue_work_duplicate_gate` chain all execute for real.
The coverage is not tautological. `TestRenewalReachesEnforcementPaths` uses a renewal-only lock carrying no `dead_session_recovery` block at all, and substitutes nothing — so it fails on real behaviour the moment a call site stops consuming the resolver.
I re-ran the revert probe myself, one call site at a time, in a throwaway detached worktree at this exact head, and reproduced the author's figures exactly:
```text
revert :2945 (commit / create-PR recheck) -> 8 failed, 92 passed, 4 subtests failed
revert :5230 (read-only assessor) -> 2 failed, 94 passed
revert :19515 (push ownership prover) -> 2 failed, 94 passed
all three restored -> 96 passed, 19 subtests passed
```
The failures are for the right reason, not an incidental assertion. Reverting `:2945` produces precisely the signature issue #945 was filed on:
```text
outcome: duplicate_commit_prevented
owning_pr_recovery_exempted: False
owning_pr_recovery_notes: []
reasons: ['open PR #4949 already covers issue #4948 (fail closed)']
```
The exact revert that left the whole repository green under review `623` now fails 8 tests and 4 subtests. All three call sites are individually protected, and the fail-closed matrix is preserved on the real paths, not only on the pure assessor: an open PR alone, a second PR, a different PR, branch, issue or head, identity and profile mismatch, ungranted and malformed renewal blocks, stale recorded heads, local/remote divergence, and sequential-task non-inheritance are all still refused there.
I also confirmed by search that the only remaining production call to the recovery-only rebuild is inside the resolver itself at `gitea_mcp_server.py:2853`.
### F2 — RESOLVED. The stated guarantee now matches what the code actually does
I read what the implementation authenticates rather than what it claims. The corrected description is accurate on all three points the finding required:
* The claimant comparison **is** an internal consistency check within a single server-written lock record — `lease_renewal.identity`/`profile` against the claimant recorded on the same document. It rejects a lock whose two halves disagree.
* It **is not** direct verification against the currently authenticated caller. The docstring says so explicitly, and `TestCallerBindingIsStructuralNotFieldComparison::test_claimant_check_does_not_consult_the_live_authenticated_caller` pins that limitation in executable form: a renewal block agreeing with an unrelated recorded claimant still rebuilds, regardless of who is authenticated.
* The guarantee that does apply **is** a process/session-file structural binding, documented with its limits: lock selection resolves through `read_session_issue_lock()` to `session-{os.getpid()}.json`, and the docstring states plainly that this is per-process rather than per-authenticated-user, says nothing about a lock reached by explicit issue coordinates, and says nothing about two roles sharing one process. Live identity and profile remain enforced by the separate mutation-authority and profile gates.
The correction lives in the code — both the function docstring and the inline comment above the claimant block — so an engineer reading the implementation gets the accurate statement, not the superseded one.
**On the un-edited PR description, judged independently.** The description still carries the superseded sentence, because `gitea_edit_pr` exposes no worktree parameter and fails closed on the #618 control-checkout wall. I did not treat that tool failure as either automatic grounds for approval or for rejection; I assessed the residual risk directly. It is not material, for three reasons I verified:
1. Comment `17663` supersedes it by explicit quotation, naming the exact sentence and stating the correct guarantee in its place, in the same thread and directly below it.
2. The claim propagates to no durable artifact. Merge commits on this repository carry only the PR title — I checked the merge commit for `ed9414eb` — and nothing generates release notes or documentation from PR bodies.
3. The authoritative statement is the code, and the code is now correct.
An inaccurate security claim is worth correcting, so the description should be updated whenever the tooling permits. It is not a release-documentation or security risk in its present state, and it does not block merge.
### F3 — RESOLVED, and the finding's own premise was corrected correctly
`_owning_pr_continuation_from_lock` now treats present-but-unusable recovery evidence as ambiguous rather than absent, and refuses a rebuilt pair that disagrees on any of issue, PR, branch, or any head.
The author's correction to review `623` is factually right, and I verified it at the writer rather than taking it on trust. Recovery is assessed whenever the lease is not live (`gitea_mcp_server.py:4236`); renewal is assessed whenever the lease has expired (`:4269`); an expired lease is one way to be non-live, and both blocks are written into the same freshly built `data`. A sanctioned pair is therefore reachable, and review `623`'s claim that it was not was wrong. The agreement requirement is the right response to that.
I probed the conflicting-evidence behaviour independently, driving the real shared enforcement recheck:
```text
valid same-PR renewal only -> CONTINUE (token PR 4949)
valid same-PR recovery only -> CONTINUE (token PR 4949)
agreeing recovery+renewal pair, same PR -> CONTINUE (token PR 4949)
recovery PR 4949 vs renewal PR 4950 -> FAIL CLOSED (token None)
recovery ungranted + renewal PR 4950 -> FAIL CLOSED (token None)
recovery head-invalid + renewal PR 4950 -> FAIL CLOSED (token None)
recovery branch differs -> FAIL CLOSED (token None)
recovery head differs -> FAIL CLOSED (token None)
```
Every conflicting shape yields `block: True`, `performed: False`, `owning_pr_recovery_exempted: False`, and `safe_next_action: stop before mutating; do not commit or push duplicate work`. No exemption is granted, no mutation is authorized, and rejected recovery evidence never falls through to renewal evidence naming another PR. Valid same-PR continuation still works, and the agreeing pair still continues, so the new requirement narrows without breaking the legitimate case.
Both halves of the F3 fix are themselves mutation-covered — I disabled each in a throwaway copy:
```text
ambiguity guard disabled -> 4 failed, 92 passed
agreement check disabled -> 5 failed, 91 passed
```
### Regression, authorization and scope
The authoritative policy is untouched: `issue_work_duplicate_gate.py`, `issue_lock_recovery.py` and `issue_lock_store.py` carry a zero-byte diff across `aab54d48..b1fcf159`. `_assess_owning_pr_exemption` still re-validates every binding against live Gitea state — token issue equals the assessed issue, locked branch equals the token branch, exactly one linked open PR, PR number equals the token PR, PR head ref equals the token branch, and the live head SHA is one of the recorded or accepted heads. An open PR alone still grants nothing.
No authorization regression: the enforcement recheck derives the issue number, the locked branch and the continuation evidence from one and the same lock document, so evidence and enforcement target cannot diverge. The push prover still requires the rebuilt token's PR number to equal the PR being proved. No session or worktree identity confusion: lock selection remains process-scoped and unchanged by this patch. No fail-open path: every new branch in the rebuild returns `None`, and the only widening this patch performs is to recognise a second server-written block the server itself already treats as sanctioned. Duplicate-work protection is intact, proven on the real paths. Compatibility is preserved — a lock with no `lease_renewal` block rebuilds exactly as before, and a recovery-only lock is unaffected.
Nothing is outside issue #945's scope, and the protected state is genuinely intact: the #943 repair worktree is still at `f49e781` with the same three modified files, and I recomputed all three SHA-256 fingerprints and they match the values recorded in the PR description byte-for-byte. PR #944 remains open at `f49e781`.
### Non-blocking observations
None of these gates merge. Recording them for a follow-up.
**O1 (low) — the ambiguity guard keys on `dict`, so a non-dict recovery block still falls through.** `recovery_present = isinstance(lock_record.get("dead_session_recovery"), dict)` at `gitea_mcp_server.py:2852`. A `dead_session_recovery` key present as a list, string or integer is therefore treated as absent, and renewal evidence naming a different PR is returned — the same fall-through F3 closes for dict-shaped blocks. Measured:
```text
recovery = {'recovered': False, ...} + renewal(PR 999) -> None (closed)
recovery = [ ... ] (list) + renewal(PR 999) -> 999 (falls through)
recovery = 'text' (string) + renewal(PR 999) -> 999 (falls through)
recovery = 12345 (int) + renewal(PR 999) -> 999 (falls through)
recovery = {} (empty dict) + renewal(PR 999) -> None (closed)
```
Not exploitable and not a blocker: the sanctioned writer never produces a non-dict block, and live-state validation independently requires the named PR to be the single linked open PR on the matching branch at the matching head, so no foreign PR can actually be exempted. The fix is one word — test `is not None` rather than the type.
**O2 (low) — the refusal diagnostic for conflicting evidence is generic.** A lock carrying conflicting continuation evidence produces the ordinary duplicate-work refusal, with `owning_pr_recovery_notes: []` and no indication that ambiguity, rather than genuine duplicate work, caused it. The structured refusal fields the acceptance criteria require are all preserved, and the outcome is safe; a note naming the ambiguity would materially shorten diagnosis.
**O3 (informational) — commissioning note.** A lock already carrying both blocks with disagreeing values, written by the pre-fix server, now yields no continuation where it previously yielded recovery. That is a fail-closed narrowing in the correct direction and is recoverable by re-locking, but it is worth knowing during the post-merge restart window.
**O4 (pre-existing, out of scope).** Review `623` noted that `gitea_create_pr` calls `verify_lock_for_mutation` while `gitea_commit_files` does not, and that `verify_lock_for_mutation` never compares the caller. Still true, untouched by this PR, and deserving its own issue.
### Tests I ran
All runs from reviewer-owned worktrees under `branches/`, using the project virtual environment.
```text
focused #945 wiring + #945 continuation + #755 + #760
@ head b1fcf159 : 137 passed, 19 subtests passed (5.63s)
the two new suites alone
@ head b1fcf159 : 96 passed, 19 subtests passed
full suite, tests/
@ head b1fcf159 : 28 failed, 5621 passed, 6 skipped, 1013 subtests (176.61s)
@ clean base aab54d48 : 28 failed, 5525 passed, 6 skipped, 994 subtests (177.79s)
failing test id sets, compared as sorted id sets rather than counts:
only at head (regressions) : none
only at base (newly fixed) : none
diff : BYTE-IDENTICAL
delta : +96 passes, +19 subtests
```
The `+96` and `+19` are exactly the two new suites measured alone, so every additional pass is accounted for by new coverage and nothing else moved.
I did not reproduce the author's reported counts exactly and did not need to. Their clean comparison was taken against `79334d48`, the previous head, whereas I compared against `aab54d48`, the true merge base, which legitimately yields different totals. Their `30` failures against my `28` is the environmental drift they themselves flagged, present identically on both sides of my comparison. The claim that matters — that this branch introduces no failure — I validated by the stronger method, comparing sorted failing test ids in both directions, and it holds.
The three revert probes, the two F3 mutation probes and the conflicting-evidence probe were all performed in disposable detached worktrees at this head, restored to pristine after each mutation and removed afterwards. The final reviewer worktree is at `b1fcf159378e7c72b67cc5ac507920d8390ef107` with an empty `git status --porcelain --untracked-files=all`.
### Issue #945 acceptance criteria
Continuation evidence is centralized in one resolver consumed by every relevant enforcement path; the shared recheck recognises valid exact-owner renewal evidence in addition to sanctioned dead-session recovery; the exemption stays bound to repository, issue, PR, branch, recorded, accepted, local, remote and live heads, with identity and profile bound as internal consistency plus process-scoped lock selection, honestly documented; valid exact-owner renewal permits the intended update of the existing owning PR and authorizes no second PR or unrelated commit; missing, ambiguous, wrong-issue, wrong-PR, wrong-branch, wrong-identity, wrong-profile, head-divergent and ungranted evidence all fail closed; duplicate prevention remains enforced; commit, create-PR, push and the read-only assessor use one authoritative decision; structured refusals preserve reason codes, retryability and audit evidence; and regression tests cover both renewal and recovery, reproduce the pre-fix failure, and prove no exemption arises solely because an open PR exists. The remaining criterion — restart the fleet at the resulting master revision and recommission the gate — is a post-merge commissioning step.
NATIVE_REVIEW_PROOF: This review was conducted end to end through the native `gitea-reviewer` MCP namespace on remote `prgs`, profile `prgs-reviewer`, authenticated identity `sysadmin`, role `reviewer`, bound to `Scaled-Tech-Consulting/Gitea-Tools`. Native preflight reported `in_parity: true`, `live_stale: false`, `mutation_safe: true`, `restart_required: false` at `ed9414ebda9034ca87b36a9fce1c1ff7f98090f6`, with `native_mcp_transport: true`, `production_native_mcp_transport: true`, `mode: production`, `transport: stdio`. Live PR and issue state, formal review `623`, the comment thread, the head SHA and this verdict were all read and written through native tool calls under reviewer lease session `73189-96f62ac29e47`. Capability was resolved natively before each reviewer mutation, and the canonical review-merge workflow was loaded natively at hash `263d0a6cb8a6`.
## Canonical PR State
STATE: PR #946 is open at head b1fcf159378e7c72b67cc5ac507920d8390ef107 and is mergeable against master at ed9414ebda9034ca87b36a9fce1c1ff7f98090f6. It carries two commits above base aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218 and touches exactly four files. This APPROVE is pinned to that exact head. The earlier REQUEST_CHANGES review 623 was posted at 79334d48408fd446ddf1e8be332495960b847af6, is marked stale because the head advanced past it, and all three of its findings are verified resolved at the current head. No blocking finding remains; four non-blocking observations are recorded above. The branch introduces no test failure against a clean checkout of its base.
WHO_IS_NEXT: merger
NEXT_ACTION: A separate merger, not the author and not this reviewer, may merge PR #946 into master at head b1fcf159378e7c72b67cc5ac507920d8390ef107 through the canonical merger workflow. After the merge, fast-forward the control checkout and restart all five MCP servers in one operator window before committing the PR #944 repair, since the deployed runtime executes the pre-fix code until both have happened.
NEXT_PROMPT:
```text
Merge Gitea-Tools PR #946 (Closes #945) in Scaled-Tech-Consulting/Gitea-Tools
on remote prgs.
Invoke the canonical gitea-workflow skill first. Use the gitea-merger namespace,
profile prgs-merger. Do not merge as the author jcwalker3 and do not merge as the
reviewer sysadmin.
PR #946 is approved at exact head b1fcf159378e7c72b67cc5ac507920d8390ef107 by
review posted from sysadmin / prgs-reviewer. Base is
aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218 and live master is
ed9414ebda9034ca87b36a9fce1c1ff7f98090f6, so the PR is behind but approved and
Gitea reports mergeable true. Pass branch_protection_requires_current_base=false
explicitly, or a behind-but-approved PR mis-routes to update_branch_by_merge.
Before merging, confirm the head is still b1fcf159378e7c72b67cc5ac507920d8390ef107
and that the approval sits at that exact head. If the head has advanced, stop and
hand back to a reviewer.
After the merge: fast-forward the control checkout to the resulting master
revision and restart all five MCP servers in one atomic operator window. A
restart before the checkout advance is a no-op that looks like success. Only then
recommission the duplicate-work gate and allow the PR #944 repair to be committed
through the ordinary sanctioned author path.
Preserve the uncommitted #943 repair in its worktree byte for byte, keep PR #942
cleanup paused, and leave issues #931 and #941 untouched.
```
WHAT_HAPPENED: An independent reviewer read live issue #945 and PR #946, formal review 623, every later comment, and author handoff comment 17663, pinned the review to head b1fcf159378e7c72b67cc5ac507920d8390ef107, and established a dedicated detached reviewer worktree at that head. The full production diff and both new test suites were read. Each of review 623's three findings was re-verified by measurement: the three enforcement call sites were reverted one at a time in disposable worktrees and each reverted site failed tests, with the commit recheck reproducing the exact duplicate_commit_prevented signature issue #945 was filed on; the F2 correction was checked against what the code actually authenticates and against whether the un-edited PR description creates real risk; and the F3 conflicting-evidence behaviour was probed directly through the real shared enforcement recheck across eight evidence shapes, with both halves of the fix separately mutation-tested. The authoritative exemption policy was confirmed byte-unchanged. The full suite was run at the head and at a clean checkout of the exact base and the failing test id sets were compared in both directions. The protected #943 worktree fingerprints were recomputed and matched. All disposable worktrees were removed and the reviewer worktree was proven pristine at the reviewed head.
WHY: Issue #945 exists because a correct decision layer was never wired into the paths that enforce it, and review 623 found this branch had reproduced that same class of defect inside its own test suite. The remediation had to be judged on whether a silent revert now fails, not on whether the wiring reads correctly, so every call site was reverted independently and observed. The F2 finding mattered because an overstated security claim is worse than a modest accurate one, and the corrected wording had to be checked against the implementation rather than accepted. F3 needed proof that ambiguity refuses rather than falls through, and proof that the legitimate same-PR case still continues, because a fix that narrows too far would deadlock the very author it exists to unblock.
ISSUE: #945
HEAD_SHA: b1fcf159378e7c72b67cc5ac507920d8390ef107
REVIEW_STATUS: APPROVE posted at b1fcf159378e7c72b67cc5ac507920d8390ef107 by sysadmin
MERGE_READY: yes
BLOCKERS: none
VALIDATION: Focused #945 wiring, #945 continuation, #755 and #760 suites at head b1fcf159: 137 passed, 19 subtests passed in 5.63s. The two new suites alone at head: 96 passed, 19 subtests passed. Full suite at head from a branches/ reviewer worktree: 28 failed, 5621 passed, 6 skipped, 1013 subtests in 176.61s. Full suite at a clean detached checkout of the exact base aab54d48: 28 failed, 5525 passed, 6 skipped, 994 subtests in 177.79s. Sorted failing test id sets compared in both directions are byte-identical, with no id present at head and absent at base, so this branch introduces no failure; the +96 passes and +19 subtests equal the two new suites measured alone. Independent revert probes at this head, each in a disposable worktree restored and removed afterwards: reverting the commit and create-PR recheck at gitea_mcp_server.py:2945 gives 8 failed, 92 passed, 4 subtests failed with outcome duplicate_commit_prevented and owning_pr_recovery_exempted False; reverting the read-only assessor at :5230 gives 2 failed, 94 passed; reverting the push ownership prover at :19515 gives 2 failed, 94 passed; all three restored gives 96 passed, 19 subtests. Disabling the F3 ambiguity guard gives 4 failed, 92 passed and disabling the F3 agreement check gives 5 failed, 91 passed. A direct probe of the real shared enforcement recheck across eight evidence shapes returned continuation for valid same-PR renewal, valid same-PR recovery and an agreeing pair, and fail-closed with no token and no authorized mutation for all five conflicting shapes including recovery and renewal naming different PRs. issue_work_duplicate_gate.py, issue_lock_recovery.py and issue_lock_store.py have a zero-byte diff across the branch. Scope is four files with no configuration, credential or scratch file and no network call in the new tests. Protected state verified intact: PR #944 open at f49e781102b9f363834c28c055f69639d16290c9 and the three SHA-256 fingerprints in the protected #943 worktree match the recorded values exactly. Reviewer worktree finished at b1fcf159378e7c72b67cc5ac507920d8390ef107 with an empty porcelain status including untracked files. No implementation change, no commit, no push, no branch mutation, no merge, and no PR edit was performed by this review.
LAST_UPDATED_BY: sysadmin / prgs-reviewer / gitea-reviewer namespace, reviewer lease session 73189-96f62ac29e47
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.
sysadmin
Closes #945
Diagnosis
gitea_lock_issuegrants the owning-PR duplicate-work waiver from two dispositions, and has since #760:Every later enforcement path re-derives that proof from the durable lock instead, because the live assessment ended when the lock call returned. Three call sites did so, and all three asked the same recovery-only rebuild:
gitea_mcp_server.py:2859_enforce_locked_issue_duplicate_recheck→gitea_commit_files,gitea_create_prgitea_mcp_server.py:5143gitea_assess_work_issue_duplicate(read-only assessor)gitea_mcp_server.py:19427issue_lock_recovery.recovered_owning_pr_from_lockreads exactly one key:But the two dispositions persist under different keys —
gitea_mcp_server.py:4336and:4344:So a plain exact-owner renewal wrote
lease_renewal, the gates looked fordead_session_recovery, and the waiver evaporated betweengitea_lock_issuereturning and the next author mutation:Refinement on the issue's stated root cause. #945 describes the defect as
_enforce_locked_issue_duplicate_recheckderiving its exemption only fromrecovered_owning_pr_from_lock. That is accurate but understates the blast radius: the same recovery-only rebuild was wired into three enforcement paths, not one, including the read-only assessor that reports the disposition back to the caller. Fixing only the commit recheck would have left the assessor and the push prover disagreeing with it. All three are corrected here.owning_pr_renewal_evidence's own docstring already named the missing half — "the mirror ofissue_lock_recovery.owning_pr_recovery_evidence(#755) for the renewal disposition". #760 built the mirror for the grant; nothing built it for the rebuild.Implementation
+128 / −7across two source files. No behaviour is removed.issue_lock_renewal.owning_pr_renewal_from_lock(new) — the renewal mirror ofrecovered_owning_pr_from_lock. Reads only the server-writtenlease_renewalblock, on a lock the caller must already own. Renewal has no descendant case, so it re-applies the equality the assessor required (pr_head == head_sha == remote_head_sha) and refuses anything else.gitea_mcp_server._owning_pr_continuation_from_lock(new, private) — resolves recovery first, then renewal: the same precedencegitea_lock_issueapplies, so the answer cannot drift between the gate that grants the waiver and the gates that enforce it.Deliberate hardening beyond a pure mirror. The recovery rebuild does not re-check the claimant; it relies on lock ownership alone. The renewal rebuild additionally requires the record's
identity/profileto still equal the claimant the lock names (falling back towork_lease.claimant). Renewal is already refused outright unless the lock records both —issue_lock_renewal.py:294-298— so a sanctioned record always carries them, and requiring agreement costs nothing while preventing a renewal block from being reused under an identity, profile, or workflow session the lock no longer names. This narrows the exemption; it never widens it.Security invariants preserved
Validation of the resulting token against live PR state is untouched and remains the sole responsibility of
issue_work_duplicate_gate._assess_owning_pr_exemption. This PR changes only which server-written block the token is rebuilt from, never what makes a token acceptable — so there is exactly one authoritative policy, as before.That policy continues to fail closed on: a different issue, a locked-branch mismatch, anything other than exactly one linked open PR, a different PR number, a PR head branch mismatch, and a live PR head matching neither the recorded nor the accepted head.
A live confirmation of the fail-closed side arrived during this task: reclaiming the lapsed #945 lease wrote a real
lease_renewalblock withpr_number: nullandpr_head_sha: null, because no PR existed yet. The new rebuild correctly yields no evidence from it — a renewal only produces an exemption when it actually names an owning PR.Tests
tests/test_issue_945_owning_pr_renewal_continuation.py— 49 tests, 8 subtests. Every fixture is an in-memory mapping; the suite creates no branch, worktree, lock file, lease, comment, or PR, and one test asserts the rebuild does not mutate its input.Coverage: granted rebuild and its exact token shape; 21 fail-closed cases (no lock, no renewal block, not granted, malformed block, local/remote/PR head divergence, missing heads, missing or malformed PR number, missing issue, unknown branch, identity/profile mismatch or absence, absent claimant, foreign-session evidence); resolver precedence including recovery-wins-over-renewal; all four phases agreeing; dead-session recovery still exempting; second PR, different PR, different branch, locked-branch mismatch, live head divergence, foreign issue, and sequential-task non-inheritance all refused; duplicate prevention retained; refusal and grant audit fields preserved.
The 2 that pass on base are
TestPreFixReproduction::test_recovery_only_rebuild_cannot_see_a_renewal_lockand::test_renewal_lock_produced_no_exemption_before_the_fix— they pin the defect itself, so they must pass on both sides. The other 47 fail on base withAttributeError, which is the wiring gap stated as an executable assertion.Failure classification, by test id rather than by count:
comm -23of the sorted failing-id sets is empty; the two sets are byte-identical.test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected, was run in isolation against the clean base checkout and fails identically there.The
+49passes and+8subtests over base are exactly this PR's new tests.Scope
gitea_mcp_server.py,issue_lock_renewal.py,tests/test_issue_945_owning_pr_renewal_continuation.py+593 / −7fix/issue-945-owning-pr-renewal-evidencemasterataab54d4825270f5a5c6f9c1abc1ab09eb4f3e21879334d48408fd446ddf1e8be332495960b847af6branches/issue-945-owning-pr-renewal-evidenceAuthor worktree provenance
gitea_bootstrap_author_issue_worktreeis still broken by #943, whose repair is the very thing #945 blocks from delivery, so it could not be used. Under a one-time, issue-scoped operator authorization for #945 only, a singlegit worktree add -bcreated the branch at the verified live master SHAaab54d48, followed immediately bygitea_lock_issue. The known-broken bootstrap capability was not called. Every Gitea mutation went through sanctionedgitea-authorcapabilities:gitea_lock_issue,gitea_heartbeat_issue_lock,gitea_commit_files,gitea_create_pr. Notea, nocurl, no raw API, no direct database access, no manual push.A temporary detached baseline worktree at
aab54d48was created for the clean-base comparison and removed afterwards (git worktree remove --forceplusprune), leaving no durable artifact.Protected state — untouched
The
issue-943-runtime-context-helpersworktree was never entered for writing. Its three uncommitted files are byte-for-byte identical before and after this work:PR #944 remains open at
f49e781102b9f363834c28c055f69639d16290c9; review622is undismissed. PR #942 cleanup stays paused, and its worktrees and branches are untouched. Issues #931 and #941 received nothing. The stable control checkout remains clean onmasterataab54d48. No stash was created; no MCP server was restarted or reconnected.Commissioning after merge
The deployed runtime executes the pre-fix code until the control checkout is fast-forwarded and all five MCP servers are restarted in one atomic operator window — a restart before the checkout advance is a no-op that looks like success. Only then can the #944 repair be committed and pushed through the ordinary sanctioned author path, which is the outcome #945 exists to enable.
Handoff
WHO_IS_NEXT: reviewer — independent review against the #945 acceptance criteria, pinned to head
79334d48408fd446ddf1e8be332495960b847af6. Do not self-review and do not self-merge. Preserve the uncommitted #943 repair and keep PR #942 cleanup paused.repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 93257-2e2eb56e1823
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr946-head
phase: claimed
candidate_head:
79334d4840target_branch: master
target_branch_sha:
aab54d4825last_activity: 2026-07-26T19:11:45Z
expires_at: 2026-07-26T19:21:45Z
blocker: none
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 93257-9ba6b15dd243
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr946-head
phase: claimed
candidate_head:
79334d4840target_branch: master
target_branch_sha:
aab54d4825last_activity: 2026-07-26T19:25:07Z
expires_at: 2026-07-26T19:35:07Z
blocker: none
REQUEST_CHANGES — PR #946 at head
79334d48408fd446ddf1e8be332495960b847af6Reviewed independently at base
aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218(livemaster, unmoved — the PR head's parent equals live master exactly, so no base drift affects this review). Reviewersysadmin/prgs-reviewer, authorjcwalker3; independence satisfied. Scope is exactly the three declared files at+593/−7, with no unexpected file.The diagnosis is correct and sharper than the issue's own. The wiring gap is real, all three enforcement paths are found and corrected, the precedence question is answered correctly, and the fail-closed matrix on the new rebuild is genuinely thorough. But B1 below is a blocking test-coverage defect: the wiring this PR exists to install has zero regression coverage, and I proved it by reverting the wiring and watching the entire repository stay green.
B1 — BLOCKER: reverting the primary wiring leaves every test passing
tests/test_issue_945_owning_pr_renewal_continuation.py(whole file); wiring atgitea_mcp_server.py:2894,:5179,:19464.The suite exercises
issue_lock_renewal.owning_pr_renewal_from_lock,gitea_mcp_server._owning_pr_continuation_from_lock, andissue_work_duplicate_gate.assess_work_issue_duplicate_gatedirectly. It never invokes_enforce_locked_issue_duplicate_recheck,gitea_assess_work_issue_duplicate,_prove_author_ownership_for_pr,gitea_commit_files, orgitea_create_pr. Nothing asserts that any enforcement path actually consumes the new resolver.Proven, not inferred. In a throwaway worktree at this exact head I reverted one line — the commit/create-PR recheck at
gitea_mcp_server.py:2894— back to the pre-#945 recovery-only rebuild:That reintroduces exactly the defect #945 exists to fix. Results:
Not one test in the repository detects it. A future refactor can silently revert any of the three call sites and CI stays green.
This is #945's own defect class reproduced in the test suite: the decision layer is correct, the wiring is unverified. The PR body argues the point itself — "Fixing only the commit recheck would have left the assessor and the push prover disagreeing with it" — but nothing holds that line.
The
47 failed / 2 passedbase result does not cover this. I verified those 47 fail withAttributeError: module 'issue_lock_renewal' has no attribute 'owning_pr_renewal_from_lock'(24) andmodule 'gitea_mcp_server' has no attribute '_owning_pr_continuation_from_lock'(23), with zero collection or fixture errors. That proves the two functions are new. It says nothing about who calls them.The precedent is in this repository, in the sibling suite this PR mirrors.
tests/test_issue_755_owning_pr_recovery.pyhas 12mcp_server.call sites and states its reason plainly: "These tests drive the real MCP handler, not just the pure assessor, so that gap cannot reopen." #755 met that bar for recovery; #945 should meet it for renewal.Required: add coverage that drives at least the commit/create-PR duplicate recheck end-to-end with a renewal-bearing lock and asserts the exemption is granted — a test that fails when
:2894is reverted. Ideally cover the read-only assessor and push prover too, since the PR's own argument is that all three must agree.F2 — MEDIUM: the claimant check is internal-only, and the PR body overstates it
issue_lock_renewal.py(newowning_pr_renewal_from_lock, claimant block).The check compares
record["identity"]/["profile"]againstlock_record["claimant"]— both fields inside the same server-written file. It is not bound to the authenticated caller or the owning task session. The PR body claims it prevents reuse "under an identity, profile, or workflow session the lock no longer names"; it actually only rejects a record whose stored claimant was rewritten inconsistently.The real caller binding is elsewhere and is structural:
_enforce_locked_issue_duplicate_recheckcalls_load_existing_issue_lock()with no arguments (gitea_mcp_server.py:2872), which resolvesissue_lock_store.read_session_issue_lock()→session-{os.getpid()}.json(issue_lock_store.py:85). Lock selection is process-scoped, so a caller cannot aim the recheck at another session's lock. That is what actually prevents cross-session reuse.Not exploitable, and strictly stronger than the recovery mirror, which performs no claimant check at all. Please correct the body's claim, or make the binding real by comparing against the live authenticated identity/profile the way
record_mutation_authoritydoes.Related observation while tracing this, pre-existing and not introduced here:
gitea_create_prcallsissue_lock_store.verify_lock_for_mutationatgitea_mcp_server.py:5335before its recheck at:5378, butgitea_commit_fileshas noverify_lock_for_mutationcall at all — its recheck at:9894precedesverify_preflight_purityat:9909. Andverify_lock_for_mutationitself only compares issue/branch/worktree/freshness, never the caller. Worth a follow-up issue; out of scope here.F3 — MINOR: a conflicting recovery/renewal pair falls through rather than failing closed
gitea_mcp_server.py(_owning_pr_continuation_from_lock).Probed directly at this head:
The third case is the one worth naming: a recovery block naming a different PR exists and fails validation, and the helper still returns renewal evidence rather than refusing.
Not a blocker, because it is unreachable through the sanctioned writer.
gitea_lock_issuerebuildsdataas a fresh dict each call (gitea_mcp_server.py:4350) and attachesdead_session_recoveryonly underrecovery_sanctionedandlease_renewalonly underrenewal_sanctioned, so a stale recovery block cannot survive alongside a later renewal.tests/test_issue_760_mcp_renewal_path.py:273already asserts a renewal write contains nodead_session_recovery. Live-PR validation backstops it regardless. Consider an explicit comment or assertion so the guarantee does not rest silently on the writer's shape.What is correct — for the record
gitea_lock_issueappliesrecovery if recovery_sanctioned else renewal(:4258-4268); the resolver applies the same order, verified by probe. The stated design goal is met.recovered_owning_pr_from_lockin production code is inside the resolver itself (gitea_mcp_server.py:2808). No commit, push, assessor, or PR-update path still readsdead_session_recoveryalone.issue_work_duplicate_gate._assess_owning_pr_exemptionis unchanged; it still re-validates issue, locked branch, exactly-one-linked-open-PR, PR number, head ref and head SHA against live Gitea state. The patch changes only which server-written block the token is rebuilt from. An open PR alone still grants nothing.None. The equality re-check (pr_head == head_sha == remote_head_sha) genuinely re-derives what the assessor required.AttributeErrorat call time, 0 collection/fixture errors, and the 2 base-passing tests are the defect-pinning reproductions that must pass on both sides.branches/:The single targeted failure,
test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected, reproduces on base in isolation. The+49passes and+8subtests are exactly this PR's new tests. The suite creates no durable branch, worktree, lock, lease, comment or PR.Canonical PR State
STATE: PR #946 is open at head
79334d4840and has received one formal REQUEST_CHANGES review from sysadmin at that exact head. One blocking finding (B1) plus one medium (F2) and one minor (F3) are open. The branch introduces no test regression against baseaab54d4825.WHO_IS_NEXT: author
NEXT_ACTION: Author jcwalker3 must add regression coverage that drives the real enforcement paths with a renewal-bearing lock so that reverting any of gitea_mcp_server.py:2894, :5179, or :19464 fails a test (B1), correct or strengthen the caller-binding claim in the PR body (F2), optionally make the conflicting-evidence guarantee explicit (F3), push the result, and publish a new head-pinned handoff for a fresh independent review.
NEXT_PROMPT:
WHAT_HAPPENED: An independent review at the exact head read all three changed files, traced the evidence flow from owning_pr_renewal_evidence through the new rebuild, the shared resolver, and all three enforcement paths into issue_work_duplicate_gate._assess_owning_pr_exemption, and probed precedence and fall-through behaviour directly against the patched modules. The wiring, precedence and fail-closed matrix are correct. Reverting the primary wiring in a throwaway worktree at the same head left the new suite at 49 passed and the full suite byte identical to the PR's own result, proving no test protects the fix. Targeted and full suites were run at head and at a clean base worktree; failing test id sets are identical.
WHY: #945 exists because a correct decision layer was never wired into the paths that enforce it. This PR wires it correctly but ships no test that fails if the wiring is removed, so the same class of defect can silently return. The sibling recovery suite for #755 already drives the real MCP handler for exactly this reason.
ISSUE: #945
HEAD_SHA:
79334d4840REVIEW_STATUS: REQUEST_CHANGES posted at
79334d4840by sysadminMERGE_READY: no
BLOCKERS: test coverage blocker
VALIDATION: New #945 suite at head: 49 passed, 8 subtests. Targeted 24-file sweep at head: 1 failed, 514 passed, 32 subtests in 32.49s; at clean base worktree
aab54d48: 1 failed, 465 passed, 24 subtests in 32.13s; the single failure test_pidless_durable_lock_rejected reproduces on base in isolation. Full suite at head: 28 failed, 5574 passed, 6 skipped, 1002 subtests in 149.90s. Full suite at base: 28 failed, 5525 passed, 6 skipped, 994 subtests in 148.91s. Failing test id sets identical, so no regression originates from this branch. Wiring-revert probe at the same head: new suite 49 passed, full suite 28 failed/5574 passed with an identical failing id set, demonstrating the absent coverage. Base reproduction of the new suite fails with AttributeError at call time on both new symbols, 0 collection or fixture errors.LAST_UPDATED_BY: sysadmin / prgs-reviewer / gitea-reviewer namespace, reviewer lease session 93257-9ba6b15dd243
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 93257-9ba6b15dd243
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr946-head
phase: released
candidate_head:
79334d4840target_branch: master
target_branch_sha:
aab54d4825last_activity: 2026-07-26T19:30:14Z
expires_at: 2026-07-26T19:40:14Z
blocker: manual-release
[THREAD STATE LEDGER] PR #946 — review 623 remediation pushed; head advanced to
b1fcf159Server-side mutation ledger:
fix/issue-945-owning-pr-renewal-evidenceadvanced79334d4..b1fcf15; PR #946 head is nowb1fcf159378e7c72b67cc5ac507920d8390ef107Blockers:
gitea_edit_prexposes noworktree_pathparameter and resolves its workspace from the MCP process root, so the PR description update fails closed on the #618 control-checkout wall. The correction it was to carry is reproduced in full below.What is true now:
b1fcf1593779334d48, failing test id sets byte-identicalWhat changed:
b1fcf159published, addressing B1, F2 and F3 from review 623What is blocked:
Who/what acts next:
b1fcf159378e7c72b67cc5ac507920d8390ef107b1fcf159Review 623 findings → fixes and evidence
B1 (BLOCKER) — reverting the wiring left every test passing
Fix: new
tests/test_issue_945_enforcement_path_wiring.py(685 lines). It drives the real handlers rather than the pure assessor —mcp_server._enforce_locked_issue_duplicate_recheck(the shared recheck behindgitea_commit_filesandgitea_create_pr),mcp_server.gitea_assess_work_issue_duplicate, andmcp_server._prove_author_ownership_for_pr— against a renewal-bearing lock, and asserts each grants the exemption. This follows the precedent review 623 cited,tests/test_issue_755_owning_pr_recovery.py.Evidence — revert probes at head
b1fcf159, each in a throwaway worktree removed afterwards:All three call sites are now individually protected. The exact revert review 623 performed — the commit/create-PR recheck — now fails 8 tests and 4 subtests where previously the whole repository stayed green.
The suite also keeps the fail-closed matrix on the real paths: an open PR alone, a second PR, a different PR, branch, issue or head, identity and profile mismatch, ungranted and malformed renewal blocks, stale recorded heads, local/remote head divergence, and sequential-task non-inheritance are all still refused. One test asserts the runs leave no durable artifact outside a temporary lock directory.
F2 (MEDIUM) — the claimant check was overstated
Accepted; the claim was wrong and is corrected.
lease_renewal.identity/profileagainst the claimant recorded on the same lock file. Both sides are server-written fields of one document, so it is an internal-consistency check within a server-written lock record. It rejects a lock whose renewal block and claimant disagree._load_existing_issue_lock()with no issue coordinates, which resolvesissue_lock_store.read_session_issue_lock()→ the session pointer atsession-{os.getpid()}.json. Lock selection is scoped to the operating-system process, so a caller cannot aim the recheck at a lock some other process bound.Evidence: the corrected wording is in the code, not only in prose —
issue_lock_renewal.owning_pr_renewal_from_lock's docstring and the inline comment above the claimant block both now say this.TestCallerBindingIsStructuralNotFieldComparisonin the new suite pins the behaviour:test_lock_selection_is_keyed_to_the_operating_system_process,test_a_lock_bound_by_another_process_is_not_reachable,test_claimant_check_does_not_consult_the_live_authenticated_caller, andtest_internal_disagreement_is_what_the_check_actually_rejects.The pre-existing observation in review 623 — that
gitea_commit_fileshas noverify_lock_for_mutationcall whilegitea_create_prdoes, and thatverify_lock_for_mutationnever compares the caller — is untouched here and merits its own issue.F3 (MINOR) — conflicting recovery/renewal pair fell through
Fix:
_owning_pr_continuation_from_locknow treats present-but-unusable recovery evidence as ambiguous rather than absent and fails closed, so a recovery record naming one PR can no longer be bypassed by renewal evidence naming another.One correction to the finding's premise. Review 623 judged a sanctioned pair unreachable because
gitea_lock_issuerebuildsdataper call. Tracing it further: recovery is assessed whenever the lease is not live and requires the recorded PID to be dead, while renewal is assessed whenever the lease has expired — itself one way to be non-live — and deliberately does not branch on PID liveness (#760 AC16). An expired lease whose recorded owner has also died satisfies both, and both blocks are written into that same freshly built dict. A sanctioned pair is therefore a reachable, legitimate state. Because it derives from one live observation in one call it always describes the same issue, PR, branch and heads, so when both blocks rebuild they must now agree on every one of those bindings or no continuation authority is returned. Recovery-only and renewal-only locks keep their existing behaviour exactly.Evidence:
_CONTINUATION_EVIDENCE_BINDINGSand_continuation_evidence_agreesingitea_mcp_server.py, with the reasoning recorded in the resolver docstring rather than left implicit in the writer's shape.PR description correction (published here because the description edit fails closed)
The F2 text above is what was to replace the "Deliberate hardening beyond a pure mirror" paragraph in the PR description. Because
gitea_edit_pris unavailable to this workspace binding, it is published here instead and this comment is the authoritative record. The PR description's claim that the claimant check prevents reuse "under an identity, profile, or workflow session the lock no longer names" is superseded by the F2 section of this comment.Canonical Issue State
STATE: Issue #945 is open with
status:pr-open. Its owning PR #946 is open at headb1fcf159378e7c72b67cc5ac507920d8390ef107, carrying two commits:79334d48(original fix) andb1fcf159(review 623 remediation). Review 623 REQUEST_CHANGES from sysadmin is undismissed and still gates merge, and is now stale because the head advanced past the reviewed SHA79334d48. B1, F2 and F3 have been addressed and verified locally, but no independent review has yet evaluated the new head.WHO_IS_NEXT: reviewer
NEXT_ACTION: An independent reviewer — not jcwalker3 — must review PR #946 pinned to head
b1fcf159378e7c72b67cc5ac507920d8390ef107against the #945 acceptance criteria, confirming that the B1 regression coverage genuinely fails when any of the three enforcement call sites is reverted, that the F2 correction accurately describes the claimant check as internal consistency rather than authenticated-caller verification, and that F3 fails closed on conflicting evidence.NEXT_PROMPT:
WHAT_HAPPENED: The B1, F2 and F3 remediation for review 623 existed only as uncommitted changes in the
issue-945-owning-pr-renewal-evidenceworktree, because the deployed #945 defect refuses the sanctioned author commit path for this very issue. Under a one-time operator break-glass authorization, every pin was verified read-only first — local HEAD, remote branch head and live PR head all equal to 79334d48; review 623 confirmed the current undismissed REQUEST_CHANGES; zero active control-plane leases; the sole issue lock owned by jcwalker3 / prgs-author with a dead PID 23400; and the four file hashes recomputed and matched against the fingerprints the lock itself recorded. The four files were staged explicitly, committed asb1fcf159, and pushed as a fast-forward to the existing branch. PR #946 was then natively confirmed to sit at exactly that commit. The PR description edit failed closed on a workspace-binding wall, so the F2 correction is published in this comment instead.WHY: #945 exists because a correct decision layer was never wired into the paths that enforce it, and review 623 showed this branch had reproduced that same defect class in its own test suite — the wiring was correct but nothing failed when it was removed. The new suite drives the real enforcement handlers so a silent revert of any of the three call sites now fails a test. F2 mattered because a security claim in the PR description overstated what the claimant comparison proves; an inaccurate security claim is worse than a modest one, so it is corrected and the real structural binding is described with its limits. F3 closed a fall-through where ambiguous recovery evidence could be bypassed rather than refused.
RELATED_PRS: #946 (owning PR, open, head
b1fcf15937); #944 (open atf49e781102, review 622 undismissed, waiting on this issue); #942 (cleanup paused)BLOCKERS: awaiting fresh independent review at the new head; review 623 undismissed. MERGE_READY: no.
VALIDATION: Focused #945/#755/#760 suites at
b1fcf159: 137 passed, 19 subtests passed. Full suite from abranches/worktree atb1fcf159: 30 failed, 5619 passed, 6 skipped, 1013 subtests in 175.50s. Full suite at a clean checkout of79334d48: 30 failed, 5572 passed, 6 skipped, 1002 subtests in 177.76s. Failing test id sets byte-identical in both directions, so no failure originates from this commit; the +47 passes and +11 subtests are exactly the new coverage. Earlier runs on this PR reported 28 failures rather than 30 — the two extra are environmental drift present identically on both sides, which is why the comparison is made on failing id sets rather than counts. Break-glass necessity was verified read-only:gitea_assess_work_issue_duplicate(issue 945, phase=commit)returnedoutcome: duplicate_commit_prevented,owning_pr_recovery_exempted: false,owning_pr_recovery_notes: [],reasons: ["open PR #946 already covers issue #945 (fail closed)"], and the durable lock carries alease_renewalblock withpr_number: nullandpr_head_sha: nulland nodead_session_recoveryblock, so no owning-PR waiver is rebuildable from it even by the corrected code. Exactly one commit was created and pushed as a fast-forward: no force-push, no new branch, no new issue, no new PR, no review, no approval, no merge, and no change tomaster. Protected state is intact — theissue-943-runtime-context-helpersworktree was never entered for writing, PR #944 remains open atf49e781with review 622 undismissed, PR #942 cleanup stays paused, issues #931 and #941 received nothing, and the control checkout remains clean onmasterated9414eb.LAST_UPDATED_BY: jcwalker3 / prgs-author / gitea-author namespace
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 73189-96f62ac29e47
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr946-b1fcf159
phase: claimed
candidate_head:
b1fcf15937target_branch: master
target_branch_sha:
ed9414ebdalast_activity: 2026-07-27T22:49:31Z
expires_at: 2026-07-27T22:59:31Z
blocker: none
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 73189-96f62ac29e47
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr946-b1fcf159
phase: submitting
candidate_head:
b1fcf15937target_branch: master
target_branch_sha:
ed9414ebdalast_activity: 2026-07-27T23:15:48Z
expires_at: 2026-07-27T23:25:48Z
blocker: none
APPROVE — PR #946 at head
b1fcf159378e7c72b67cc5ac507920d8390ef107Fresh independent review at the exact current head, base
aab54d4825270f5a5c6f9c1abc1ab09eb4f3e218(the merge-base with livemastered9414ebda9034ca87b36a9fce1c1ff7f98090f6). Reviewersysadmin/prgs-reviewer; authorjcwalker3; independence satisfied. Head's parent is exactly79334d48, so the advance was a fast-forward with no history rewrite. Scope is exactly four files,+1493 / −7, with no configuration, credential, or scratch file and no network call in the new tests.Review
623is superseded on the merits. All three of its findings are resolved, and I verified each by my own measurement rather than by accepting the author's account.B1 — RESOLVED. The wiring is now genuinely mutation-sensitive at all three call sites
tests/test_issue_945_enforcement_path_wiring.pyis new (685 lines) and drives the real production entry points:_enforce_locked_issue_duplicate_recheck(the shared recheck behindgitea_commit_filesandgitea_create_pr),gitea_assess_work_issue_duplicate, and_prove_author_ownership_for_pr. Each test writes a real durable lock into a temporary directory and binds it through the ordinary session pointer, so_load_existing_issue_lock()resolves it the way the server does. Only the external Gitea read boundary and the credential header are substituted; the lock load, evidence rebuild, resolver precedence andissue_work_duplicate_gatechain all execute for real.The coverage is not tautological.
TestRenewalReachesEnforcementPathsuses a renewal-only lock carrying nodead_session_recoveryblock at all, and substitutes nothing — so it fails on real behaviour the moment a call site stops consuming the resolver.I re-ran the revert probe myself, one call site at a time, in a throwaway detached worktree at this exact head, and reproduced the author's figures exactly:
The failures are for the right reason, not an incidental assertion. Reverting
:2945produces precisely the signature issue #945 was filed on:The exact revert that left the whole repository green under review
623now fails 8 tests and 4 subtests. All three call sites are individually protected, and the fail-closed matrix is preserved on the real paths, not only on the pure assessor: an open PR alone, a second PR, a different PR, branch, issue or head, identity and profile mismatch, ungranted and malformed renewal blocks, stale recorded heads, local/remote divergence, and sequential-task non-inheritance are all still refused there.I also confirmed by search that the only remaining production call to the recovery-only rebuild is inside the resolver itself at
gitea_mcp_server.py:2853.F2 — RESOLVED. The stated guarantee now matches what the code actually does
I read what the implementation authenticates rather than what it claims. The corrected description is accurate on all three points the finding required:
lease_renewal.identity/profileagainst the claimant recorded on the same document. It rejects a lock whose two halves disagree.TestCallerBindingIsStructuralNotFieldComparison::test_claimant_check_does_not_consult_the_live_authenticated_callerpins that limitation in executable form: a renewal block agreeing with an unrelated recorded claimant still rebuilds, regardless of who is authenticated.read_session_issue_lock()tosession-{os.getpid()}.json, and the docstring states plainly that this is per-process rather than per-authenticated-user, says nothing about a lock reached by explicit issue coordinates, and says nothing about two roles sharing one process. Live identity and profile remain enforced by the separate mutation-authority and profile gates.The correction lives in the code — both the function docstring and the inline comment above the claimant block — so an engineer reading the implementation gets the accurate statement, not the superseded one.
On the un-edited PR description, judged independently. The description still carries the superseded sentence, because
gitea_edit_prexposes no worktree parameter and fails closed on the #618 control-checkout wall. I did not treat that tool failure as either automatic grounds for approval or for rejection; I assessed the residual risk directly. It is not material, for three reasons I verified:17663supersedes it by explicit quotation, naming the exact sentence and stating the correct guarantee in its place, in the same thread and directly below it.ed9414eb— and nothing generates release notes or documentation from PR bodies.An inaccurate security claim is worth correcting, so the description should be updated whenever the tooling permits. It is not a release-documentation or security risk in its present state, and it does not block merge.
F3 — RESOLVED, and the finding's own premise was corrected correctly
_owning_pr_continuation_from_locknow treats present-but-unusable recovery evidence as ambiguous rather than absent, and refuses a rebuilt pair that disagrees on any of issue, PR, branch, or any head.The author's correction to review
623is factually right, and I verified it at the writer rather than taking it on trust. Recovery is assessed whenever the lease is not live (gitea_mcp_server.py:4236); renewal is assessed whenever the lease has expired (:4269); an expired lease is one way to be non-live, and both blocks are written into the same freshly builtdata. A sanctioned pair is therefore reachable, and review623's claim that it was not was wrong. The agreement requirement is the right response to that.I probed the conflicting-evidence behaviour independently, driving the real shared enforcement recheck:
Every conflicting shape yields
block: True,performed: False,owning_pr_recovery_exempted: False, andsafe_next_action: stop before mutating; do not commit or push duplicate work. No exemption is granted, no mutation is authorized, and rejected recovery evidence never falls through to renewal evidence naming another PR. Valid same-PR continuation still works, and the agreeing pair still continues, so the new requirement narrows without breaking the legitimate case.Both halves of the F3 fix are themselves mutation-covered — I disabled each in a throwaway copy:
Regression, authorization and scope
The authoritative policy is untouched:
issue_work_duplicate_gate.py,issue_lock_recovery.pyandissue_lock_store.pycarry a zero-byte diff acrossaab54d48..b1fcf159._assess_owning_pr_exemptionstill re-validates every binding against live Gitea state — token issue equals the assessed issue, locked branch equals the token branch, exactly one linked open PR, PR number equals the token PR, PR head ref equals the token branch, and the live head SHA is one of the recorded or accepted heads. An open PR alone still grants nothing.No authorization regression: the enforcement recheck derives the issue number, the locked branch and the continuation evidence from one and the same lock document, so evidence and enforcement target cannot diverge. The push prover still requires the rebuilt token's PR number to equal the PR being proved. No session or worktree identity confusion: lock selection remains process-scoped and unchanged by this patch. No fail-open path: every new branch in the rebuild returns
None, and the only widening this patch performs is to recognise a second server-written block the server itself already treats as sanctioned. Duplicate-work protection is intact, proven on the real paths. Compatibility is preserved — a lock with nolease_renewalblock rebuilds exactly as before, and a recovery-only lock is unaffected.Nothing is outside issue #945's scope, and the protected state is genuinely intact: the #943 repair worktree is still at
f49e781with the same three modified files, and I recomputed all three SHA-256 fingerprints and they match the values recorded in the PR description byte-for-byte. PR #944 remains open atf49e781.Non-blocking observations
None of these gates merge. Recording them for a follow-up.
O1 (low) — the ambiguity guard keys on
dict, so a non-dict recovery block still falls through.recovery_present = isinstance(lock_record.get("dead_session_recovery"), dict)atgitea_mcp_server.py:2852. Adead_session_recoverykey present as a list, string or integer is therefore treated as absent, and renewal evidence naming a different PR is returned — the same fall-through F3 closes for dict-shaped blocks. Measured:Not exploitable and not a blocker: the sanctioned writer never produces a non-dict block, and live-state validation independently requires the named PR to be the single linked open PR on the matching branch at the matching head, so no foreign PR can actually be exempted. The fix is one word — test
is not Nonerather than the type.O2 (low) — the refusal diagnostic for conflicting evidence is generic. A lock carrying conflicting continuation evidence produces the ordinary duplicate-work refusal, with
owning_pr_recovery_notes: []and no indication that ambiguity, rather than genuine duplicate work, caused it. The structured refusal fields the acceptance criteria require are all preserved, and the outcome is safe; a note naming the ambiguity would materially shorten diagnosis.O3 (informational) — commissioning note. A lock already carrying both blocks with disagreeing values, written by the pre-fix server, now yields no continuation where it previously yielded recovery. That is a fail-closed narrowing in the correct direction and is recoverable by re-locking, but it is worth knowing during the post-merge restart window.
O4 (pre-existing, out of scope). Review
623noted thatgitea_create_prcallsverify_lock_for_mutationwhilegitea_commit_filesdoes not, and thatverify_lock_for_mutationnever compares the caller. Still true, untouched by this PR, and deserving its own issue.Tests I ran
All runs from reviewer-owned worktrees under
branches/, using the project virtual environment.The
+96and+19are exactly the two new suites measured alone, so every additional pass is accounted for by new coverage and nothing else moved.I did not reproduce the author's reported counts exactly and did not need to. Their clean comparison was taken against
79334d48, the previous head, whereas I compared againstaab54d48, the true merge base, which legitimately yields different totals. Their30failures against my28is the environmental drift they themselves flagged, present identically on both sides of my comparison. The claim that matters — that this branch introduces no failure — I validated by the stronger method, comparing sorted failing test ids in both directions, and it holds.The three revert probes, the two F3 mutation probes and the conflicting-evidence probe were all performed in disposable detached worktrees at this head, restored to pristine after each mutation and removed afterwards. The final reviewer worktree is at
b1fcf159378e7c72b67cc5ac507920d8390ef107with an emptygit status --porcelain --untracked-files=all.Issue #945 acceptance criteria
Continuation evidence is centralized in one resolver consumed by every relevant enforcement path; the shared recheck recognises valid exact-owner renewal evidence in addition to sanctioned dead-session recovery; the exemption stays bound to repository, issue, PR, branch, recorded, accepted, local, remote and live heads, with identity and profile bound as internal consistency plus process-scoped lock selection, honestly documented; valid exact-owner renewal permits the intended update of the existing owning PR and authorizes no second PR or unrelated commit; missing, ambiguous, wrong-issue, wrong-PR, wrong-branch, wrong-identity, wrong-profile, head-divergent and ungranted evidence all fail closed; duplicate prevention remains enforced; commit, create-PR, push and the read-only assessor use one authoritative decision; structured refusals preserve reason codes, retryability and audit evidence; and regression tests cover both renewal and recovery, reproduce the pre-fix failure, and prove no exemption arises solely because an open PR exists. The remaining criterion — restart the fleet at the resulting master revision and recommission the gate — is a post-merge commissioning step.
NATIVE_REVIEW_PROOF: This review was conducted end to end through the native
gitea-reviewerMCP namespace on remoteprgs, profileprgs-reviewer, authenticated identitysysadmin, rolereviewer, bound toScaled-Tech-Consulting/Gitea-Tools. Native preflight reportedin_parity: true,live_stale: false,mutation_safe: true,restart_required: falseated9414ebda9034ca87b36a9fce1c1ff7f98090f6, withnative_mcp_transport: true,production_native_mcp_transport: true,mode: production,transport: stdio. Live PR and issue state, formal review623, the comment thread, the head SHA and this verdict were all read and written through native tool calls under reviewer lease session73189-96f62ac29e47. Capability was resolved natively before each reviewer mutation, and the canonical review-merge workflow was loaded natively at hash263d0a6cb8a6.Canonical PR State
STATE: PR #946 is open at head
b1fcf15937and is mergeable against master ated9414ebda. It carries two commits above baseaab54d4825and touches exactly four files. This APPROVE is pinned to that exact head. The earlier REQUEST_CHANGES review 623 was posted at79334d4840, is marked stale because the head advanced past it, and all three of its findings are verified resolved at the current head. No blocking finding remains; four non-blocking observations are recorded above. The branch introduces no test failure against a clean checkout of its base.WHO_IS_NEXT: merger
NEXT_ACTION: A separate merger, not the author and not this reviewer, may merge PR #946 into master at head
b1fcf15937through the canonical merger workflow. After the merge, fast-forward the control checkout and restart all five MCP servers in one operator window before committing the PR #944 repair, since the deployed runtime executes the pre-fix code until both have happened.NEXT_PROMPT:
WHAT_HAPPENED: An independent reviewer read live issue #945 and PR #946, formal review 623, every later comment, and author handoff comment 17663, pinned the review to head
b1fcf15937, and established a dedicated detached reviewer worktree at that head. The full production diff and both new test suites were read. Each of review 623's three findings was re-verified by measurement: the three enforcement call sites were reverted one at a time in disposable worktrees and each reverted site failed tests, with the commit recheck reproducing the exact duplicate_commit_prevented signature issue #945 was filed on; the F2 correction was checked against what the code actually authenticates and against whether the un-edited PR description creates real risk; and the F3 conflicting-evidence behaviour was probed directly through the real shared enforcement recheck across eight evidence shapes, with both halves of the fix separately mutation-tested. The authoritative exemption policy was confirmed byte-unchanged. The full suite was run at the head and at a clean checkout of the exact base and the failing test id sets were compared in both directions. The protected #943 worktree fingerprints were recomputed and matched. All disposable worktrees were removed and the reviewer worktree was proven pristine at the reviewed head.WHY: Issue #945 exists because a correct decision layer was never wired into the paths that enforce it, and review 623 found this branch had reproduced that same class of defect inside its own test suite. The remediation had to be judged on whether a silent revert now fails, not on whether the wiring reads correctly, so every call site was reverted independently and observed. The F2 finding mattered because an overstated security claim is worse than a modest accurate one, and the corrected wording had to be checked against the implementation rather than accepted. F3 needed proof that ambiguity refuses rather than falls through, and proof that the legitimate same-PR case still continues, because a fix that narrows too far would deadlock the very author it exists to unblock.
ISSUE: #945
HEAD_SHA:
b1fcf15937REVIEW_STATUS: APPROVE posted at
b1fcf15937by sysadminMERGE_READY: yes
BLOCKERS: none
VALIDATION: Focused #945 wiring, #945 continuation, #755 and #760 suites at head
b1fcf159: 137 passed, 19 subtests passed in 5.63s. The two new suites alone at head: 96 passed, 19 subtests passed. Full suite at head from a branches/ reviewer worktree: 28 failed, 5621 passed, 6 skipped, 1013 subtests in 176.61s. Full suite at a clean detached checkout of the exact baseaab54d48: 28 failed, 5525 passed, 6 skipped, 994 subtests in 177.79s. Sorted failing test id sets compared in both directions are byte-identical, with no id present at head and absent at base, so this branch introduces no failure; the +96 passes and +19 subtests equal the two new suites measured alone. Independent revert probes at this head, each in a disposable worktree restored and removed afterwards: reverting the commit and create-PR recheck at gitea_mcp_server.py:2945 gives 8 failed, 92 passed, 4 subtests failed with outcome duplicate_commit_prevented and owning_pr_recovery_exempted False; reverting the read-only assessor at :5230 gives 2 failed, 94 passed; reverting the push ownership prover at :19515 gives 2 failed, 94 passed; all three restored gives 96 passed, 19 subtests. Disabling the F3 ambiguity guard gives 4 failed, 92 passed and disabling the F3 agreement check gives 5 failed, 91 passed. A direct probe of the real shared enforcement recheck across eight evidence shapes returned continuation for valid same-PR renewal, valid same-PR recovery and an agreeing pair, and fail-closed with no token and no authorized mutation for all five conflicting shapes including recovery and renewal naming different PRs. issue_work_duplicate_gate.py, issue_lock_recovery.py and issue_lock_store.py have a zero-byte diff across the branch. Scope is four files with no configuration, credential or scratch file and no network call in the new tests. Protected state verified intact: PR #944 open atf49e781102and the three SHA-256 fingerprints in the protected #943 worktree match the recorded values exactly. Reviewer worktree finished atb1fcf15937with an empty porcelain status including untracked files. No implementation change, no commit, no push, no branch mutation, no merge, and no PR edit was performed by this review.LAST_UPDATED_BY: sysadmin / prgs-reviewer / gitea-reviewer namespace, reviewer lease session 73189-96f62ac29e47
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 73189-96f62ac29e47
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr946-b1fcf159
phase: released
candidate_head:
b1fcf15937target_branch: master
target_branch_sha:
ed9414ebdalast_activity: 2026-07-27T23:17:57Z
expires_at: 2026-07-27T23:27:57Z
blocker: manual-release
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #946
issue: #945
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 54461-14cd5ebc7ccf
worktree: /Users/jasonwalker/Development/Gitea-Tools
phase: claimed
candidate_head:
b1fcf15937target_branch: master
target_branch_sha:
ed9414ebdalast_activity: 2026-07-27T23:27:06Z
expires_at: 2026-07-27T23:37:06Z
blocker: none
Stale #332 review-decision lock cleanup (#594)
Status: APPLIED
sysadminprgs-merger2026-07-27T23:27:40.943462+00:00approveon PR fix(gate): preserve exact-owner renewal evidence across duplicate rechecks (Closes #945) (#946)closed(merged=True)35ed8a2fcb11134a37c862ca6eaca26e3028902a9prgs-reviewerManual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.