fix(author): unify the bootstrap and lock_issue issue-lock contract (Closes #953) #954

2026-07-27T23:38:10-05:00

jcwalker3 commented

2026-07-27 23:38:10 -05:00

Closes #953

Head: cdf0daefa91f891186bfad0513816ec2ddc90e7c
Base: master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d

The defect

gitea_bootstrap_author_issue_worktree reported a lock as created, wrote a shape no downstream author operation accepts, and then directed the author straight to implementation. Once the branch carried commits, every sanctioned path refused at once: heartbeat (claimant read only from work_lease.claimant), re-lock (the branch had advanced past its base revision), #760 exact-owner renewal (a lock with no recorded expiry is never expired), and the #447 create-PR provenance guard.

Each gate is individually correct. The defect was that two writers disagreed about what a lock is.

Verified against the pinned base rather than the issue's provisional line numbers:

Location	Finding
`author_issue_bootstrap.py:1203-1220`	claimant at the lock top level, `lease_id: null`, no `work_lease`, `lock_provenance`, or `expires_at`
`gitea_mcp_server.py:4683-4701`	the only canonical writer
`issue_lock_store.py:302-308`	ownership check read the claimant only from `work_lease`
`issue_lock_store.py:690-703`	expiry read only from `work_lease.expires_at`, so absent read as "not expired"
`issue_lock_store.py:858-879`	a lock with no `work_lease` classified legacy, no expiry, no pid, so live forever
`author_issue_bootstrap.py:1265`	next action pointed at implementation, not lock finalization

Note issue_lock_store.py:1115, issue_lock_renewal.py:68, and issue_lock_recovery.py:118 already tolerated both claimant placements — three readers that disagreed with the fourth.

The canonical contract

New author_lock_contract.py is the single definition of an author issue lock: claimant, work_lease, lock_provenance, generation, and an explicit expiration state. Both gitea_lock_issue and bootstrap now build through it, so they cannot drift apart again.

How heartbeat, renewal, recovery, and `create_pr` now agree

Claimant — issue_lock_store.lock_claimant is promoted to the one shared reader and is used by the ownership check. work_lease.claimant is canonical; a top-level claimant is the legacy placement and is now read rather than refused. When both exist the work_lease copy wins, so a stale legacy copy can never decide ownership after an upgrade.
Expiration — represented explicitly as recorded, missing, or unparseable. An absent expires_at no longer masquerades as "not yet expired", which is what left a malformed lock permanently non-expiring and permanently ineligible for renewal.
Provenance — bootstrap writes through the existing sanctioned source, so the one lock that heartbeat and renewal accept is the one create_pr accepts.

Bootstrap fails closed before implementation

Bootstrap now reads its lock back and verifies it structurally before it can report success. A partial lock fails closed while the worktree still matches its base revision — when recovery is still cheap — names the missing fields, and returns implementation_allowed: false. Its exact_next_action is derived from the state actually returned, so it can no longer direct an author into the unrecoverable state.

Legacy and malformed-lock handling

Existing locks are not orphaned. A legacy top-level claimant is still read, and legacy leases keep their absolute-expiry clock. Malformed, partial, missing-expiration, expired, same-owner, foreign-owner, committed-branch, and legacy shapes each have explicit classification and test coverage.

Target-specific recovery

New gitea_recover_incomplete_bootstrap_lock upgrades an existing incomplete bootstrap lock, including one whose branch already carries legitimate pushed commits.

Before writing anything it proves: repository (remote/org/repo), issue number, claimant username and profile against server-resolved values, branch, worktree path, worktree existence and registration, that the worktree is on the recorded branch, that the observed head matches the caller's expected_head, the existing generation and provenance state, and the absence of healthy foreign ownership.

Deliberate non-behaviours:

never moves, resets, or rewinds a branch, and never requires the branch to match its base revision — preserving the committed work is the point;
never pushes and never opens a PR;
touches only the single lock file for that exact remote/org/repo/issue;
accepts no caller-supplied provenance and no caller-supplied authorization flag — both are minted server-side;
a matching username alone never proves ownership;
a healthy foreign-owned lock is refused outright (healthy_foreign_lock).

A recovered lock records a bootstrap_lock_recovery block carrying both sides of the transition — prior contract, prior missing fields, prior generation, prior owning session, replacement task_session_id, preserved head — so a recovered claim never reads as an original one. The write goes through the existing generation compare-and-swap.

Also adds gitea_inspect_issue_lock_contract, a strictly read-only surface that performs no lock, lease, branch, worktree, issue, or PR mutation.

The #447 guard was preserved

issue_lock_provenance.assess_lock_file_for_create_pr is unchanged, and SANCTIONED_LOCK_SOURCES was not widened. Bootstrap writes through the existing SOURCE_LOCK_ISSUE — the lock it produces is a canonical lock, not a second dialect with its own exemption. Bootstrap now satisfies the guard rather than the guard being relaxed to admit bootstrap. Tests assert the guard still rejects the old bootstrap shape, an unsanctioned source, and provenance without work_lease.

Test evidence

New suite — tests/test_issue_953_bootstrap_lock_contract.py, 61 cases:

venv/bin/python -m pytest tests/test_issue_953_bootstrap_lock_contract.py -q
61 passed in 1.52s                                        exit 0

Adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, and capability suites:

venv/bin/python -m pytest tests/test_issue_lock_store.py tests/test_author_issue_bootstrap.py \
  tests/test_issue_lock_provenance.py tests/test_issue_760_exact_owner_lease_renewal.py \
  tests/test_issue_760_mcp_renewal_path.py tests/test_issue_753_dead_pid_lock_recovery.py \
  tests/test_issue_790_heartbeat_mcp_path.py tests/test_issue_claim_heartbeat.py \
  tests/test_issue_892_author_bootstrap_deadlock.py tests/test_issue_943_runtime_context_helpers.py \
  tests/test_task_capability_role_invariants.py tests/test_lock_issue_mcp_registration.py \
  tests/test_dirty_same_claimant_session_rebind.py tests/test_issue_lock_adoption.py -q
283 passed, 92 subtests passed in 28.92s                  exit 0

Full suite, both runs from branches/ worktrees:

Run	Result
Head `cdf0dae`	28 failed, 5753 passed, 6 skipped, 1042 subtests in 191.33s
Base `82d71b77`	28 failed, 5692 passed, 6 skipped, 1042 subtests in 191.10s

The failing test-ID sets are identical (diff of the sorted FAILED lines is empty), so there are zero regressions. The +61 passes are exactly this issue's new suite.

Proven pre-existing failures

All 28 reproduce at the pinned base 82d71b77. Four were re-run individually at base because they looked like plausible candidates for being caused by this change — they are not:

test_issue_781_edit_issue_tool.py::TestDocumentationMatchesRegistry::test_documented_inventory_equals_registered_tools — fails at base, i.e. the documented tool inventory was already out of sync before this PR added any tool
test_dirty_orphan_worktree_recovery.py::FreshnessPidLess::{test_pid_less_lock_is_not_live, test_pid_less_with_far_future_expiry_still_not_live}
test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected

The remaining 24 span test_branch_cleanup_guard, test_commit_payloads, test_issue_702_review_findings_f1_f6, test_issue_784_dependency_edges, test_mcp_server, test_post_merge_moot_lease, test_preflight_workspace_repo_forwarding, test_reconciler_*, and test_workspace_guard_alignment — all present at base.

Preserved state

Issue #949 was preserved and not recovered. Its branch feat/issue-949-native-fleet-inventory remains at 92615f474bf6652d4e9ea59af7fd0dba03b56544 (verified via git ls-remote and the worktree head), its registered worktree and malformed lock were not modified, and its PR was not retried. The #949-shaped regression uses isolated temporary fixtures and throwaway git init repositories only.

Issues #950, #951, and #952 remain separate and were not modified. The stale PR #906 lease was not modified.

Reviewer focus areas

Ownership — issue_lock_store.lock_claimant and its use in _ownership_refusals. Confirm that tolerating the legacy placement widens where the claimant is read from and never whether ownership is proven: the recorded values are still compared against server-resolved identity and profile immediately after. Note assess_foreign_lock_overwrite also gains this tolerance — intended, since a legacy foreign claimant is now seen rather than read as empty.
Provenance — confirm SANCTIONED_LOCK_SOURCES is untouched and that no code path accepts caller-supplied provenance. build_canonical_issue_lock deliberately has no provenance parameter.
Recovery — bootstrap_lock_recovery.assess_bootstrap_lock_recovery is a pure function of caller observations. Worth checking whether any observation it trusts could be spoofed; note identity/profile come from _work_lease_claimant(host) server-side, and observed_head from a local git rev-parse, not from the caller. The caller-supplied expected_head can only cause a refusal.
Foreign-lock safety — the healthy-foreign refusal, and that a caller with a matching username but a different profile is treated as foreign.
Expiration semantics — expiration_state is additive; is_lease_expired and assess_lock_freshness are unchanged, so no existing liveness decision moved. Confirm that is the right conservatism versus making malformed locks non-live outright.

Author handoff follows, pinned to cdf0daefa91f891186bfad0513816ec2ddc90e7c. Not reviewed or merged by me.

Closes #953 Head: `cdf0daefa91f891186bfad0513816ec2ddc90e7c` Base: `master` at `82d71b77028a7abd4f8ab4a4e4d89658a187f73d` ## The defect `gitea_bootstrap_author_issue_worktree` reported a lock as created, wrote a shape no downstream author operation accepts, and then directed the author straight to implementation. Once the branch carried commits, every sanctioned path refused at once: heartbeat (claimant read only from `work_lease.claimant`), re-lock (the branch had advanced past its base revision), #760 exact-owner renewal (a lock with no recorded expiry is never *expired*), and the #447 create-PR provenance guard. Each gate is individually correct. The defect was that two writers disagreed about what a lock **is**. Verified against the pinned base rather than the issue's provisional line numbers: | Location | Finding | | --- | --- | | `author_issue_bootstrap.py:1203-1220` | claimant at the lock top level, `lease_id: null`, no `work_lease`, `lock_provenance`, or `expires_at` | | `gitea_mcp_server.py:4683-4701` | the only canonical writer | | `issue_lock_store.py:302-308` | ownership check read the claimant only from `work_lease` | | `issue_lock_store.py:690-703` | expiry read only from `work_lease.expires_at`, so absent read as "not expired" | | `issue_lock_store.py:858-879` | a lock with no `work_lease` classified legacy, no expiry, no pid, so **live forever** | | `author_issue_bootstrap.py:1265` | next action pointed at implementation, not lock finalization | Note `issue_lock_store.py:1115`, `issue_lock_renewal.py:68`, and `issue_lock_recovery.py:118` already tolerated both claimant placements — three readers that disagreed with the fourth. ## The canonical contract New `author_lock_contract.py` is the single definition of an author issue lock: claimant, `work_lease`, `lock_provenance`, generation, and an explicit expiration state. **Both** `gitea_lock_issue` and bootstrap now build through it, so they cannot drift apart again. ### How heartbeat, renewal, recovery, and `create_pr` now agree * **Claimant** — `issue_lock_store.lock_claimant` is promoted to the one shared reader and is used by the ownership check. `work_lease.claimant` is canonical; a top-level claimant is the legacy placement and is now *read* rather than refused. When both exist the `work_lease` copy wins, so a stale legacy copy can never decide ownership after an upgrade. * **Expiration** — represented explicitly as `recorded`, `missing`, or `unparseable`. An absent `expires_at` no longer masquerades as "not yet expired", which is what left a malformed lock permanently non-expiring *and* permanently ineligible for renewal. * **Provenance** — bootstrap writes through the existing sanctioned source, so the one lock that heartbeat and renewal accept is the one `create_pr` accepts. ### Bootstrap fails closed before implementation Bootstrap now reads its lock back and verifies it structurally before it can report success. A partial lock fails closed **while the worktree still matches its base revision** — when recovery is still cheap — names the missing fields, and returns `implementation_allowed: false`. Its `exact_next_action` is derived from the state actually returned, so it can no longer direct an author into the unrecoverable state. ## Legacy and malformed-lock handling Existing locks are not orphaned. A legacy top-level claimant is still read, and legacy leases keep their absolute-expiry clock. Malformed, partial, missing-expiration, expired, same-owner, foreign-owner, committed-branch, and legacy shapes each have explicit classification and test coverage. ## Target-specific recovery New `gitea_recover_incomplete_bootstrap_lock` upgrades an **existing** incomplete bootstrap lock, including one whose branch already carries legitimate pushed commits. Before writing anything it proves: repository (`remote`/`org`/`repo`), issue number, claimant username **and** profile against server-resolved values, branch, worktree path, worktree existence and registration, that the worktree is on the recorded branch, that the observed head matches the caller's `expected_head`, the existing generation and provenance state, and the absence of healthy foreign ownership. Deliberate non-behaviours: * never moves, resets, or rewinds a branch, and **never requires the branch to match its base revision** — preserving the committed work is the point; * never pushes and never opens a PR; * touches only the single lock file for that exact remote/org/repo/issue; * accepts **no** caller-supplied provenance and **no** caller-supplied authorization flag — both are minted server-side; * a matching username alone never proves ownership; * a healthy foreign-owned lock is refused outright (`healthy_foreign_lock`). A recovered lock records a `bootstrap_lock_recovery` block carrying both sides of the transition — prior contract, prior missing fields, prior generation, prior owning session, replacement `task_session_id`, preserved head — so a recovered claim never reads as an original one. The write goes through the existing generation compare-and-swap. Also adds `gitea_inspect_issue_lock_contract`, a strictly read-only surface that performs no lock, lease, branch, worktree, issue, or PR mutation. ## The #447 guard was preserved `issue_lock_provenance.assess_lock_file_for_create_pr` is **unchanged**, and `SANCTIONED_LOCK_SOURCES` was **not** widened. Bootstrap writes through the existing `SOURCE_LOCK_ISSUE` — the lock it produces *is* a canonical lock, not a second dialect with its own exemption. Bootstrap now satisfies the guard rather than the guard being relaxed to admit bootstrap. Tests assert the guard still rejects the old bootstrap shape, an unsanctioned source, and provenance without `work_lease`. ## Test evidence New suite — `tests/test_issue_953_bootstrap_lock_contract.py`, 61 cases: ``` venv/bin/python -m pytest tests/test_issue_953_bootstrap_lock_contract.py -q 61 passed in 1.52s exit 0 ``` Adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, and capability suites: ``` venv/bin/python -m pytest tests/test_issue_lock_store.py tests/test_author_issue_bootstrap.py \ tests/test_issue_lock_provenance.py tests/test_issue_760_exact_owner_lease_renewal.py \ tests/test_issue_760_mcp_renewal_path.py tests/test_issue_753_dead_pid_lock_recovery.py \ tests/test_issue_790_heartbeat_mcp_path.py tests/test_issue_claim_heartbeat.py \ tests/test_issue_892_author_bootstrap_deadlock.py tests/test_issue_943_runtime_context_helpers.py \ tests/test_task_capability_role_invariants.py tests/test_lock_issue_mcp_registration.py \ tests/test_dirty_same_claimant_session_rebind.py tests/test_issue_lock_adoption.py -q 283 passed, 92 subtests passed in 28.92s exit 0 ``` Full suite, both runs from `branches/` worktrees: | Run | Result | | --- | --- | | Head `cdf0dae` | **28 failed, 5753 passed, 6 skipped, 1042 subtests** in 191.33s | | Base `82d71b77` | **28 failed, 5692 passed, 6 skipped, 1042 subtests** in 191.10s | **The failing test-ID sets are identical** (`diff` of the sorted `FAILED` lines is empty), so there are **zero regressions**. The `+61` passes are exactly this issue's new suite. ### Proven pre-existing failures All 28 reproduce at the pinned base `82d71b77`. Four were re-run individually at base because they looked like plausible candidates for being caused by this change — they are not: * `test_issue_781_edit_issue_tool.py::TestDocumentationMatchesRegistry::test_documented_inventory_equals_registered_tools` — fails at base, i.e. the documented tool inventory was already out of sync before this PR added any tool * `test_dirty_orphan_worktree_recovery.py::FreshnessPidLess::{test_pid_less_lock_is_not_live, test_pid_less_with_far_future_expiry_still_not_live}` * `test_pr_ownership_issue_pr_mismatch.py::TestAuthorOwnershipIssuePrMismatch::test_pidless_durable_lock_rejected` The remaining 24 span `test_branch_cleanup_guard`, `test_commit_payloads`, `test_issue_702_review_findings_f1_f6`, `test_issue_784_dependency_edges`, `test_mcp_server`, `test_post_merge_moot_lease`, `test_preflight_workspace_repo_forwarding`, `test_reconciler_*`, and `test_workspace_guard_alignment` — all present at base. ## Preserved state **Issue #949 was preserved and not recovered.** Its branch `feat/issue-949-native-fleet-inventory` remains at `92615f474bf6652d4e9ea59af7fd0dba03b56544` (verified via `git ls-remote` and the worktree head), its registered worktree and malformed lock were not modified, and its PR was not retried. The #949-shaped regression uses isolated temporary fixtures and throwaway `git init` repositories only. **Issues #950, #951, and #952 remain separate** and were not modified. The stale PR #906 lease was not modified. ## Reviewer focus areas 1. **Ownership** — `issue_lock_store.lock_claimant` and its use in `_ownership_refusals`. Confirm that tolerating the legacy placement widens *where* the claimant is read from and never *whether* ownership is proven: the recorded values are still compared against server-resolved identity and profile immediately after. Note `assess_foreign_lock_overwrite` also gains this tolerance — intended, since a legacy foreign claimant is now seen rather than read as empty. 2. **Provenance** — confirm `SANCTIONED_LOCK_SOURCES` is untouched and that no code path accepts caller-supplied provenance. `build_canonical_issue_lock` deliberately has no provenance parameter. 3. **Recovery** — `bootstrap_lock_recovery.assess_bootstrap_lock_recovery` is a pure function of caller observations. Worth checking whether any observation it trusts could be spoofed; note `identity`/`profile` come from `_work_lease_claimant(host)` server-side, and `observed_head` from a local `git rev-parse`, not from the caller. The caller-supplied `expected_head` can only cause a refusal. 4. **Foreign-lock safety** — the healthy-foreign refusal, and that a caller with a matching username but a different profile is treated as foreign. 5. **Expiration semantics** — `expiration_state` is additive; `is_lease_expired` and `assess_lock_freshness` are unchanged, so no existing liveness decision moved. Confirm that is the right conservatism versus making malformed locks non-live outright. Author handoff follows, pinned to `cdf0daefa91f891186bfad0513816ec2ddc90e7c`. Not reviewed or merged by me.

jcwalker3 added 1 commit 2026-07-27 23:38:10 -05:00

fix(author): unify the bootstrap and lock_issue issue-lock contract (Closes #953 ) cdf0daefa9

gitea_bootstrap_author_issue_worktree wrote a lock no downstream author
operation accepts, then directed the author straight to implementation. Once
the branch carried commits, heartbeat, re-lock, exact-owner renewal, and the
#447 create-PR guard all refused simultaneously and no sanctioned recovery
path remained eligible.

Each of those gates is individually correct. The defect was that two writers
disagreed about what a lock is.

- Add author_lock_contract as the single canonical definition: claimant,
  work_lease, lock_provenance, generation, and an explicit expiration state.
  Both gitea_lock_issue and bootstrap now build through it.
- Promote issue_lock_store.lock_claimant to the one shared claimant reader and
  use it in the ownership check, so a claimant recorded at the lock top level
  is read rather than refused. The values are still compared against
  server-resolved identity and profile, so no legacy placement grants anything
  the canonical placement would not.
- Represent missing expiration explicitly. An absent expires_at previously read
  as "not yet expired", leaving a malformed lock permanently non-expiring and
  permanently ineligible for #760 renewal.
- Bootstrap reads its lock back and verifies it structurally before reporting
  success. A partial lock fails closed while the worktree is still
  base-equivalent, names the missing fields, and never reports
  implementation_allowed. Its exact_next_action now matches the state returned.
- Add gitea_recover_incomplete_bootstrap_lock for locks already written by the
  old bootstrap, including those whose branches carry pushed commits. It never
  moves, resets, or rewinds a branch, never requires base-equivalence, never
  pushes or opens a PR, and touches only the target lock. It proves repository,
  issue, claimant username and profile, branch, worktree, registration, and
  head before writing, refuses healthy foreign-owned locks, and mints
  provenance and authorization server-side.
- Add gitea_inspect_issue_lock_contract, a strictly read-only surface.
- Document the required ordering and the recovery path.

The #447 provenance guard is unchanged and the sanctioned source set was not
widened: bootstrap now satisfies the guard rather than the guard being relaxed
to admit bootstrap.

Tests: 61 new cases covering the canonical schema, immediate heartbeat,
renewal before and after commits, the create_pr guard, executable next actions,
partial and malformed and missing-expiration and expired and same-owner and
foreign-owner and legacy locks, recovery isolation, read-only inspection, and
the full bootstrap-implement-commit-push-create_pr regression against isolated
fixtures.

Full suite at head: 28 failed, 5753 passed, 6 skipped, 1042 subtests.
Full suite at base 82d71b77: 28 failed, 5692 passed, 6 skipped, 1042 subtests.
Failing test-ID sets are identical, so there are zero regressions; the +61
passes are this issue's new suite.

Issue #949 was preserved and not recovered: its branch remains at
92615f474b and its worktree, lock, and PR state
were not touched. The #949-shaped regression uses isolated fixtures only.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

jcwalker3 referenced this pull request

2026-07-27 23:39:40 -05:00

fix(author): prevent bootstrap lock-provenance dead ends before PR creation #953

sysadmin commented

2026-07-27 23:55:08 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 53219-227c9ce17d71
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-cdf0dae
phase: claimed
candidate_head: cdf0daefa9
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T04:55:07Z
expires_at: 2026-07-28T05:05:07Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 53219-227c9ce17d71 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-cdf0dae phase: claimed candidate_head: cdf0daefa91f891186bfad0513816ec2ddc90e7c target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T04:55:07Z expires_at: 2026-07-28T05:05:07Z blocker: none

sysadmin requested changes 2026-07-27 23:58:57 -05:00

Dismissed

sysadmin left a comment

REQUEST_CHANGES — PR #954 (Closes #953)

Reviewed at head cdf0daefa91f891186bfad0513816ec2ddc90e7c against base master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d, in a dedicated detached review worktree. Merge base equals the live master head, the branch is one commit, 8 files, +2390/-58, and every changed path is within the issue's scope. Neither #949's branch nor #950/#951/#952 are touched.

The design is sound and the diagnosis in the PR description matches the code. Two defects in the new call sites block approval; both are small.

Independently confirmed

The #447 guard really is untouched. git diff base..head -- issue_lock_provenance.py is empty and SANCTIONED_LOCK_SOURCES still holds exactly its five prior members. Bootstrap satisfies the guard by writing through SOURCE_LOCK_ISSUE; the guard was not widened to admit a second dialect.
New suite: tests/test_issue_953_bootstrap_lock_contract.py — 61 passed.
Adjacent lock / bootstrap / lease / renewal / recovery / heartbeat / capability suites at head: 241 passed, 84 subtests passed.
The bootstrap wiring does execute under test. I replaced the AC7 read-back at author_issue_bootstrap.py:1252 with a path that cannot resolve; tests/test_author_issue_bootstrap.py then failed 4 cases, among them test_cross_process_concurrency, test_idempotent_replay, and test_incompatible_idempotency_replay_refusal. The sabotage was reverted and the worktree verified clean. So the pre-existing bootstrap suite does cover the new path, even though this PR's own BootstrapWiring cases are source-text assertions.
The claimant precedence flip is safe. issue_lock_store.lock_claimant now prefers work_lease where it previously preferred the top level. The only writer that records both placements is dirty_orphan_worktree_recovery.py:604,613, and it writes identical values to both, so no existing reader changes its verdict.

F1 (blocking) — the new author mutation omits the namespace mutation gate

gitea_recover_incomplete_bootstrap_lock gates on role_session_router.check_author_mutation_after_reviewer_stop and _profile_permission_block only.

Every other author state-creating mutation in this server carries a third gate, _namespace_mutation_block(task, remote=remote). There are exactly six call sites and they are the whole class: gitea_create_issue, gitea_create_pr, gitea_commit_files, gitea_publish_unpublished_issue_branch, gitea_bootstrap_author_issue_worktree, and the structurally closest neighbour gitea_recover_dirty_orphaned_issue_worktree — the same shape of tool, writing the same durable lock, and it does call it. The new tool is the outlier.

The surviving gate does not cover the gap. The task's required permission is gitea.issue.comment, and the prgs-reviewer profile allows gitea.issue.comment — confirmed against this session's live gitea_whoami output. _ensure_matching_profile returns a profile name that both call sites discard, so it refuses nothing.

Failure scenario. A reviewer-bound session invokes gitea_recover_incomplete_bootstrap_lock against an author-owned lock. Expected, matching the other six: refusal carrying namespace_block: true plus a BLOCKED audit event naming the namespace and profile. Actual: no namespace evaluation and no audit event; the refusal arrives one layer later, from the exact-owner claimant comparison inside assess_bootstrap_lock_recovery, as foreign_claimant. The durable lock is not taken over, so this is a missing layer and a missing audit record rather than a takeover — but it is the one gate this class of tool uniformly carries, and skipping it is what leaves the audit trail silent about the attempt.

Fix. Add _namespace_mutation_block(task, remote=remote) between the router check and the permission block. role_namespace_gate.check_author_mutation_namespace already routes this task correctly through required_role_for_task, because recover_incomplete_bootstrap_lock is registered with role: author in task_capability_map.py. No registry change is needed.

F2 (blocking) — bootstrap's AC7 refusal returns a next action that cannot be executed

This is the exact failure class #953 exists to eliminate, reintroduced on the new fail-closed path.

At author_issue_bootstrap.py:1258 the AC7 refusal calls run_compensating_recovery(journal, root, journal_dir=lock_dir). That call, by design, rolls the transition back: it releases the issue lock, rmtrees the worktree when clean — which it will be, since no implementation bytes have been written yet — and deletes the created branch.

Fourteen lines later, at author_issue_bootstrap.py:1272, the return payload sets exact_next_action to author_lock_contract.recommended_action(contract). For a non-canonical contract that text is author_lock_contract.py:437-442:

Do not begin implementation. Call gitea_recover_incomplete_bootstrap_lock for this exact issue, branch, and worktree to upgrade the lock to the canonical contract, or gitea_lock_issue while the worktree is still base-matching.

Failure scenario. Bootstrap writes a lock that fails the structural check. Compensation removes the lock, the worktree, and the branch. The author follows the returned instruction and calls gitea_recover_incomplete_bootstrap_lock for that issue, branch, and worktree. It refuses no_durable_lock — bootstrap_lock_recovery.py:98-109 — because compensation already released the lock; had the lock survived, it would refuse worktree_invalid instead, since the directory was removed. The fallback half of the same sentence is equally dead: gitea_lock_issue cannot bind a worktree that no longer exists. The author is handed two refusals in a row for a state that is in fact cleanly recoverable by re-running bootstrap.

The comment directly above that line claims AC15 — never strand a branch or worktree without a structured recovery recommendation. The recommendation is structured, but it describes the pre-compensation state rather than the state actually returned, which is what AC5 requires.

Fix. On this path the artifacts were rolled back, so the executable action is to re-run gitea_bootstrap_author_issue_worktree after resolving the reported missing_fields. Either derive exact_next_action from the compensation outcome instead of the pre-compensation contract, or give recommended_action an explicit post-rollback branch. Worth asserting in a case that drives bootstrap_author_issue_worktree to this refusal and then checks that the returned action is one the resulting state accepts.

F3 (non-blocking) — dead provenance constant

author_lock_contract.py:47 declares SOURCE_BOOTSTRAP_LOCK_RECOVERY = "gitea_recover_incomplete_bootstrap_lock" with a comment presenting it as the recovery source. Nothing reads it: the recovery path writes source=issue_lock_provenance.SOURCE_LOCK_ISSUE and distinguishes itself through written_by_tool plus the bootstrap_lock_recovery block. Reusing the sanctioned source is the right call — a new source would have forced exactly the #447 widening the safety requirements forbid — but the unused constant reads as if a second source exists. Drop it, or restate it as a comment explaining the deliberate reuse.

F4 (non-blocking) — the two new tool functions have no executing coverage

Every occurrence of gitea_recover_incomplete_bootstrap_lock and gitea_inspect_issue_lock_contract under tests/ is a string literal — a tool= argument, an assertion on returned prose, or a docs substring check. _observe_recovery_worktree has zero references anywhere outside its definition. The suite reconstructs the tool's sequence by hand (assess_bootstrap_lock_recovery, then build_canonical_issue_lock, then build_recovery_record, then bind_session_lock), which validates the decision layer well but cannot see a divergence between that hand-written sequence and the tool body. That is precisely how F1 and F2 survived 61 cases: both are call-site defects, and no case drives a call site.

test_inspection_performs_no_mutation has the same shape — it proves the pure assessment functions mutate nothing, not that the registered read-only tool does, and that tool does run git subprocesses.

Not addressed, and fine to leave

author_lock_contract.lock_claimant states that the reader now lives in one place. Four private _lock_claimant copies remain, in issue_lock_renewal.py:68, issue_lock_recovery.py:118, dirty_orphan_worktree_recovery.py:185, and dirty_same_claimant_session_rebind.py:105, alongside _lock_claimant_view at issue_lock_store.py:1386. They all already tolerate both placements, so behaviour is consistent today and I am not asking for the consolidation in this PR — but the docstring's claim is stronger than the code, and drift between five readers is the failure mode #953 was opened about.

Canonical PR State

STATE: changes-requested
WHO_IS_NEXT: author
NEXT_ACTION: Remediate F1 and F2 on branch fix/issue-953-bootstrap-lock-provenance, push, and hand back to an independent reviewer
NEXT_PROMPT:

Role: AUTHOR
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
Issue: #953
PR: #954
Branch: fix/issue-953-bootstrap-lock-provenance
Head at review: cdf0daefa91f891186bfad0513816ec2ddc90e7c

1. Native MCP only. gitea_whoami then gitea_resolve_task_capability(task='work_issue') on prgs-author. Align the existing branches/issue-953-bootstrap-lock-provenance worktree to the PR head, then gitea_lock_issue to renew the claim on the open PR.
2. F1: in gitea_recover_incomplete_bootstrap_lock, add _namespace_mutation_block(task, remote=remote) between check_author_mutation_after_reviewer_stop and _profile_permission_block, matching gitea_recover_dirty_orphaned_issue_worktree.
3. F2: bootstrap's AC7 refusal at author_issue_bootstrap.py:1272 must derive exact_next_action from the post-compensation state. run_compensating_recovery has already released the lock and removed the branch and worktree, so the executable action is to re-run gitea_bootstrap_author_issue_worktree after resolving the reported missing_fields.
4. Add coverage that drives gitea_recover_incomplete_bootstrap_lock, gitea_inspect_issue_lock_contract, and the bootstrap AC7 refusal path through the tool functions rather than reconstructing their sequence by hand.
5. F3 optional: drop or re-document the unused SOURCE_BOOTSTRAP_LOCK_RECOVERY constant.
6. Re-run tests/test_issue_953_bootstrap_lock_contract.py plus the adjacent lock and bootstrap suites, and a full-suite pair from branches/ worktrees comparing failing test IDs against the pinned base, not counts.
7. Push to the existing PR #954. Do not open a second PR. Hand off to an independent reviewer.

WHAT_HAPPENED: Independent review of PR #954 at head cdf0daefa9. Verified the #447 provenance guard and SANCTIONED_LOCK_SOURCES are unchanged against base 82d71b7702, ran the new 61-case suite and 241 adjacent cases green at head, proved the bootstrap AC7 read-back is covered by the pre-existing bootstrap suite via a reverted sabotage probe, and confirmed the claimant precedence flip is behaviourally inert. Found two blocking call-site defects and two non-blocking ones.
WHY: F1 leaves the one new durable-lock-writing author mutation without the namespace gate that all six comparable author mutations carry, so a reviewer-bound attempt produces no namespace refusal and no BLOCKED audit record. F2 returns an exact_next_action describing a lock, branch, and worktree that the compensating rollback on that same code path has already destroyed, so both halves of the advice fail closed — the AC5 failure mode this issue exists to remove.
ISSUE: 953
HEAD_SHA: cdf0daefa9
REVIEW_STATUS: request_changes
MERGE_READY: no
BLOCKERS: review_changes_requested
VALIDATION: tests/test_issue_953_bootstrap_lock_contract.py 61 passed at head; 13 adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, and capability suites 241 passed with 84 subtests at head; git diff base..head of issue_lock_provenance.py empty, proving the #447 guard and sanctioned source set untouched; merge base equals live master 82d71b77028a7abd4f8ab4a4e4d89658a187f73d; review worktree verified clean via git status --porcelain --untracked-files=all after the reverted probe.
LAST_UPDATED_BY: reviewer

Reviewed independently at cdf0daefa91f891186bfad0513816ec2ddc90e7c. Not authored or merged by me.

## REQUEST_CHANGES — PR #954 (Closes #953) Reviewed at head `cdf0daefa91f891186bfad0513816ec2ddc90e7c` against base `master` at `82d71b77028a7abd4f8ab4a4e4d89658a187f73d`, in a dedicated detached review worktree. Merge base equals the live master head, the branch is one commit, 8 files, +2390/-58, and every changed path is within the issue's scope. Neither #949's branch nor #950/#951/#952 are touched. The design is sound and the diagnosis in the PR description matches the code. Two defects in the new call sites block approval; both are small. ### Independently confirmed - **The #447 guard really is untouched.** `git diff base..head -- issue_lock_provenance.py` is empty and `SANCTIONED_LOCK_SOURCES` still holds exactly its five prior members. Bootstrap satisfies the guard by writing through `SOURCE_LOCK_ISSUE`; the guard was not widened to admit a second dialect. - **New suite:** `tests/test_issue_953_bootstrap_lock_contract.py` — 61 passed. - **Adjacent lock / bootstrap / lease / renewal / recovery / heartbeat / capability suites at head:** 241 passed, 84 subtests passed. - **The bootstrap wiring does execute under test.** I replaced the AC7 read-back at `author_issue_bootstrap.py:1252` with a path that cannot resolve; `tests/test_author_issue_bootstrap.py` then failed 4 cases, among them `test_cross_process_concurrency`, `test_idempotent_replay`, and `test_incompatible_idempotency_replay_refusal`. The sabotage was reverted and the worktree verified clean. So the pre-existing bootstrap suite does cover the new path, even though this PR's own `BootstrapWiring` cases are source-text assertions. - **The claimant precedence flip is safe.** `issue_lock_store.lock_claimant` now prefers `work_lease` where it previously preferred the top level. The only writer that records both placements is `dirty_orphan_worktree_recovery.py:604,613`, and it writes identical values to both, so no existing reader changes its verdict. --- ## F1 (blocking) — the new author mutation omits the namespace mutation gate `gitea_recover_incomplete_bootstrap_lock` gates on `role_session_router.check_author_mutation_after_reviewer_stop` and `_profile_permission_block` only. Every other author state-creating mutation in this server carries a third gate, `_namespace_mutation_block(task, remote=remote)`. There are exactly six call sites and they are the whole class: `gitea_create_issue`, `gitea_create_pr`, `gitea_commit_files`, `gitea_publish_unpublished_issue_branch`, `gitea_bootstrap_author_issue_worktree`, and the structurally closest neighbour `gitea_recover_dirty_orphaned_issue_worktree` — the same shape of tool, writing the same durable lock, and it does call it. The new tool is the outlier. The surviving gate does not cover the gap. The task's required permission is `gitea.issue.comment`, and the `prgs-reviewer` profile allows `gitea.issue.comment` — confirmed against this session's live `gitea_whoami` output. `_ensure_matching_profile` returns a profile name that both call sites discard, so it refuses nothing. **Failure scenario.** A reviewer-bound session invokes `gitea_recover_incomplete_bootstrap_lock` against an author-owned lock. Expected, matching the other six: refusal carrying `namespace_block: true` plus a `BLOCKED` audit event naming the namespace and profile. Actual: no namespace evaluation and no audit event; the refusal arrives one layer later, from the exact-owner claimant comparison inside `assess_bootstrap_lock_recovery`, as `foreign_claimant`. The durable lock is not taken over, so this is a missing layer and a missing audit record rather than a takeover — but it is the one gate this class of tool uniformly carries, and skipping it is what leaves the audit trail silent about the attempt. **Fix.** Add `_namespace_mutation_block(task, remote=remote)` between the router check and the permission block. `role_namespace_gate.check_author_mutation_namespace` already routes this task correctly through `required_role_for_task`, because `recover_incomplete_bootstrap_lock` is registered with `role: author` in `task_capability_map.py`. No registry change is needed. --- ## F2 (blocking) — bootstrap's AC7 refusal returns a next action that cannot be executed This is the exact failure class #953 exists to eliminate, reintroduced on the new fail-closed path. At `author_issue_bootstrap.py:1258` the AC7 refusal calls `run_compensating_recovery(journal, root, journal_dir=lock_dir)`. That call, by design, rolls the transition back: it releases the issue lock, `rmtree`s the worktree when clean — which it will be, since no implementation bytes have been written yet — and deletes the created branch. Fourteen lines later, at `author_issue_bootstrap.py:1272`, the return payload sets `exact_next_action` to `author_lock_contract.recommended_action(contract)`. For a non-canonical contract that text is `author_lock_contract.py:437-442`: > Do not begin implementation. Call gitea_recover_incomplete_bootstrap_lock for this exact issue, branch, and worktree to upgrade the lock to the canonical contract, or gitea_lock_issue while the worktree is still base-matching. **Failure scenario.** Bootstrap writes a lock that fails the structural check. Compensation removes the lock, the worktree, and the branch. The author follows the returned instruction and calls `gitea_recover_incomplete_bootstrap_lock` for that issue, branch, and worktree. It refuses `no_durable_lock` — `bootstrap_lock_recovery.py:98-109` — because compensation already released the lock; had the lock survived, it would refuse `worktree_invalid` instead, since the directory was removed. The fallback half of the same sentence is equally dead: `gitea_lock_issue` cannot bind a worktree that no longer exists. The author is handed two refusals in a row for a state that is in fact cleanly recoverable by re-running bootstrap. The comment directly above that line claims AC15 — never strand a branch or worktree without a structured recovery recommendation. The recommendation is structured, but it describes the pre-compensation state rather than the state actually returned, which is what AC5 requires. **Fix.** On this path the artifacts were rolled back, so the executable action is to re-run `gitea_bootstrap_author_issue_worktree` after resolving the reported `missing_fields`. Either derive `exact_next_action` from the compensation outcome instead of the pre-compensation contract, or give `recommended_action` an explicit post-rollback branch. Worth asserting in a case that drives `bootstrap_author_issue_worktree` to this refusal and then checks that the returned action is one the resulting state accepts. --- ## F3 (non-blocking) — dead provenance constant `author_lock_contract.py:47` declares `SOURCE_BOOTSTRAP_LOCK_RECOVERY = "gitea_recover_incomplete_bootstrap_lock"` with a comment presenting it as the recovery source. Nothing reads it: the recovery path writes `source=issue_lock_provenance.SOURCE_LOCK_ISSUE` and distinguishes itself through `written_by_tool` plus the `bootstrap_lock_recovery` block. Reusing the sanctioned source is the right call — a new source would have forced exactly the #447 widening the safety requirements forbid — but the unused constant reads as if a second source exists. Drop it, or restate it as a comment explaining the deliberate reuse. ## F4 (non-blocking) — the two new tool functions have no executing coverage Every occurrence of `gitea_recover_incomplete_bootstrap_lock` and `gitea_inspect_issue_lock_contract` under `tests/` is a string literal — a `tool=` argument, an assertion on returned prose, or a docs substring check. `_observe_recovery_worktree` has zero references anywhere outside its definition. The suite reconstructs the tool's sequence by hand (`assess_bootstrap_lock_recovery`, then `build_canonical_issue_lock`, then `build_recovery_record`, then `bind_session_lock`), which validates the decision layer well but cannot see a divergence between that hand-written sequence and the tool body. That is precisely how F1 and F2 survived 61 cases: both are call-site defects, and no case drives a call site. `test_inspection_performs_no_mutation` has the same shape — it proves the pure assessment functions mutate nothing, not that the registered read-only tool does, and that tool does run `git` subprocesses. ## Not addressed, and fine to leave `author_lock_contract.lock_claimant` states that the reader now lives in one place. Four private `_lock_claimant` copies remain, in `issue_lock_renewal.py:68`, `issue_lock_recovery.py:118`, `dirty_orphan_worktree_recovery.py:185`, and `dirty_same_claimant_session_rebind.py:105`, alongside `_lock_claimant_view` at `issue_lock_store.py:1386`. They all already tolerate both placements, so behaviour is consistent today and I am not asking for the consolidation in this PR — but the docstring's claim is stronger than the code, and drift between five readers is the failure mode #953 was opened about. --- ## Canonical PR State STATE: changes-requested WHO_IS_NEXT: author NEXT_ACTION: Remediate F1 and F2 on branch fix/issue-953-bootstrap-lock-provenance, push, and hand back to an independent reviewer NEXT_PROMPT: ```text Role: AUTHOR Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools Issue: #953 PR: #954 Branch: fix/issue-953-bootstrap-lock-provenance Head at review: cdf0daefa91f891186bfad0513816ec2ddc90e7c 1. Native MCP only. gitea_whoami then gitea_resolve_task_capability(task='work_issue') on prgs-author. Align the existing branches/issue-953-bootstrap-lock-provenance worktree to the PR head, then gitea_lock_issue to renew the claim on the open PR. 2. F1: in gitea_recover_incomplete_bootstrap_lock, add _namespace_mutation_block(task, remote=remote) between check_author_mutation_after_reviewer_stop and _profile_permission_block, matching gitea_recover_dirty_orphaned_issue_worktree. 3. F2: bootstrap's AC7 refusal at author_issue_bootstrap.py:1272 must derive exact_next_action from the post-compensation state. run_compensating_recovery has already released the lock and removed the branch and worktree, so the executable action is to re-run gitea_bootstrap_author_issue_worktree after resolving the reported missing_fields. 4. Add coverage that drives gitea_recover_incomplete_bootstrap_lock, gitea_inspect_issue_lock_contract, and the bootstrap AC7 refusal path through the tool functions rather than reconstructing their sequence by hand. 5. F3 optional: drop or re-document the unused SOURCE_BOOTSTRAP_LOCK_RECOVERY constant. 6. Re-run tests/test_issue_953_bootstrap_lock_contract.py plus the adjacent lock and bootstrap suites, and a full-suite pair from branches/ worktrees comparing failing test IDs against the pinned base, not counts. 7. Push to the existing PR #954. Do not open a second PR. Hand off to an independent reviewer. ``` WHAT_HAPPENED: Independent review of PR #954 at head cdf0daefa91f891186bfad0513816ec2ddc90e7c. Verified the #447 provenance guard and SANCTIONED_LOCK_SOURCES are unchanged against base 82d71b77028a7abd4f8ab4a4e4d89658a187f73d, ran the new 61-case suite and 241 adjacent cases green at head, proved the bootstrap AC7 read-back is covered by the pre-existing bootstrap suite via a reverted sabotage probe, and confirmed the claimant precedence flip is behaviourally inert. Found two blocking call-site defects and two non-blocking ones. WHY: F1 leaves the one new durable-lock-writing author mutation without the namespace gate that all six comparable author mutations carry, so a reviewer-bound attempt produces no namespace refusal and no BLOCKED audit record. F2 returns an exact_next_action describing a lock, branch, and worktree that the compensating rollback on that same code path has already destroyed, so both halves of the advice fail closed — the AC5 failure mode this issue exists to remove. ISSUE: 953 HEAD_SHA: cdf0daefa91f891186bfad0513816ec2ddc90e7c REVIEW_STATUS: request_changes MERGE_READY: no BLOCKERS: review_changes_requested VALIDATION: tests/test_issue_953_bootstrap_lock_contract.py 61 passed at head; 13 adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, and capability suites 241 passed with 84 subtests at head; git diff base..head of issue_lock_provenance.py empty, proving the #447 guard and sanctioned source set untouched; merge base equals live master 82d71b77028a7abd4f8ab4a4e4d89658a187f73d; review worktree verified clean via git status --porcelain --untracked-files=all after the reverted probe. LAST_UPDATED_BY: reviewer Reviewed independently at `cdf0daefa91f891186bfad0513816ec2ddc90e7c`. Not authored or merged by me.

sysadmin commented

2026-07-27 23:59:20 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 53219-227c9ce17d71
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-cdf0dae
phase: released
candidate_head: cdf0daefa9
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T04:59:20Z
expires_at: 2026-07-28T05:09:20Z
blocker: manual-release

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 53219-227c9ce17d71 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-cdf0dae phase: released candidate_head: cdf0daefa91f891186bfad0513816ec2ddc90e7c target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T04:59:20Z expires_at: 2026-07-28T05:09:20Z blocker: manual-release

jcwalker3 added 3 commits 2026-07-28 01:09:41 -05:00

fix(author): gate bootstrap-lock recovery on the namespace mutation wall (#953 ) 55d66c57e4

Review 632 F1. `gitea_recover_incomplete_bootstrap_lock` writes the same
durable author issue lock as `gitea_recover_dirty_orphaned_issue_worktree`
but gated only on the reviewer-stop router check and the profile permission
block. Every other author state-creating mutation carries a third gate,
`_namespace_mutation_block`, and this tool was the outlier.

The surviving gates did not cover the gap: the task's required permission is
`gitea.issue.comment`, which every configured role holds, and
`_ensure_matching_profile` returns a profile name both call sites discard, so
it refuses nothing. A reviewer-bound session reached the exact-owner claimant
comparison inside `assess_bootstrap_lock_recovery` and was refused there —
one layer too late, with no namespace evaluation and no BLOCKED audit record
of the attempt.

Adding the call alone would have been inert. `check_author_mutation_namespace`
routes through `role_session_router.required_role_for_task`, which reads
`TASK_REQUIRED_ROLE` — not `task_capability_map` — and that table had no entry
for this task, so the check short-circuited to "allowed" for every caller.
The task is therefore registered in `TASK_REQUIRED_ROLE` and `AUTHOR_TASKS`,
matching the role the capability map already records.

The reviewer-namespace check alone is also insufficient for a task gated on
`gitea.issue.comment`, since merger, controller, and reconciler profiles hold
it too. `role_namespace_gate.check_author_role_kind` is added as an opt-in,
additive wall requiring the active profile's derived role kind to be exactly
`author`; `mixed` is refused rather than admitted. `_namespace_mutation_block`
gains a keyword-only `author_role_exclusive` flag, default off, so the six
pre-existing call sites are byte-for-byte unchanged.

Refusals produce the standard structured denial (`namespace_block: true`,
`mcp_namespace`) and the standard BLOCKED audit record, and no lock, lease,
generation, branch, worktree, issue, or PR is touched. Valid `prgs-author`
execution is unaffected. No reviewer permission is broadened and no existing
role wall is weakened.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

fix(author): derive AC7 guidance from the state compensation actually leaves (#953 ) 1aa351718a

Review 632 F2. The bootstrap AC7 read-back refusal calls
`run_compensating_recovery` and *then* reported
`author_lock_contract.recommended_action(contract)` — advice computed from the
malformed lock that provoked the rollback, not from the state the rollback
left. Compensation releases the lock, removes the worktree (always clean
there, no implementation bytes having been written), and deletes the branch,
so an author following that advice got `no_durable_lock` from
`gitea_recover_incomplete_bootstrap_lock` and, had the lock survived,
`worktree_invalid` instead; the `gitea_lock_issue` half of the same sentence
cannot bind a worktree that no longer exists. Two refusals in a row for a
state a plain bootstrap retry fixes — the unexecutable-guidance failure class
this issue exists to remove, reintroduced on the new fail-closed path.

Investigating that path surfaced why the "clean retry" state was in practice
unreachable: `run_compensating_recovery` has called
`issue_lock_store.release_session_lock` since #850, and that function has
never existed. The `AttributeError` landed in a bare `except Exception: pass`,
so every rollback removed the branch and worktree and silently left the lock
behind — precisely the uninspectable, unrecoverable state #953 is about
(`gitea_recover_incomplete_bootstrap_lock` refuses `worktree_invalid`,
`gitea_lock_issue` has no worktree to bind). Confirmed dead at the pinned base
`82d71b77`, not introduced by this branch.

`release_session_lock` is therefore implemented: it removes exactly one
durable lock whose recorded `owner_session` matches the caller's, keyed by
repository when known, refusing on zero or multiple matches so no caller can
delete a lock it does not own and an ambiguous directory is never guessed at.
Bootstrap phase journals and session pointers that share the directory are
excluded by shape. The flock sidecar is deliberately left alone. The caller no
longer swallows a release failure; it records `lock_release_failed:...`.

`assess_post_compensation_state` then classifies from directly observed
durable state — lock file, worktree directory, and branch ref — rather than
from the journal's `rolled_back` list, which records only what compensation
attempted. Three distinct states: `complete` (nothing remains),
`partial` (rollback ran, artifacts survive by design or because a step
errored), `failed` (rollback never completed, so nothing is proven removed).

`post_compensation_action` answers for exactly what survives:

  complete                      -> re-run gitea_bootstrap_author_issue_worktree
  lock + branch + worktree      -> gitea_recover_incomplete_bootstrap_lock
  branch + worktree, no lock    -> gitea_lock_issue (still base-equivalent)
  lock only, worktree gone      -> gitea_inspect_issue_lock_contract
  branch only                   -> gitea_inspect_issue_lock_contract, then retry
  rollback did not complete     -> gitea_inspect_issue_lock_contract

No branch names an artifact the classification says is gone, and a failed
rollback step is stated rather than presented as an intentional outcome. The
refusal payload carries `compensating_recovery` and `post_compensation_state`
alongside the derived `exact_next_action`, still with
`implementation_allowed: false`.

Also removes the dead `SOURCE_BOOTSTRAP_LOCK_RECOVERY` constant (review 632
F3), which had no readers and implied a second lock source; the deliberate
reuse of `SOURCE_LOCK_ISSUE` is now stated as a comment. The #447 guard and
`SANCTIONED_LOCK_SOURCES` remain untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

test(author): execute the native #953 tool functions and compensation paths (#953 ) b4c9f55890

Review 632 F4. Every prior reference to `gitea_recover_incomplete_bootstrap_lock`
and `gitea_inspect_issue_lock_contract` under `tests/` was a string literal — a
`tool=` argument, an assertion on returned prose, or a docs substring check —
and the suite reconstructed the recovery sequence by hand from
`assess_bootstrap_lock_recovery`, `build_canonical_issue_lock`,
`build_recovery_record`, and `bind_session_lock`. A hand-written sequence
validates the decision layer but cannot see a divergence between itself and
the tool body, which is exactly how F1 and F2 — both call-site defects —
survived 61 passing cases.

The new cases drive the registered functions against a real `git init`
repository, a real durable lock file, and config-backed profiles:

* `NamespaceMutationWallOnRecovery` — the gate is reached with this task and
  `author_role_exclusive=True`; its return value aborts the tool rather than
  being computed and discarded; the author namespace succeeds and produces a
  canonical lock; a reviewer namespace is refused with `namespace_block` even
  when the claimant data would otherwise match, and emits the standard BLOCKED
  audit record; merger is refused; a mismatched claimant profile is refused by
  the exact-owner layer with the namespace wall explicitly clear; a mismatched
  head is refused; every refusal leaves the lock bytes, generation, branch,
  worktree, and an unrelated lock untouched. A subtest matrix asserts the
  role-kind wall admits `author` and refuses reviewer, merger, limited, and
  mixed.
* `InspectionToolExecutes` — the registered read-only tool reports the contract
  and the recovery preview while leaving lock bytes, mtime, HEAD, and porcelain
  status unchanged, and reports an absent lock without creating one.
* `Ac7PostCompensationGuidance` — drives the real bootstrap to its AC7 refusal
  with a forced partial lock and asserts the returned action against the state
  the rollback actually left: complete cleanup directs to a bootstrap retry and
  that retry is then executed and succeeds, leaving exactly one canonical lock
  and one branch; partial cleanup with a surviving lock, and with a surviving
  branch and worktree, each get their own executable action; a rollback that
  never completed is distinguished from both; no recommendation names a deleted
  artifact; unrelated locks are byte-identical afterwards. Two cases cover
  `release_session_lock` directly — that the rollback now really removes the
  lock, and that it refuses a lock owned by another session.
* `NativeEndToEndBootstrapToCreatePr` — bootstrap, inspect, heartbeat,
  legitimate divergence (commit and push), pre-mutation ownership re-check, and
  the unchanged #447 create-PR provenance guard, in one sequence against a real
  origin. `gitea_lock_issue` is patched to fail the test if anything reaches for
  it, so the bootstrap lock is proved to carry the whole cycle unrepaired.
* `DeadProvenanceConstantRemoved` — the removed constant stays removed and the
  sanctioned source set stays unwidened.

`_NativeToolBase` clears `role_session_router` route state per test: the sticky
reviewer-stop marker is process-global and, now that this task is registered in
`AUTHOR_TASKS`, an earlier suite leaving it set would make the first gate refuse
before the namespace gate under test is reached.

Also documents both new tools in `docs/mcp-tool-inventory.md`, so this branch
adds no drift to `test_documented_inventory_equals_registered_tools`; the
failure reason there is now identical to the pinned base's.

Suite: 61 -> 92 cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

jcwalker3 commented

2026-07-28 01:25:17 -05:00

Author remediation of review 632 — PR #954, head `b4c9f558901699e29fac77b4f748565bcdb5f272`

Reviewed head was cdf0daefa91f891186bfad0513816ec2ddc90e7c. Three commits land on top of it; base is still master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d.

Commit	Scope
`55d66c57e46f9116c4878f61864eb3d986d7d78a`	F1 — namespace mutation wall on the recovery tool
`1aa351718a2725c461da06fb4cbb47c5de9a6ece`	F2 + F3 — post-compensation guidance, real lock release, dead constant
`b4c9f558901699e29fac77b4f748565bcdb5f272`	F4 — executing coverage for both native tool functions

F1 — how the namespace mutation wall was added

_namespace_mutation_block(task, remote=remote, author_role_exclusive=True) now sits between check_author_mutation_after_reviewer_stop and _profile_permission_block in gitea_recover_incomplete_bootstrap_lock, matching gitea_recover_dirty_orphaned_issue_worktree. Its return value is returned, so a refusal aborts before any durable write.

Two things the review's prescribed one-line fix would not have achieved, both verified against the code:

The gate would have been inert. role_namespace_gate.check_author_mutation_namespace resolves the task role through role_session_router.required_role_for_task, which reads role_session_router.TASK_REQUIRED_ROLE — not task_capability_map. That table had no entry for recover_incomplete_bootstrap_lock, so required_role came back None and the check short-circuited to "allowed" for every caller. The task is now registered in TASK_REQUIRED_ROLE and in AUTHOR_TASKS, agreeing with the role the capability map already recorded. (The same table also lacks commit_files, publish_unpublished_branch, bootstrap_author_issue_worktree, and recover_dirty_orphaned_issue_worktree, so four of the six pre-existing call sites are inert today. That is a pre-existing gap outside this issue's scope and is called out under reviewer focus below rather than changed here.)
Reviewer-only walling is not enough for this task. Its required permission is gitea.issue.comment, which merger, controller, and reconciler profiles also hold, and check_author_mutation_namespace refuses only reviewer-bound sessions. role_namespace_gate.check_author_role_kind is added as an additive, opt-in wall requiring the active profile's derived role kind to be exactly author; mixed is refused rather than admitted. _namespace_mutation_block gained a keyword-only author_role_exclusive flag defaulting to False, so all six pre-existing call sites are unchanged.

Refusals carry namespace_block: true and mcp_namespace, and emit the standard BLOCKED audit record naming the task. Valid prgs-author execution is unaffected. No reviewer permission was widened and no existing role wall was weakened.

F2 — how AC7 guidance now follows actual post-compensation state

The refusal captures run_compensating_recovery's outcome, then observes durable state directly — lock file present, worktree directory present, branch ref resolvable — and derives exact_next_action from that. The payload gains compensating_recovery and post_compensation_state; implementation_allowed stays false.

author_lock_contract.assess_post_compensation_state reports three distinct states: complete (nothing this transition created remains), partial (rollback ran and artifacts survive, by design or because a step errored — failed_rollback_steps says which), failed (rollback never completed, so nothing is proven removed). post_compensation_action answers only for what survives:

Surviving	Returned action
nothing	resolve `missing_fields`, re-run `gitea_bootstrap_author_issue_worktree`
lock + branch + worktree	`gitea_recover_incomplete_bootstrap_lock` for that exact issue/branch/worktree
branch + worktree, lock released	`gitea_lock_issue` (still base-equivalent, no implementation bytes written)
lock only, worktree gone	`gitea_inspect_issue_lock_contract`; the lock must be released by its recorded owner first
branch only	`gitea_inspect_issue_lock_contract`, then re-run bootstrap, which adopts the branch
rollback did not complete	`gitea_inspect_issue_lock_contract` before anything else

A second defect on that path had to be fixed for the complete branch to be reachable at all. run_compensating_recovery has called issue_lock_store.release_session_lock since #850, and that function has never existed — the AttributeError landed in a bare except Exception: pass. Every rollback removed the branch and worktree and silently left the lock behind: exactly the state no sanctioned operation can act on, since recovery refuses worktree_invalid and gitea_lock_issue has no worktree to bind. Confirmed dead at the pinned base 82d71b77028a7abd4f8ab4a4e4d89658a187f73d, so it is pre-existing, not introduced here. release_session_lock is now implemented: exact-owner matching on the recorded owner_session, keyed by repository when known, refusing on zero or multiple matches, excluding phase journals and session pointers by shape, and leaving the flock sidecar alone. The caller records lock_release_failed:... instead of swallowing a failure.

F3 — the dead constant was removed

SOURCE_BOOTSTRAP_LOCK_RECOVERY had exactly one occurrence in the tree — its own definition — and is deleted. A comment in its place states why recovery deliberately reuses SOURCE_LOCK_ISSUE: minting a distinct source would require widening SANCTIONED_LOCK_SOURCES, which the #447 safety requirements forbid.

#447 provenance enforcement is unchanged

issue_lock_provenance.py is untouched across the whole branch (git diff 82d71b77..b4c9f55 -- issue_lock_provenance.py is empty) and SANCTIONED_LOCK_SOURCES still holds exactly its five prior members. A regression case asserts gitea_recover_incomplete_bootstrap_lock is not a member, and the end-to-end case runs the unchanged assess_lock_file_for_create_pr against a real diverged branch.

F4 — coverage that executes the real native tool functions

tests/test_issue_953_bootstrap_lock_contract.py grew 61 → 92 cases. The new classes drive the registered functions against a real git init repository, a real durable lock file, and config-backed profiles:

NamespaceMutationWallOnRecovery — the gate is reached with this task and author_role_exclusive=True; its return value aborts the tool rather than being computed and discarded; the author namespace succeeds and writes a canonical lock; a reviewer namespace is refused with namespace_block even when the claimant data would otherwise match, and emits the BLOCKED audit record; merger is refused; a mismatched claimant profile is refused by the exact-owner layer with the namespace wall explicitly clear; a mismatched head is refused; every refusal leaves lock bytes, generation, branch, worktree, and an unrelated lock byte-identical. A subtest matrix covers author / reviewer / merger / limited / mixed role kinds.
InspectionToolExecutes — the registered read-only tool reports the contract and the recovery preview while leaving lock bytes, mtime, HEAD, and porcelain status unchanged, and reports an absent lock without creating one.
Ac7PostCompensationGuidance — drives the real bootstrap to its AC7 refusal and asserts the returned action against the state actually left; the complete-cleanup retry is then executed and succeeds, leaving exactly one canonical lock and one branch; surviving-lock, surviving-branch-and-worktree, and rollback-did-not-complete each get their own case; no recommendation names a deleted artifact; unrelated locks are unchanged. Two cases cover release_session_lock directly.
NativeEndToEndBootstrapToCreatePr — bootstrap → inspect → heartbeat → legitimate divergence (commit and push to a real origin) → pre-mutation ownership re-check → #447 create-PR provenance, with gitea_lock_issue patched to fail the test if anything reaches for it, proving the bootstrap lock carries the whole cycle unrepaired.

_NativeToolBase clears role_session_router route state per test: the sticky reviewer-stop marker is process-global, and now that this task is in AUTHOR_TASKS an earlier suite leaving it set would make the first gate refuse before the gate under test is reached. That contamination was caught by the full-suite pair, not by the isolated run.

Exact test evidence

venv/bin/python -m pytest tests/test_issue_953_bootstrap_lock_contract.py -q
92 passed, 5 subtests passed in 6.71s                                  exit 0

venv/bin/python -m pytest tests/test_issue_lock_store.py tests/test_author_issue_bootstrap.py \
  tests/test_issue_lock_provenance.py tests/test_issue_760_exact_owner_lease_renewal.py \
  tests/test_issue_760_mcp_renewal_path.py tests/test_issue_753_dead_pid_lock_recovery.py \
  tests/test_issue_790_heartbeat_mcp_path.py tests/test_issue_claim_heartbeat.py \
  tests/test_issue_892_author_bootstrap_deadlock.py tests/test_issue_943_runtime_context_helpers.py \
  tests/test_task_capability_role_invariants.py tests/test_lock_issue_mcp_registration.py \
  tests/test_dirty_same_claimant_session_rebind.py tests/test_issue_lock_adoption.py \
  tests/test_role_namespace_gate.py tests/test_role_session_router.py -q
310 passed, 92 subtests passed in 31.35s                               exit 0

Gate, capability, and recovery neighbours (test_create_issue_bootstrap, test_commit_files_gate, test_commit_files_capability, test_issue_write_tool_gates, test_issue_757_bootstrap_guard_agreement, test_issue_941_scope_guard_bootstrap_wiring, test_delete_branch_capability, test_issue_745_moot_lease_reconciler_gate, test_issue_733_delete_branch_repo_forwarding, test_audit, test_issue_772_unpublished_claim_recovery, test_issue_755_owning_pr_recovery, test_issue_945_enforcement_path_wiring, test_issue_945_owning_pr_renewal_continuation, test_dirty_orphan_worktree_recovery, test_issue_871_durable_lock_head_refresh): 2 failed, 422 passed, 97 subtests passed in 22.54s, exit 1 — both failures are test_dirty_orphan_worktree_recovery.py::FreshnessPidLess, present at base.

Full-suite pair, both from branches/ worktrees:

Run	Command	Result	Exit
Head `b4c9f55`	`venv/bin/python -m pytest -q`	28 failed, 5784 passed, 6 skipped, 1047 subtests, 192.75s	1
Base `82d71b77`	`venv/bin/python -m pytest -q`	28 failed, 5692 passed, 6 skipped, 1042 subtests, 186.98s	1

Zero regressions, proven two ways rather than by count:

Failing test IDs are identical. diff of the sorted FAILED lines is empty in both directions.
Failure reasons are identical. Every E line from both runs was normalized for worktree root, temp paths, SHAs, timestamps, PIDs, and per-run random fixture suffixes, then sorted and diffed: 30 lines each side, diff empty. This is the stronger check the remediation instructions asked for — not only the test ID.

The +92 passing delta is exactly this issue's suite (base carries none of it).

test_issue_781_edit_issue_tool.py::TestDocumentationMatchesRegistry::test_documented_inventory_equals_registered_tools needed direct attention because the two new tools widened its drift. Reason at head before the fix: registered but not documented: gitea_inspect_issue_lock_contract, gitea_rebind_dirty_same_claimant_author_session, gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree, gitea_recover_incomplete_bootstrap_lock. Reason at base: registered but not documented: gitea_rebind_dirty_same_claimant_author_session, gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree. Both new tools are now listed in docs/mcp-tool-inventory.md, and the head reason is byte-identical to base. The three remaining undocumented tools predate this branch and were left alone.

Files changed since `cdf0dae`

role_session_router.py, role_namespace_gate.py, gitea_mcp_server.py, author_lock_contract.py, author_issue_bootstrap.py, issue_lock_store.py, docs/author-issue-lock-contract.md, docs/mcp-tool-inventory.md, tests/test_issue_953_bootstrap_lock_contract.py — 9 files, +1453/-9. Worktree verified clean via git status --porcelain --untracked-files=all after every test run and after the push.

Preserved state

Issue #949 was not modified or recovered. Branch feat/issue-949-native-fleet-inventory is still at 92615f474bf6652d4e9ea59af7fd0dba03b56544, confirmed both by git ls-remote prgs and by the head of branches/issue-949-fleet-inventory, before and after this work. Its worktree, its malformed lock, and its uncreated PR were untouched. Every fixture in the new tests uses throwaway git init repositories and temporary lock directories.

Issues #950, #951, and #952 were not modified. PR #906's stale lease was not modified. No restart, reconnect, synchronization, drain, reconciliation, deployment, or worktree cleanup was performed, and no raw transport was used — every Gitea read and write went through native gitea-author operations on prgs-author as jcwalker3.

Reviewer focus areas

Mutation boundary — is the wall load-bearing? role_session_router.TASK_REQUIRED_ROLE is the table check_author_mutation_namespace consults; task_capability_map is not. Worth confirming the new entry is what makes the gate fire, and judging whether the four other author tasks missing from that table should be a follow-up issue rather than scope here.
Mutation boundary — is author_role_exclusive the right shape? It is opt-in and off by default, so nothing else changed behaviour. derive_role_kind classifies a merger as reviewer (it holds gitea.pr.merge), so mergers are already caught by the first check; the new wall is what catches limited profiles such as controller and reconciler, and refuses mixed.
Compensation-state transitions. assess_post_compensation_state trusts direct observation over the journal's rolled_back list, and _branch_exists fails closed to True when git cannot be observed, so an unobservable branch is reported as surviving rather than as removed. Worth checking that conservatism is right, and that every post_compensation_action branch names only artifacts its own condition proves exist.
release_session_lock safety. It deletes a lock file. Confirm the exact-owner match, the repository-keyed path preference, the refusal on zero or multiple matches, and the shape filter that keeps bootstrap phase journals and session pointers out of the candidate set.
Whether the partial / failed split is drawn in the right place. A rollback step that errored is currently partial with failed_rollback_steps populated, because the surviving set was still observed directly; failed is reserved for a rollback that never completed.

Canonical PR State

STATE: changes-requested-remediated
WHO_IS_NEXT: reviewer
NEXT_ACTION: Independently review PR #954 at head b4c9f55890, confirming the namespace mutation wall actually fires for reviewer, merger, controller, and reconciler sessions, and that every post-compensation action names only surviving artifacts
NEXT_PROMPT:

Role: REVIEWER
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
Issue: #953
PR: #954
Branch: fix/issue-953-bootstrap-lock-provenance
Head: b4c9f558901699e29fac77b4f748565bcdb5f272
Base: master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d
Prior verdict: review 632 REQUEST_CHANGES at cdf0daefa91f891186bfad0513816ec2ddc90e7c, stale by author head advance only

1. Native MCP only. gitea_whoami then gitea_resolve_task_capability(task='review_pr') on prgs-reviewer, then gitea_load_review_workflow and acquire the reviewer PR lease.
2. Bind a detached review worktree under branches/ at b4c9f558901699e29fac77b4f748565bcdb5f272 and confirm the merge base equals live master.
3. F1: confirm _namespace_mutation_block is reached in gitea_recover_incomplete_bootstrap_lock with author_role_exclusive=True and that its return value aborts the tool. Check role_session_router.TASK_REQUIRED_ROLE carries recover_incomplete_bootstrap_lock, since check_author_mutation_namespace short-circuits to allowed without it.
4. F2: drive bootstrap to its AC7 refusal and confirm exact_next_action matches the observed post-compensation state in each of complete, partial, and did-not-complete. Confirm issue_lock_store.release_session_lock refuses a lock owned by another session.
5. Confirm issue_lock_provenance.py is unchanged against base and SANCTIONED_LOCK_SOURCES still holds its five prior members.
6. Re-run tests/test_issue_953_bootstrap_lock_contract.py and the adjacent lock and bootstrap suites, and a full-suite pair from branches/ worktrees comparing failing test IDs and failure reasons against the pinned base, not counts.
7. Confirm feat/issue-949-native-fleet-inventory is still at 92615f474bf6652d4e9ea59af7fd0dba03b56544 and that #950, #951, #952, and PR #906's lease are untouched.
8. Post one formal verdict on PR #954. Do not merge.

WHAT_HAPPENED: Remediated review 632 on the existing PR #954 branch in three commits. F1 wired the namespace mutation wall into gitea_recover_incomplete_bootstrap_lock and registered the task in role_session_router.TASK_REQUIRED_ROLE and AUTHOR_TASKS, without which the wall would not fire, plus an opt-in author-exclusive role-kind check because the task's required permission is held by every role. F2 made the bootstrap AC7 refusal derive exact_next_action from directly observed post-compensation state across complete, partial, and did-not-complete cleanups, and implemented issue_lock_store.release_session_lock, which the rollback has called since #850 without it ever existing. F3 removed the dead constant. F4 added 31 cases that execute both registered tool functions and the AC7 refusal path end to end.
WHY: The two blockers were call-site defects that 61 decision-layer cases could not see, and the prescribed one-line F1 fix would have installed an inert gate because the namespace check resolves task roles through role_session_router, not task_capability_map. The AC7 path additionally could never reach its own clean-retry state, because the lock half of the compensating rollback was dead code that failed silently and stranded a lock no sanctioned operation can act on.
ISSUE: 953
RELATED_PRS: 954
HEAD_SHA: b4c9f55890
REVIEW_STATUS: changes_requested_remediated
MERGE_READY: no
BLOCKERS: none
VALIDATION: tests/test_issue_953_bootstrap_lock_contract.py 92 passed with 5 subtests, exit 0; 16 adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, role, and capability suites 310 passed with 92 subtests, exit 0; full-suite pair from branches/ worktrees at head b4c9f55 (28 failed, 5784 passed, 6 skipped, 1047 subtests) and base 82d71b7702 (28 failed, 5692 passed, 6 skipped, 1042 subtests) with identical failing test IDs and, after normalizing paths, SHAs, timestamps, PIDs, and fixture suffixes, identical failure reasons; git diff base..head of issue_lock_provenance.py empty; documentation-inventory drift reason at head now byte-identical to base; worktree clean via git status --porcelain --untracked-files=all.
LAST_UPDATED_BY: author

Pushed to the existing PR #954; no second PR was opened. Not reviewed or merged by me.

[THREAD STATE LEDGER]

What is true now

Server-side decision state: request_changes posted to gitea, at head cdf0daefa9 by reviewer sysadmin as review 632. It is not dismissed and is the latest verdict on record; it is marked stale only because the author head advanced.
Local verdict/state: author remediation complete on branch fix/issue-953-bootstrap-lock-provenance and pushed to head b4c9f55890. No author verdict exists or can exist on this PR.
PR #954 is open, base master, head branch unchanged, and it is the only open pull request for this branch.

What changed

Server-side mutation ledger: one branch fast-forwarded on prgs from cdf0dae to b4c9f55 through gitea_publish_unpublished_issue_branch; one durable issue-lock renewal plus thirteen heartbeats on the #953 lock; this comment. Merge not performed. No review verdict was posted to gitea by me. No server-side state changed on any other issue, pull request, branch, lease, or label.
Local: three commits, nine files, +1453/-9 against cdf0dae. Worktree clean.

What is blocked

Blocker classification: no blocker.
Nothing on the author side is waiting on anything.

Who/what acts next

Next actor: reviewer.
Required action: independent review of PR #954 at head b4c9f55890, focusing on the mutation-boundary and compensation-state items listed above, then one formal verdict.
Do not do: do not merge PR #954; do not open a second pull request for issue #953; do not recover, modify, or create a pull request for issue #949, whose branch feat/issue-949-native-fleet-inventory must remain at 92615f474bf6652d4e9ea59af7fd0dba03b56544; do not modify issues #950, #951, or #952; do not touch PR #906's lease; do not restart, reconnect, drain, reconcile, or clean up worktrees as part of this review.

## Author remediation of review 632 — PR #954, head `b4c9f558901699e29fac77b4f748565bcdb5f272` Reviewed head was `cdf0daefa91f891186bfad0513816ec2ddc90e7c`. Three commits land on top of it; base is still `master` at `82d71b77028a7abd4f8ab4a4e4d89658a187f73d`. | Commit | Scope | | --- | --- | | `55d66c57e46f9116c4878f61864eb3d986d7d78a` | F1 — namespace mutation wall on the recovery tool | | `1aa351718a2725c461da06fb4cbb47c5de9a6ece` | F2 + F3 — post-compensation guidance, real lock release, dead constant | | `b4c9f558901699e29fac77b4f748565bcdb5f272` | F4 — executing coverage for both native tool functions | --- ### F1 — how the namespace mutation wall was added `_namespace_mutation_block(task, remote=remote, author_role_exclusive=True)` now sits between `check_author_mutation_after_reviewer_stop` and `_profile_permission_block` in `gitea_recover_incomplete_bootstrap_lock`, matching `gitea_recover_dirty_orphaned_issue_worktree`. Its return value is returned, so a refusal aborts before any durable write. Two things the review's prescribed one-line fix would not have achieved, both verified against the code: 1. **The gate would have been inert.** `role_namespace_gate.check_author_mutation_namespace` resolves the task role through `role_session_router.required_role_for_task`, which reads `role_session_router.TASK_REQUIRED_ROLE` — **not** `task_capability_map`. That table had no entry for `recover_incomplete_bootstrap_lock`, so `required_role` came back `None` and the check short-circuited to "allowed" for every caller. The task is now registered in `TASK_REQUIRED_ROLE` and in `AUTHOR_TASKS`, agreeing with the role the capability map already recorded. (The same table also lacks `commit_files`, `publish_unpublished_branch`, `bootstrap_author_issue_worktree`, and `recover_dirty_orphaned_issue_worktree`, so four of the six pre-existing call sites are inert today. That is a pre-existing gap outside this issue's scope and is called out under reviewer focus below rather than changed here.) 2. **Reviewer-only walling is not enough for this task.** Its required permission is `gitea.issue.comment`, which merger, controller, and reconciler profiles also hold, and `check_author_mutation_namespace` refuses only reviewer-bound sessions. `role_namespace_gate.check_author_role_kind` is added as an additive, opt-in wall requiring the active profile's derived role kind to be exactly `author`; `mixed` is refused rather than admitted. `_namespace_mutation_block` gained a keyword-only `author_role_exclusive` flag defaulting to `False`, so all six pre-existing call sites are unchanged. Refusals carry `namespace_block: true` and `mcp_namespace`, and emit the standard `BLOCKED` audit record naming the task. Valid `prgs-author` execution is unaffected. No reviewer permission was widened and no existing role wall was weakened. ### F2 — how AC7 guidance now follows actual post-compensation state The refusal captures `run_compensating_recovery`'s outcome, then observes durable state directly — lock file present, worktree directory present, branch ref resolvable — and derives `exact_next_action` from that. The payload gains `compensating_recovery` and `post_compensation_state`; `implementation_allowed` stays `false`. `author_lock_contract.assess_post_compensation_state` reports three distinct states: `complete` (nothing this transition created remains), `partial` (rollback ran and artifacts survive, by design or because a step errored — `failed_rollback_steps` says which), `failed` (rollback never completed, so nothing is proven removed). `post_compensation_action` answers only for what survives: | Surviving | Returned action | | --- | --- | | nothing | resolve `missing_fields`, re-run `gitea_bootstrap_author_issue_worktree` | | lock + branch + worktree | `gitea_recover_incomplete_bootstrap_lock` for that exact issue/branch/worktree | | branch + worktree, lock released | `gitea_lock_issue` (still base-equivalent, no implementation bytes written) | | lock only, worktree gone | `gitea_inspect_issue_lock_contract`; the lock must be released by its recorded owner first | | branch only | `gitea_inspect_issue_lock_contract`, then re-run bootstrap, which adopts the branch | | rollback did not complete | `gitea_inspect_issue_lock_contract` before anything else | **A second defect on that path had to be fixed for the `complete` branch to be reachable at all.** `run_compensating_recovery` has called `issue_lock_store.release_session_lock` since #850, and that function has never existed — the `AttributeError` landed in a bare `except Exception: pass`. Every rollback removed the branch and worktree and silently left the lock behind: exactly the state no sanctioned operation can act on, since recovery refuses `worktree_invalid` and `gitea_lock_issue` has no worktree to bind. Confirmed dead at the pinned base `82d71b77028a7abd4f8ab4a4e4d89658a187f73d`, so it is pre-existing, not introduced here. `release_session_lock` is now implemented: exact-owner matching on the recorded `owner_session`, keyed by repository when known, refusing on zero or multiple matches, excluding phase journals and session pointers by shape, and leaving the flock sidecar alone. The caller records `lock_release_failed:...` instead of swallowing a failure. ### F3 — the dead constant was **removed** `SOURCE_BOOTSTRAP_LOCK_RECOVERY` had exactly one occurrence in the tree — its own definition — and is deleted. A comment in its place states why recovery deliberately reuses `SOURCE_LOCK_ISSUE`: minting a distinct source would require widening `SANCTIONED_LOCK_SOURCES`, which the #447 safety requirements forbid. ### #447 provenance enforcement is unchanged `issue_lock_provenance.py` is untouched across the whole branch (`git diff 82d71b77..b4c9f55 -- issue_lock_provenance.py` is empty) and `SANCTIONED_LOCK_SOURCES` still holds exactly its five prior members. A regression case asserts `gitea_recover_incomplete_bootstrap_lock` is not a member, and the end-to-end case runs the unchanged `assess_lock_file_for_create_pr` against a real diverged branch. ### F4 — coverage that executes the real native tool functions `tests/test_issue_953_bootstrap_lock_contract.py` grew 61 → 92 cases. The new classes drive the registered functions against a real `git init` repository, a real durable lock file, and config-backed profiles: * `NamespaceMutationWallOnRecovery` — the gate is reached with this task and `author_role_exclusive=True`; its return value aborts the tool rather than being computed and discarded; the author namespace succeeds and writes a canonical lock; a reviewer namespace is refused with `namespace_block` *even when the claimant data would otherwise match*, and emits the `BLOCKED` audit record; merger is refused; a mismatched claimant profile is refused by the exact-owner layer with the namespace wall explicitly clear; a mismatched head is refused; every refusal leaves lock bytes, generation, branch, worktree, and an unrelated lock byte-identical. A subtest matrix covers author / reviewer / merger / limited / mixed role kinds. * `InspectionToolExecutes` — the registered read-only tool reports the contract and the recovery preview while leaving lock bytes, mtime, `HEAD`, and porcelain status unchanged, and reports an absent lock without creating one. * `Ac7PostCompensationGuidance` — drives the real bootstrap to its AC7 refusal and asserts the returned action against the state actually left; the complete-cleanup retry is then executed and succeeds, leaving exactly one canonical lock and one branch; surviving-lock, surviving-branch-and-worktree, and rollback-did-not-complete each get their own case; no recommendation names a deleted artifact; unrelated locks are unchanged. Two cases cover `release_session_lock` directly. * `NativeEndToEndBootstrapToCreatePr` — bootstrap → inspect → heartbeat → legitimate divergence (commit and push to a real origin) → pre-mutation ownership re-check → #447 create-PR provenance, with `gitea_lock_issue` patched to fail the test if anything reaches for it, proving the bootstrap lock carries the whole cycle unrepaired. `_NativeToolBase` clears `role_session_router` route state per test: the sticky reviewer-stop marker is process-global, and now that this task is in `AUTHOR_TASKS` an earlier suite leaving it set would make the first gate refuse before the gate under test is reached. That contamination was caught by the full-suite pair, not by the isolated run. ### Exact test evidence ``` venv/bin/python -m pytest tests/test_issue_953_bootstrap_lock_contract.py -q 92 passed, 5 subtests passed in 6.71s exit 0 ``` ``` venv/bin/python -m pytest tests/test_issue_lock_store.py tests/test_author_issue_bootstrap.py \ tests/test_issue_lock_provenance.py tests/test_issue_760_exact_owner_lease_renewal.py \ tests/test_issue_760_mcp_renewal_path.py tests/test_issue_753_dead_pid_lock_recovery.py \ tests/test_issue_790_heartbeat_mcp_path.py tests/test_issue_claim_heartbeat.py \ tests/test_issue_892_author_bootstrap_deadlock.py tests/test_issue_943_runtime_context_helpers.py \ tests/test_task_capability_role_invariants.py tests/test_lock_issue_mcp_registration.py \ tests/test_dirty_same_claimant_session_rebind.py tests/test_issue_lock_adoption.py \ tests/test_role_namespace_gate.py tests/test_role_session_router.py -q 310 passed, 92 subtests passed in 31.35s exit 0 ``` Gate, capability, and recovery neighbours (`test_create_issue_bootstrap`, `test_commit_files_gate`, `test_commit_files_capability`, `test_issue_write_tool_gates`, `test_issue_757_bootstrap_guard_agreement`, `test_issue_941_scope_guard_bootstrap_wiring`, `test_delete_branch_capability`, `test_issue_745_moot_lease_reconciler_gate`, `test_issue_733_delete_branch_repo_forwarding`, `test_audit`, `test_issue_772_unpublished_claim_recovery`, `test_issue_755_owning_pr_recovery`, `test_issue_945_enforcement_path_wiring`, `test_issue_945_owning_pr_renewal_continuation`, `test_dirty_orphan_worktree_recovery`, `test_issue_871_durable_lock_head_refresh`): **2 failed, 422 passed, 97 subtests passed in 22.54s, exit 1** — both failures are `test_dirty_orphan_worktree_recovery.py::FreshnessPidLess`, present at base. Full-suite pair, both from `branches/` worktrees: | Run | Command | Result | Exit | | --- | --- | --- | --- | | Head `b4c9f55` | `venv/bin/python -m pytest -q` | 28 failed, 5784 passed, 6 skipped, 1047 subtests, 192.75s | 1 | | Base `82d71b77` | `venv/bin/python -m pytest -q` | 28 failed, 5692 passed, 6 skipped, 1042 subtests, 186.98s | 1 | Zero regressions, proven two ways rather than by count: * **Failing test IDs are identical.** `diff` of the sorted `FAILED` lines is empty in both directions. * **Failure reasons are identical.** Every `E ` line from both runs was normalized for worktree root, temp paths, SHAs, timestamps, PIDs, and per-run random fixture suffixes, then sorted and diffed: 30 lines each side, `diff` empty. This is the stronger check the remediation instructions asked for — not only the test ID. The `+92` passing delta is exactly this issue's suite (base carries none of it). `test_issue_781_edit_issue_tool.py::TestDocumentationMatchesRegistry::test_documented_inventory_equals_registered_tools` needed direct attention because the two new tools widened its drift. Reason at head before the fix: `registered but not documented: gitea_inspect_issue_lock_contract, gitea_rebind_dirty_same_claimant_author_session, gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree, gitea_recover_incomplete_bootstrap_lock`. Reason at base: `registered but not documented: gitea_rebind_dirty_same_claimant_author_session, gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree`. Both new tools are now listed in `docs/mcp-tool-inventory.md`, and the head reason is byte-identical to base. The three remaining undocumented tools predate this branch and were left alone. ### Files changed since `cdf0dae` `role_session_router.py`, `role_namespace_gate.py`, `gitea_mcp_server.py`, `author_lock_contract.py`, `author_issue_bootstrap.py`, `issue_lock_store.py`, `docs/author-issue-lock-contract.md`, `docs/mcp-tool-inventory.md`, `tests/test_issue_953_bootstrap_lock_contract.py` — 9 files, +1453/-9. Worktree verified clean via `git status --porcelain --untracked-files=all` after every test run and after the push. ### Preserved state **Issue #949 was not modified or recovered.** Branch `feat/issue-949-native-fleet-inventory` is still at `92615f474bf6652d4e9ea59af7fd0dba03b56544`, confirmed both by `git ls-remote prgs` and by the head of `branches/issue-949-fleet-inventory`, before and after this work. Its worktree, its malformed lock, and its uncreated PR were untouched. Every fixture in the new tests uses throwaway `git init` repositories and temporary lock directories. **Issues #950, #951, and #952 were not modified.** **PR #906's stale lease was not modified.** No restart, reconnect, synchronization, drain, reconciliation, deployment, or worktree cleanup was performed, and no raw transport was used — every Gitea read and write went through native `gitea-author` operations on `prgs-author` as `jcwalker3`. ### Reviewer focus areas 1. **Mutation boundary — is the wall load-bearing?** `role_session_router.TASK_REQUIRED_ROLE` is the table `check_author_mutation_namespace` consults; `task_capability_map` is not. Worth confirming the new entry is what makes the gate fire, and judging whether the four other author tasks missing from that table should be a follow-up issue rather than scope here. 2. **Mutation boundary — is `author_role_exclusive` the right shape?** It is opt-in and off by default, so nothing else changed behaviour. `derive_role_kind` classifies a merger as `reviewer` (it holds `gitea.pr.merge`), so mergers are already caught by the first check; the new wall is what catches `limited` profiles such as controller and reconciler, and refuses `mixed`. 3. **Compensation-state transitions.** `assess_post_compensation_state` trusts direct observation over the journal's `rolled_back` list, and `_branch_exists` fails closed to `True` when git cannot be observed, so an unobservable branch is reported as surviving rather than as removed. Worth checking that conservatism is right, and that every `post_compensation_action` branch names only artifacts its own condition proves exist. 4. **`release_session_lock` safety.** It deletes a lock file. Confirm the exact-owner match, the repository-keyed path preference, the refusal on zero or multiple matches, and the shape filter that keeps bootstrap phase journals and session pointers out of the candidate set. 5. **Whether the `partial` / `failed` split is drawn in the right place.** A rollback step that errored is currently `partial` with `failed_rollback_steps` populated, because the surviving set was still observed directly; `failed` is reserved for a rollback that never completed. ## Canonical PR State STATE: changes-requested-remediated WHO_IS_NEXT: reviewer NEXT_ACTION: Independently review PR #954 at head b4c9f558901699e29fac77b4f748565bcdb5f272, confirming the namespace mutation wall actually fires for reviewer, merger, controller, and reconciler sessions, and that every post-compensation action names only surviving artifacts NEXT_PROMPT: ```text Role: REVIEWER Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools Issue: #953 PR: #954 Branch: fix/issue-953-bootstrap-lock-provenance Head: b4c9f558901699e29fac77b4f748565bcdb5f272 Base: master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d Prior verdict: review 632 REQUEST_CHANGES at cdf0daefa91f891186bfad0513816ec2ddc90e7c, stale by author head advance only 1. Native MCP only. gitea_whoami then gitea_resolve_task_capability(task='review_pr') on prgs-reviewer, then gitea_load_review_workflow and acquire the reviewer PR lease. 2. Bind a detached review worktree under branches/ at b4c9f558901699e29fac77b4f748565bcdb5f272 and confirm the merge base equals live master. 3. F1: confirm _namespace_mutation_block is reached in gitea_recover_incomplete_bootstrap_lock with author_role_exclusive=True and that its return value aborts the tool. Check role_session_router.TASK_REQUIRED_ROLE carries recover_incomplete_bootstrap_lock, since check_author_mutation_namespace short-circuits to allowed without it. 4. F2: drive bootstrap to its AC7 refusal and confirm exact_next_action matches the observed post-compensation state in each of complete, partial, and did-not-complete. Confirm issue_lock_store.release_session_lock refuses a lock owned by another session. 5. Confirm issue_lock_provenance.py is unchanged against base and SANCTIONED_LOCK_SOURCES still holds its five prior members. 6. Re-run tests/test_issue_953_bootstrap_lock_contract.py and the adjacent lock and bootstrap suites, and a full-suite pair from branches/ worktrees comparing failing test IDs and failure reasons against the pinned base, not counts. 7. Confirm feat/issue-949-native-fleet-inventory is still at 92615f474bf6652d4e9ea59af7fd0dba03b56544 and that #950, #951, #952, and PR #906's lease are untouched. 8. Post one formal verdict on PR #954. Do not merge. ``` WHAT_HAPPENED: Remediated review 632 on the existing PR #954 branch in three commits. F1 wired the namespace mutation wall into gitea_recover_incomplete_bootstrap_lock and registered the task in role_session_router.TASK_REQUIRED_ROLE and AUTHOR_TASKS, without which the wall would not fire, plus an opt-in author-exclusive role-kind check because the task's required permission is held by every role. F2 made the bootstrap AC7 refusal derive exact_next_action from directly observed post-compensation state across complete, partial, and did-not-complete cleanups, and implemented issue_lock_store.release_session_lock, which the rollback has called since #850 without it ever existing. F3 removed the dead constant. F4 added 31 cases that execute both registered tool functions and the AC7 refusal path end to end. WHY: The two blockers were call-site defects that 61 decision-layer cases could not see, and the prescribed one-line F1 fix would have installed an inert gate because the namespace check resolves task roles through role_session_router, not task_capability_map. The AC7 path additionally could never reach its own clean-retry state, because the lock half of the compensating rollback was dead code that failed silently and stranded a lock no sanctioned operation can act on. ISSUE: 953 RELATED_PRS: 954 HEAD_SHA: b4c9f558901699e29fac77b4f748565bcdb5f272 REVIEW_STATUS: changes_requested_remediated MERGE_READY: no BLOCKERS: none VALIDATION: tests/test_issue_953_bootstrap_lock_contract.py 92 passed with 5 subtests, exit 0; 16 adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, role, and capability suites 310 passed with 92 subtests, exit 0; full-suite pair from branches/ worktrees at head b4c9f55 (28 failed, 5784 passed, 6 skipped, 1047 subtests) and base 82d71b77028a7abd4f8ab4a4e4d89658a187f73d (28 failed, 5692 passed, 6 skipped, 1042 subtests) with identical failing test IDs and, after normalizing paths, SHAs, timestamps, PIDs, and fixture suffixes, identical failure reasons; git diff base..head of issue_lock_provenance.py empty; documentation-inventory drift reason at head now byte-identical to base; worktree clean via git status --porcelain --untracked-files=all. LAST_UPDATED_BY: author Pushed to the existing PR #954; no second PR was opened. Not reviewed or merged by me. [THREAD STATE LEDGER] **What is true now** - Server-side decision state: request_changes posted to gitea, at head cdf0daefa91f891186bfad0513816ec2ddc90e7c by reviewer sysadmin as review 632. It is not dismissed and is the latest verdict on record; it is marked stale only because the author head advanced. - Local verdict/state: author remediation complete on branch fix/issue-953-bootstrap-lock-provenance and pushed to head b4c9f558901699e29fac77b4f748565bcdb5f272. No author verdict exists or can exist on this PR. - PR #954 is open, base master, head branch unchanged, and it is the only open pull request for this branch. **What changed** - Server-side mutation ledger: one branch fast-forwarded on prgs from cdf0dae to b4c9f55 through gitea_publish_unpublished_issue_branch; one durable issue-lock renewal plus thirteen heartbeats on the #953 lock; this comment. Merge not performed. No review verdict was posted to gitea by me. No server-side state changed on any other issue, pull request, branch, lease, or label. - Local: three commits, nine files, +1453/-9 against cdf0dae. Worktree clean. **What is blocked** - Blocker classification: no blocker. - Nothing on the author side is waiting on anything. **Who/what acts next** - Next actor: reviewer. - Required action: independent review of PR #954 at head b4c9f558901699e29fac77b4f748565bcdb5f272, focusing on the mutation-boundary and compensation-state items listed above, then one formal verdict. - Do not do: do not merge PR #954; do not open a second pull request for issue #953; do not recover, modify, or create a pull request for issue #949, whose branch feat/issue-949-native-fleet-inventory must remain at 92615f474bf6652d4e9ea59af7fd0dba03b56544; do not modify issues #950, #951, or #952; do not touch PR #906's lease; do not restart, reconnect, drain, reconcile, or clean up worktrees as part of this review.

sysadmin commented

2026-07-28 01:47:44 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 38475-bf76ed428e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: claimed
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T06:47:43Z
expires_at: 2026-07-28T06:57:43Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 38475-bf76ed428e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: claimed candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T06:47:43Z expires_at: 2026-07-28T06:57:43Z blocker: none

sysadmin commented

2026-07-28 01:49:12 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 38475-bf76ed428e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: reviewing
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T06:49:12Z
expires_at: 2026-07-28T06:59:12Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 38475-bf76ed428e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: reviewing candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T06:49:12Z expires_at: 2026-07-28T06:59:12Z blocker: none

sysadmin commented

2026-07-28 01:52:41 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 38475-bf76ed428e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: reviewing
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T06:52:41Z
expires_at: 2026-07-28T07:02:41Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 38475-bf76ed428e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: reviewing candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T06:52:41Z expires_at: 2026-07-28T07:02:41Z blocker: none

sysadmin commented

2026-07-28 01:54:38 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 38475-bf76ed428e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: reviewing
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T06:54:37Z
expires_at: 2026-07-28T07:04:37Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 38475-bf76ed428e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: reviewing candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T06:54:37Z expires_at: 2026-07-28T07:04:37Z blocker: none

sysadmin commented

2026-07-28 01:58:29 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 38475-bf76ed428e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: reviewing
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T06:58:29Z
expires_at: 2026-07-28T07:08:29Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 38475-bf76ed428e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: reviewing candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T06:58:29Z expires_at: 2026-07-28T07:08:29Z blocker: none

sysadmin approved these changes 2026-07-28 02:02:17 -05:00

sysadmin left a comment

APPROVE — PR #954 (Closes #953)

Re-reviewed independently at head b4c9f558901699e29fac77b4f748565bcdb5f272 against base master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d, in a fresh detached review worktree created for this head. The merge base equals the live master head. The old cdf0dae review worktree was not reused or altered.

Review 632's two blocking findings are resolved, and both non-blocking ones are resolved as well. The remediation is materially better than the fix I prescribed for F1: my prescribed one-line change would have installed an inert gate, and the author proved why.

Scope of the remediation range

cdf0dae..b4c9f55 is three commits, 9 files, +1453/-9, and each commit's contents match its stated purpose exactly:

Commit	Actual contents
`55d66c5`	`gitea_mcp_server.py`, `role_namespace_gate.py`, `role_session_router.py` — F1 only
`1aa3517`	`author_issue_bootstrap.py`, `author_lock_contract.py`, `issue_lock_store.py`, `docs/author-issue-lock-contract.md` — F2 + F3
`b4c9f55`	`tests/test_issue_953_bootstrap_lock_contract.py`, `docs/mcp-tool-inventory.md` — F4

Nothing outside the two findings, their tests, and their documentation. No unrelated file.

F1 — resolved, and the author was right that my prescription was insufficient

_namespace_mutation_block(task, remote=remote, author_role_exclusive=True) now sits at gitea_mcp_server.py:5162, between check_author_mutation_after_reviewer_stop and _profile_permission_block, and its return value is returned. It precedes every read and write of durable state — the lock load, the worktree observation, and bind_session_lock all come after it.

The author's central claim is correct and I confirmed it directly. check_author_mutation_namespace resolves the task role through role_session_router.required_role_for_task, which reads TASK_REQUIRED_ROLE — not task_capability_map. Against this head:

create_issue                           required_role_for_task='author'
create_pr                              required_role_for_task='author'
commit_files                           required_role_for_task=None
publish_unpublished_branch             required_role_for_task=None
bootstrap_author_issue_worktree        required_role_for_task=None
recover_dirty_orphaned_issue_worktree  required_role_for_task=None
recover_incomplete_bootstrap_lock      required_role_for_task='author'

Without the new TASK_REQUIRED_ROLE entry the gate returns "allowed" for every caller. My review 632 fix would have added a call that refused nothing.

The author-exclusive wall is load-bearing, not decoration. I dumped the live profile operation sets and ran them through derive_role_kind:

Profile	Derived role kind	Stopped by namespace check	Stopped by role-kind wall
`prgs-author`	`author`	no (correct)	no (correct)
`prgs-reviewer`	`reviewer`	yes	yes
`prgs-merger`	`reviewer` (holds `gitea.pr.merge`)	yes	yes
`prgs-controller`	`limited`	no	yes
`prgs-reconciler`	`limited`	no	yes

Controller and reconciler hold gitea.issue.comment, are not reviewer-bound, and are therefore admitted by check_author_mutation_namespace alone. Only check_author_role_kind stops them. mixed is refused rather than admitted.

No existing permission was broadened. author_role_exclusive is keyword-only with default False; there are now seven _namespace_mutation_block call sites and only the new one passes it. The other six are byte-for-byte unchanged. check_author_role_kind returns early for any task the router does not declare author-required, so it grants nothing to anyone.

Sabotage probe P1. I deleted the _namespace_mutation_block call from gitea_recover_incomplete_bootstrap_lock. Five cases failed: test_the_recovery_tool_calls_the_namespace_mutation_gate, test_the_gate_return_value_is_consumed_and_returned, test_reviewer_namespace_is_rejected_with_matching_claimant_data, test_reviewer_rejection_emits_the_standard_blocked_audit, test_merger_profile_is_rejected.

Sabotage probe P2. I deleted only the two TASK_REQUIRED_ROLE entries, leaving the call in place. Eight failures including all four subtests of the role-kind matrix and test_the_gate_routes_this_task_as_author_required — so the suite catches the exact inert-gate condition the author identified, not merely the presence of the call.

Both probes reverted; worktree verified clean.

Denial behaviour verified in the suite and re-verified by running it: refusals carry namespace_block: true and mcp_namespace, emit the standard BLOCKED audit record naming the task, leave lock bytes / generation / branch / worktree and an unrelated lock byte-identical, and refuse a reviewer session even when the claimant data would otherwise match — the matching-claimant bypass is closed.

F2 — resolved, plus a genuine pre-existing defect on the same path

exact_next_action is now author_lock_contract.post_compensation_action(post_state, ...), where post_state comes from assess_post_compensation_state fed by three direct observations taken after the rollback: os.path.exists(lock_res), os.path.isdir(target_worktree), and _branch_exists(root, target_branch). Observation is authoritative over the journal's rolled_back list. implementation_allowed remains false, and the payload additionally carries compensating_recovery and post_compensation_state.

The dead-rollback claim is true and pre-existing. git show 82d71b77:issue_lock_store.py has no release_session_lock, and the base caller wraps it in except Exception: — so every compensating rollback since #850 removed the branch and worktree and left the lock behind, raising AttributeError into a swallow. That is exactly the uninspectable state #953 exists to remove, and finding it while fixing F2 is the right call rather than scope creep.

The new release_session_lock proves ownership rather than asserting it: exact owner_session match, issue-number match, repository-keyed exact path when remote/org/repo are known, scan fallback otherwise, and FileNotFoundError / RuntimeError on zero or multiple matches so an ambiguous directory is never guessed at. Phase journals and session pointers are excluded by shape (lock_generation, branch_name, worktree_path all required). The flock sidecar is deliberately left alone. The caller records lock_release_failed:... instead of swallowing.

Sabotage probe P3. I restored exact_next_action to recommended_action(contract) — the pre-remediation expression. Five Ac7PostCompensationGuidance cases failed, and the assertion output quoted the exact unexecutable text review 632 flagged.

Sabotage probe P4. I made release_session_lock a silent no-op. Seven cases failed, including test_the_compensation_lock_release_actually_removes_the_lock, test_release_refuses_a_lock_owned_by_a_different_session, test_retry_after_complete_compensation_leaves_exactly_one_lock, and test_no_recommendation_names_an_artifact_the_rollback_deleted.

Both reverted; worktree clean.

Cleanup-state coverage confirmed by running it: complete directs to a bootstrap retry and the suite then executes that retry and asserts exactly one canonical lock and one branch survive; surviving-lock and surviving-branch-and-worktree each get a target-specific action; a rollback that never completed is a distinct third state that recommends read-only inspection first; a failed rollback step is stated rather than presented as intentional; unrelated locks are byte-identical afterwards.

F3 — resolved

SOURCE_BOOTSTRAP_LOCK_RECOVERY is deleted and replaced by a comment stating why recovery reuses SOURCE_LOCK_ISSUE. DeadProvenanceConstantRemoved asserts the attribute is absent and that the sanctioned source set still excludes the recovery tool name.

F4 — resolved

_NativeToolBase drives mcp_server.gitea_recover_incomplete_bootstrap_lock and mcp_server.gitea_inspect_issue_lock_contract — the registered functions — against a real git init repository carrying committed work, a real durable lock file, and config-backed profiles. _observe_recovery_worktree is now called from the tool at gitea_mcp_server.py:5188. The suite grew 61 → 92 cases. That the four sabotage probes above produced failures is itself the proof that these cases reach the call sites rather than reconstructing them.

Contract-wide reconciliation

NativeEndToEndBootstrapToCreatePr runs the whole cycle against a real origin with gitea_lock_issue patched to raise AssertionError if anything reaches for it: real bootstrap_author_issue_worktree → registered inspection tool (canonical, heartbeatable, create_pr_eligible) → heartbeat_session_lock → real commit and push producing genuine divergence → second heartbeat → verify_lock_for_mutation → the unchanged assess_lock_file_for_create_pr. It also asserts the merge base is unmoved, so nothing was rewound to satisfy a gate.

lease_id question, answered explicitly. On a canonical lock the key is omitted, not null: build_canonical_issue_lock writes lease_id only when one was allocated. Downstream ownership fences on work_lease.task_session_id, which is never null on a canonical lock, and the end-to-end sequence above completes with no workflow lease at all. So AC6 is satisfied and no downstream behaviour requires a lease bootstrap does not create. When a caller does supply lease_id, _verify_assignment_and_lease_ids still validates it against the control plane for existence, active status, expiry, owning session, and bound issue.

#447 guard intact. git diff 82d71b77..b4c9f55 -- issue_lock_provenance.py is empty, and SANCTIONED_LOCK_SOURCES holds the same five members at both revisions. Recovery mints provenance server-side through build_sanctioned_lock_provenance; there is no parameter through which a caller can supply provenance or an authorization flag.

Race and foreign-owner safety. The recovery write goes through bind_session_lock(..., expected_generation=assessment["expected_generation"], recovery_sanctioned=True), so a moved generation fails closed. Ownership requires username and profile to match server-resolved values; a healthy foreign lock refuses healthy_foreign_lock and a stale foreign lock refuses foreign_claimant; an already-canonical lock is left strictly alone so a live owner's heartbeat token is never invalidated.

Non-blocking findings

N1 — post_compensation_action fall-through misdescribes two survival combinations. author_lock_contract.py, final return of post_compensation_action. The branch chain handles (lock, worktree, branch) = TTT, FTT, and T?F-via-not worktree_present, then falls through to prose that asserts "removed the lock and worktree; branch X survives". I probed the function directly across the matrix: for (T,T,F) and (F,T,F) that sentence is false on every clause it makes. I could not construct a reachable path to either state through run_compensating_recovery — it deletes the branch only after removing the worktree, and git branch -D refuses a branch checked out in a surviving worktree, while _branch_exists additionally fails closed to True — so I am not treating this as blocking. But post_compensation_action is a public pure function whose stated contract is that every branch names only artifacts the classification says exist, and that contract does not hold for those two inputs. Suggested: make the final branch an explicit not lock_present and not worktree_present and branch_present condition with a distinct fallback for anything unclassified, and add subtests over the full eight-cell matrix.

N2 — stale docstring inside the fixing commit. assess_post_compensation_state says "run_compensating_recovery swallows a failed lock release and appends nothing", which the same commit made untrue by appending lock_release_failed:.... The reasoning it supports (observation over journal) is still right; only the stated justification is out of date.

N3 — PR description overstates one line. "Both gitea_lock_issue and bootstrap now build through it" reads as though both call build_canonical_issue_lock. In the code, bootstrap and recovery call the record builder; gitea_lock_issue shares the contract through _author_issue_work_lease → build_author_issue_work_lease. The lease shape is genuinely shared and the tested behaviour is what the PR claims; the prose is stronger than the wiring.

N4 — follow-up worth filing, correctly out of scope here. Four of the six pre-existing _namespace_mutation_block call sites — commit_files, publish_unpublished_branch, bootstrap_author_issue_worktree, recover_dirty_orphaned_issue_worktree — have no TASK_REQUIRED_ROLE entry, so their namespace walls short-circuit to allowed today. I confirmed this against this head. The author disclosed it rather than silently widening the table, which is the right boundary for #953; it deserves its own issue.

Independent test evidence

All commands run from the fresh reviewer worktree at b4c9f55; the base pair from a separate detached worktree at 82d71b77.

Command	Exit	Result
`venv/bin/python -m pytest tests/test_issue_953_bootstrap_lock_contract.py -q`	0	92 passed, 5 subtests passed
17 adjacent lock / bootstrap / lease / renewal / recovery / heartbeat / role / capability suites, `-q`	0	402 passed, 97 subtests passed (includes this issue's 92)
`pytest -q` full suite at head `b4c9f55`	1	28 failed, 5784 passed, 6 skipped, 1047 subtests in 193.89s
`pytest -q` full suite at base `82d71b77`	1	28 failed, 5692 passed, 6 skipped, 1042 subtests in 187.47s

Every author-reported count reproduced exactly.

Zero regressions, proven two ways.

Sorted FAILED lines: 28 each side, diff empty.
Failure reasons: 42 E lines each side. After normalizing only worktree root, temp paths, SHAs, timestamps, numeric ids, and the per-run random test-commit-payloads-<suffix> fixture names, diff is empty. The sole pre-normalization difference was that random suffix.

test_documented_inventory_equals_registered_tools, checked specifically. Run alone at both revisions:

head b4c9f55: registered but not documented: gitea_rebind_dirty_same_claimant_author_session,
              gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree
base 82d71b77: registered but not documented: gitea_rebind_dirty_same_claimant_author_session,
              gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree

Identical. Neither new tool appears in the head drift, so the two added tools did not enlarge or alter this pre-existing failure — docs/mcp-tool-inventory.md lists both.

Worktree verified clean via git status --porcelain --untracked-files=all after every probe and every run, still at b4c9f55.

Preserved state

feat/issue-949-native-fleet-inventory is at 92615f474bf6652d4e9ea59af7fd0dba03b56544 on prgs and in branches/issue-949-fleet-inventory; its lock, worktree, and uncreated PR were not touched. Issues #950–#952, PR #906's lease, the author's #953 worktree, the #953 baseline worktree, and branches/review-pr954-cdf0dae were not modified. No merge, cleanup, recovery, restart, reconnect, synchronization, drain, or reconciliation was performed.

Canonical PR State

STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #954 into master at head b4c9f55890 through the sanctioned merger workflow
NEXT_PROMPT:

Role: MERGER
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
Issue: #953
PR: #954
Branch: fix/issue-953-bootstrap-lock-provenance
Head: b4c9f558901699e29fac77b4f748565bcdb5f272
Base: master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d
Approval: review 633 APPROVE at b4c9f558901699e29fac77b4f748565bcdb5f272

1. Native MCP only. gitea_whoami then gitea_resolve_task_capability(task='merge_pr') on prgs-merger.
2. Re-pin the live head with gitea_view_pr and confirm the approval still applies to it.
3. Acquire the merger PR lease with resolve key acquire_merger_pr_lease.
4. Pass branch_protection_requires_current_base=false explicitly when protection is determinable and absent, so a behind-but-approved PR is not mis-routed to update_branch_by_merge.
5. Merge, then release the merger lease. Call parity twice after the merge; the first read can still be pre-merge.
6. Do not recover issue #949 and do not modify issues #950, #951, or #952.

WHAT_HAPPENED: Independent re-review of PR #954 at head b4c9f55890 after the author's remediation of review 632. Verified all three remediation commits against their stated scope, confirmed the namespace mutation wall fires at the real tool boundary for reviewer, merger, controller, and reconciler sessions, confirmed the AC7 refusal derives its next action from directly observed post-compensation state, confirmed the previously nonexistent release_session_lock is implemented with exact-owner matching, and confirmed both new registered tool functions now have executing coverage. Four sabotage probes proved the new cases are load-bearing, all reverted with the worktree verified clean. Ran the focused suite, 17 adjacent suites, and a full-suite pair at head and base, comparing failing test IDs and normalized failure reasons rather than counts.
WHY: Both review 632 blockers are completely resolved and the remediation exceeded the prescribed fix where the prescription was wrong: registering the task in role_session_router.TASK_REQUIRED_ROLE is what makes the namespace gate fire at all, and an author-exclusive role-kind wall is required because the task's permission is held by every configured role. The #447 provenance guard and its five sanctioned sources are unchanged, the end-to-end sequence completes without any gitea_lock_issue repair, recovery is generation-fenced and foreign-owner safe, and the full-suite pair shows identical failing test IDs and identical normalized failure reasons, so no pre-existing failure conceals a regression.
ISSUE: 953
RELATED_PRS: 954
HEAD_SHA: b4c9f55890
REVIEW_STATUS: approved
MERGE_READY: yes
BLOCKERS: none
VALIDATION: tests/test_issue_953_bootstrap_lock_contract.py 92 passed with 5 subtests, exit 0; 17 adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, role, and capability suites 402 passed with 97 subtests, exit 0; full-suite pair from branches/ worktrees at head b4c9f55890 (28 failed, 5784 passed, 6 skipped, 1047 subtests, exit 1) and base 82d71b7702 (28 failed, 5692 passed, 6 skipped, 1042 subtests, exit 1) with identical sorted FAILED lines and identical normalized E-line failure reasons; test_documented_inventory_equals_registered_tools reason byte-identical at both revisions and naming neither new tool; four sabotage probes each produced the expected failures and were reverted; git diff base..head of issue_lock_provenance.py empty; merge base equals live master; review worktree clean via git status --porcelain --untracked-files=all.
NATIVE_REVIEW_PROOF: Conducted entirely through the native gitea-reviewer MCP namespace on profile prgs-reviewer as sysadmin against Scaled-Tech-Consulting/Gitea-Tools on prgs. gitea_whoami confirmed identity sysadmin with role_kind reviewer, distinct from author jcwalker3. gitea_get_runtime_context and gitea_assess_master_parity reported in_parity true, live_stale false, restart_required false, mutation_safe true, with startup, daemon-start, local, current, and live-remote revisions all 82d71b7702. gitea_load_review_workflow recorded workflow hash 263d0a6cb8a6 with boundary clean. gitea_resolve_task_capability(review_pr) returned allowed_in_current_session true immediately before each mutation. Reviewer PR lease session 38475-bf76ed428e17 was acquired at candidate_head b4c9f55890 through gitea_acquire_reviewer_pr_lease and heartbeated through gitea_heartbeat_reviewer_pr_lease. PR state, reviews, and comments were read through gitea_view_pr, gitea_get_pr_review_feedback, gitea_view_issue, gitea_list_issue_comments, and gitea_list_prs. This verdict was recorded through gitea_mark_final_review_decision and gitea_submit_pr_review. Local shell use was confined to repository inspection and test execution inside the reviewer worktree branches/review-pr954-b4c9f55 and the base worktree branches/review-pr954-base-82d71b77.
LAST_UPDATED_BY: reviewer

Reviewed independently at b4c9f558901699e29fac77b4f748565bcdb5f272. Not authored or merged by me.

## APPROVE — PR #954 (Closes #953) Re-reviewed independently at head `b4c9f558901699e29fac77b4f748565bcdb5f272` against base `master` at `82d71b77028a7abd4f8ab4a4e4d89658a187f73d`, in a fresh detached review worktree created for this head. The merge base equals the live master head. The old `cdf0dae` review worktree was not reused or altered. Review 632's two blocking findings are resolved, and both non-blocking ones are resolved as well. The remediation is materially better than the fix I prescribed for F1: my prescribed one-line change would have installed an inert gate, and the author proved why. ### Scope of the remediation range `cdf0dae..b4c9f55` is three commits, 9 files, +1453/-9, and each commit's contents match its stated purpose exactly: | Commit | Actual contents | | --- | --- | | `55d66c5` | `gitea_mcp_server.py`, `role_namespace_gate.py`, `role_session_router.py` — F1 only | | `1aa3517` | `author_issue_bootstrap.py`, `author_lock_contract.py`, `issue_lock_store.py`, `docs/author-issue-lock-contract.md` — F2 + F3 | | `b4c9f55` | `tests/test_issue_953_bootstrap_lock_contract.py`, `docs/mcp-tool-inventory.md` — F4 | Nothing outside the two findings, their tests, and their documentation. No unrelated file. --- ## F1 — resolved, and the author was right that my prescription was insufficient `_namespace_mutation_block(task, remote=remote, author_role_exclusive=True)` now sits at `gitea_mcp_server.py:5162`, between `check_author_mutation_after_reviewer_stop` and `_profile_permission_block`, and its return value is returned. It precedes every read and write of durable state — the lock load, the worktree observation, and `bind_session_lock` all come after it. **The author's central claim is correct and I confirmed it directly.** `check_author_mutation_namespace` resolves the task role through `role_session_router.required_role_for_task`, which reads `TASK_REQUIRED_ROLE` — not `task_capability_map`. Against this head: ``` create_issue required_role_for_task='author' create_pr required_role_for_task='author' commit_files required_role_for_task=None publish_unpublished_branch required_role_for_task=None bootstrap_author_issue_worktree required_role_for_task=None recover_dirty_orphaned_issue_worktree required_role_for_task=None recover_incomplete_bootstrap_lock required_role_for_task='author' ``` Without the new `TASK_REQUIRED_ROLE` entry the gate returns "allowed" for every caller. My review 632 fix would have added a call that refused nothing. **The author-exclusive wall is load-bearing, not decoration.** I dumped the live profile operation sets and ran them through `derive_role_kind`: | Profile | Derived role kind | Stopped by namespace check | Stopped by role-kind wall | | --- | --- | --- | --- | | `prgs-author` | `author` | no (correct) | no (correct) | | `prgs-reviewer` | `reviewer` | yes | yes | | `prgs-merger` | `reviewer` (holds `gitea.pr.merge`) | yes | yes | | `prgs-controller` | `limited` | **no** | **yes** | | `prgs-reconciler` | `limited` | **no** | **yes** | Controller and reconciler hold `gitea.issue.comment`, are not reviewer-bound, and are therefore admitted by `check_author_mutation_namespace` alone. Only `check_author_role_kind` stops them. `mixed` is refused rather than admitted. **No existing permission was broadened.** `author_role_exclusive` is keyword-only with default `False`; there are now seven `_namespace_mutation_block` call sites and only the new one passes it. The other six are byte-for-byte unchanged. `check_author_role_kind` returns early for any task the router does not declare author-required, so it grants nothing to anyone. **Sabotage probe P1.** I deleted the `_namespace_mutation_block` call from `gitea_recover_incomplete_bootstrap_lock`. Five cases failed: `test_the_recovery_tool_calls_the_namespace_mutation_gate`, `test_the_gate_return_value_is_consumed_and_returned`, `test_reviewer_namespace_is_rejected_with_matching_claimant_data`, `test_reviewer_rejection_emits_the_standard_blocked_audit`, `test_merger_profile_is_rejected`. **Sabotage probe P2.** I deleted only the two `TASK_REQUIRED_ROLE` entries, leaving the call in place. Eight failures including all four subtests of the role-kind matrix and `test_the_gate_routes_this_task_as_author_required` — so the suite catches the exact inert-gate condition the author identified, not merely the presence of the call. Both probes reverted; worktree verified clean. Denial behaviour verified in the suite and re-verified by running it: refusals carry `namespace_block: true` and `mcp_namespace`, emit the standard `BLOCKED` audit record naming the task, leave lock bytes / generation / branch / worktree and an unrelated lock byte-identical, and refuse a reviewer session *even when the claimant data would otherwise match* — the matching-claimant bypass is closed. ## F2 — resolved, plus a genuine pre-existing defect on the same path `exact_next_action` is now `author_lock_contract.post_compensation_action(post_state, ...)`, where `post_state` comes from `assess_post_compensation_state` fed by three direct observations taken *after* the rollback: `os.path.exists(lock_res)`, `os.path.isdir(target_worktree)`, and `_branch_exists(root, target_branch)`. Observation is authoritative over the journal's `rolled_back` list. `implementation_allowed` remains `false`, and the payload additionally carries `compensating_recovery` and `post_compensation_state`. **The dead-rollback claim is true and pre-existing.** `git show 82d71b77:issue_lock_store.py` has no `release_session_lock`, and the base caller wraps it in `except Exception:` — so every compensating rollback since #850 removed the branch and worktree and left the lock behind, raising `AttributeError` into a swallow. That is exactly the uninspectable state #953 exists to remove, and finding it while fixing F2 is the right call rather than scope creep. The new `release_session_lock` proves ownership rather than asserting it: exact `owner_session` match, issue-number match, repository-keyed exact path when `remote`/`org`/`repo` are known, scan fallback otherwise, and `FileNotFoundError` / `RuntimeError` on zero or multiple matches so an ambiguous directory is never guessed at. Phase journals and session pointers are excluded by shape (`lock_generation`, `branch_name`, `worktree_path` all required). The flock sidecar is deliberately left alone. The caller records `lock_release_failed:...` instead of swallowing. **Sabotage probe P3.** I restored `exact_next_action` to `recommended_action(contract)` — the pre-remediation expression. Five `Ac7PostCompensationGuidance` cases failed, and the assertion output quoted the exact unexecutable text review 632 flagged. **Sabotage probe P4.** I made `release_session_lock` a silent no-op. Seven cases failed, including `test_the_compensation_lock_release_actually_removes_the_lock`, `test_release_refuses_a_lock_owned_by_a_different_session`, `test_retry_after_complete_compensation_leaves_exactly_one_lock`, and `test_no_recommendation_names_an_artifact_the_rollback_deleted`. Both reverted; worktree clean. Cleanup-state coverage confirmed by running it: `complete` directs to a bootstrap retry and the suite then *executes* that retry and asserts exactly one canonical lock and one branch survive; surviving-lock and surviving-branch-and-worktree each get a target-specific action; a rollback that never completed is a distinct third state that recommends read-only inspection first; a failed rollback step is stated rather than presented as intentional; unrelated locks are byte-identical afterwards. ## F3 — resolved `SOURCE_BOOTSTRAP_LOCK_RECOVERY` is deleted and replaced by a comment stating why recovery reuses `SOURCE_LOCK_ISSUE`. `DeadProvenanceConstantRemoved` asserts the attribute is absent and that the sanctioned source set still excludes the recovery tool name. ## F4 — resolved `_NativeToolBase` drives `mcp_server.gitea_recover_incomplete_bootstrap_lock` and `mcp_server.gitea_inspect_issue_lock_contract` — the registered functions — against a real `git init` repository carrying committed work, a real durable lock file, and config-backed profiles. `_observe_recovery_worktree` is now called from the tool at `gitea_mcp_server.py:5188`. The suite grew 61 → 92 cases. That the four sabotage probes above produced failures is itself the proof that these cases reach the call sites rather than reconstructing them. --- ## Contract-wide reconciliation `NativeEndToEndBootstrapToCreatePr` runs the whole cycle against a real origin with `gitea_lock_issue` patched to raise `AssertionError` if anything reaches for it: real `bootstrap_author_issue_worktree` → registered inspection tool (`canonical`, `heartbeatable`, `create_pr_eligible`) → `heartbeat_session_lock` → real commit and push producing genuine divergence → second heartbeat → `verify_lock_for_mutation` → the unchanged `assess_lock_file_for_create_pr`. It also asserts the merge base is unmoved, so nothing was rewound to satisfy a gate. **`lease_id` question, answered explicitly.** On a canonical lock the key is *omitted*, not null: `build_canonical_issue_lock` writes `lease_id` only when one was allocated. Downstream ownership fences on `work_lease.task_session_id`, which is never null on a canonical lock, and the end-to-end sequence above completes with no workflow lease at all. So AC6 is satisfied and no downstream behaviour requires a lease bootstrap does not create. When a caller does supply `lease_id`, `_verify_assignment_and_lease_ids` still validates it against the control plane for existence, active status, expiry, owning session, and bound issue. **#447 guard intact.** `git diff 82d71b77..b4c9f55 -- issue_lock_provenance.py` is empty, and `SANCTIONED_LOCK_SOURCES` holds the same five members at both revisions. Recovery mints provenance server-side through `build_sanctioned_lock_provenance`; there is no parameter through which a caller can supply provenance or an authorization flag. **Race and foreign-owner safety.** The recovery write goes through `bind_session_lock(..., expected_generation=assessment["expected_generation"], recovery_sanctioned=True)`, so a moved generation fails closed. Ownership requires username **and** profile to match server-resolved values; a healthy foreign lock refuses `healthy_foreign_lock` and a stale foreign lock refuses `foreign_claimant`; an already-canonical lock is left strictly alone so a live owner's heartbeat token is never invalidated. --- ## Non-blocking findings **N1 — `post_compensation_action` fall-through misdescribes two survival combinations.** `author_lock_contract.py`, final `return` of `post_compensation_action`. The branch chain handles `(lock, worktree, branch)` = TTT, FTT, and T?F-via-`not worktree_present`, then falls through to prose that asserts "removed the lock and worktree; branch `X` survives". I probed the function directly across the matrix: for `(T,T,F)` and `(F,T,F)` that sentence is false on every clause it makes. I could not construct a reachable path to either state through `run_compensating_recovery` — it deletes the branch only after removing the worktree, and `git branch -D` refuses a branch checked out in a surviving worktree, while `_branch_exists` additionally fails closed to `True` — so I am not treating this as blocking. But `post_compensation_action` is a public pure function whose stated contract is that every branch names only artifacts the classification says exist, and that contract does not hold for those two inputs. Suggested: make the final branch an explicit `not lock_present and not worktree_present and branch_present` condition with a distinct fallback for anything unclassified, and add subtests over the full eight-cell matrix. **N2 — stale docstring inside the fixing commit.** `assess_post_compensation_state` says "`run_compensating_recovery` swallows a failed lock release and appends nothing", which the same commit made untrue by appending `lock_release_failed:...`. The reasoning it supports (observation over journal) is still right; only the stated justification is out of date. **N3 — PR description overstates one line.** "**Both** `gitea_lock_issue` and bootstrap now build through it" reads as though both call `build_canonical_issue_lock`. In the code, bootstrap and recovery call the record builder; `gitea_lock_issue` shares the contract through `_author_issue_work_lease` → `build_author_issue_work_lease`. The lease shape is genuinely shared and the tested behaviour is what the PR claims; the prose is stronger than the wiring. **N4 — follow-up worth filing, correctly out of scope here.** Four of the six pre-existing `_namespace_mutation_block` call sites — `commit_files`, `publish_unpublished_branch`, `bootstrap_author_issue_worktree`, `recover_dirty_orphaned_issue_worktree` — have no `TASK_REQUIRED_ROLE` entry, so their namespace walls short-circuit to allowed today. I confirmed this against this head. The author disclosed it rather than silently widening the table, which is the right boundary for #953; it deserves its own issue. --- ## Independent test evidence All commands run from the fresh reviewer worktree at `b4c9f55`; the base pair from a separate detached worktree at `82d71b77`. | Command | Exit | Result | | --- | --- | --- | | `venv/bin/python -m pytest tests/test_issue_953_bootstrap_lock_contract.py -q` | 0 | 92 passed, 5 subtests passed | | 17 adjacent lock / bootstrap / lease / renewal / recovery / heartbeat / role / capability suites, `-q` | 0 | 402 passed, 97 subtests passed (includes this issue's 92) | | `pytest -q` full suite at head `b4c9f55` | 1 | **28 failed, 5784 passed, 6 skipped, 1047 subtests** in 193.89s | | `pytest -q` full suite at base `82d71b77` | 1 | **28 failed, 5692 passed, 6 skipped, 1042 subtests** in 187.47s | Every author-reported count reproduced exactly. **Zero regressions, proven two ways.** 1. Sorted `FAILED` lines: 28 each side, `diff` empty. 2. Failure *reasons*: 42 `E ` lines each side. After normalizing only worktree root, temp paths, SHAs, timestamps, numeric ids, and the per-run random `test-commit-payloads-<suffix>` fixture names, `diff` is empty. The sole pre-normalization difference was that random suffix. **`test_documented_inventory_equals_registered_tools`, checked specifically.** Run alone at both revisions: ``` head b4c9f55: registered but not documented: gitea_rebind_dirty_same_claimant_author_session, gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree base 82d71b77: registered but not documented: gitea_rebind_dirty_same_claimant_author_session, gitea_reconcile_after_restart, gitea_recover_dirty_orphaned_issue_worktree ``` Identical. Neither new tool appears in the head drift, so the two added tools did **not** enlarge or alter this pre-existing failure — `docs/mcp-tool-inventory.md` lists both. Worktree verified clean via `git status --porcelain --untracked-files=all` after every probe and every run, still at `b4c9f55`. ## Preserved state `feat/issue-949-native-fleet-inventory` is at `92615f474bf6652d4e9ea59af7fd0dba03b56544` on `prgs` and in `branches/issue-949-fleet-inventory`; its lock, worktree, and uncreated PR were not touched. Issues #950–#952, PR #906's lease, the author's #953 worktree, the #953 baseline worktree, and `branches/review-pr954-cdf0dae` were not modified. No merge, cleanup, recovery, restart, reconnect, synchronization, drain, or reconciliation was performed. --- ## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: Merge PR #954 into master at head b4c9f558901699e29fac77b4f748565bcdb5f272 through the sanctioned merger workflow NEXT_PROMPT: ```text Role: MERGER Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools Issue: #953 PR: #954 Branch: fix/issue-953-bootstrap-lock-provenance Head: b4c9f558901699e29fac77b4f748565bcdb5f272 Base: master at 82d71b77028a7abd4f8ab4a4e4d89658a187f73d Approval: review 633 APPROVE at b4c9f558901699e29fac77b4f748565bcdb5f272 1. Native MCP only. gitea_whoami then gitea_resolve_task_capability(task='merge_pr') on prgs-merger. 2. Re-pin the live head with gitea_view_pr and confirm the approval still applies to it. 3. Acquire the merger PR lease with resolve key acquire_merger_pr_lease. 4. Pass branch_protection_requires_current_base=false explicitly when protection is determinable and absent, so a behind-but-approved PR is not mis-routed to update_branch_by_merge. 5. Merge, then release the merger lease. Call parity twice after the merge; the first read can still be pre-merge. 6. Do not recover issue #949 and do not modify issues #950, #951, or #952. ``` WHAT_HAPPENED: Independent re-review of PR #954 at head b4c9f558901699e29fac77b4f748565bcdb5f272 after the author's remediation of review 632. Verified all three remediation commits against their stated scope, confirmed the namespace mutation wall fires at the real tool boundary for reviewer, merger, controller, and reconciler sessions, confirmed the AC7 refusal derives its next action from directly observed post-compensation state, confirmed the previously nonexistent release_session_lock is implemented with exact-owner matching, and confirmed both new registered tool functions now have executing coverage. Four sabotage probes proved the new cases are load-bearing, all reverted with the worktree verified clean. Ran the focused suite, 17 adjacent suites, and a full-suite pair at head and base, comparing failing test IDs and normalized failure reasons rather than counts. WHY: Both review 632 blockers are completely resolved and the remediation exceeded the prescribed fix where the prescription was wrong: registering the task in role_session_router.TASK_REQUIRED_ROLE is what makes the namespace gate fire at all, and an author-exclusive role-kind wall is required because the task's permission is held by every configured role. The #447 provenance guard and its five sanctioned sources are unchanged, the end-to-end sequence completes without any gitea_lock_issue repair, recovery is generation-fenced and foreign-owner safe, and the full-suite pair shows identical failing test IDs and identical normalized failure reasons, so no pre-existing failure conceals a regression. ISSUE: 953 RELATED_PRS: 954 HEAD_SHA: b4c9f558901699e29fac77b4f748565bcdb5f272 REVIEW_STATUS: approved MERGE_READY: yes BLOCKERS: none VALIDATION: tests/test_issue_953_bootstrap_lock_contract.py 92 passed with 5 subtests, exit 0; 17 adjacent lock, bootstrap, lease, renewal, recovery, heartbeat, role, and capability suites 402 passed with 97 subtests, exit 0; full-suite pair from branches/ worktrees at head b4c9f558901699e29fac77b4f748565bcdb5f272 (28 failed, 5784 passed, 6 skipped, 1047 subtests, exit 1) and base 82d71b77028a7abd4f8ab4a4e4d89658a187f73d (28 failed, 5692 passed, 6 skipped, 1042 subtests, exit 1) with identical sorted FAILED lines and identical normalized E-line failure reasons; test_documented_inventory_equals_registered_tools reason byte-identical at both revisions and naming neither new tool; four sabotage probes each produced the expected failures and were reverted; git diff base..head of issue_lock_provenance.py empty; merge base equals live master; review worktree clean via git status --porcelain --untracked-files=all. NATIVE_REVIEW_PROOF: Conducted entirely through the native gitea-reviewer MCP namespace on profile prgs-reviewer as sysadmin against Scaled-Tech-Consulting/Gitea-Tools on prgs. gitea_whoami confirmed identity sysadmin with role_kind reviewer, distinct from author jcwalker3. gitea_get_runtime_context and gitea_assess_master_parity reported in_parity true, live_stale false, restart_required false, mutation_safe true, with startup, daemon-start, local, current, and live-remote revisions all 82d71b77028a7abd4f8ab4a4e4d89658a187f73d. gitea_load_review_workflow recorded workflow hash 263d0a6cb8a6 with boundary clean. gitea_resolve_task_capability(review_pr) returned allowed_in_current_session true immediately before each mutation. Reviewer PR lease session 38475-bf76ed428e17 was acquired at candidate_head b4c9f558901699e29fac77b4f748565bcdb5f272 through gitea_acquire_reviewer_pr_lease and heartbeated through gitea_heartbeat_reviewer_pr_lease. PR state, reviews, and comments were read through gitea_view_pr, gitea_get_pr_review_feedback, gitea_view_issue, gitea_list_issue_comments, and gitea_list_prs. This verdict was recorded through gitea_mark_final_review_decision and gitea_submit_pr_review. Local shell use was confined to repository inspection and test execution inside the reviewer worktree branches/review-pr954-b4c9f55 and the base worktree branches/review-pr954-base-82d71b77. LAST_UPDATED_BY: reviewer Reviewed independently at `b4c9f558901699e29fac77b4f748565bcdb5f272`. Not authored or merged by me.

sysadmin commented

2026-07-28 02:02:47 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 38475-bf76ed428e17
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: released
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T07:02:47Z
expires_at: 2026-07-28T07:12:47Z
blocker: manual-release

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 38475-bf76ed428e17 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: released candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T07:02:47Z expires_at: 2026-07-28T07:12:47Z blocker: manual-release

sysadmin commented

2026-07-28 02:19:59 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #954
issue: #953
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 13484-bcb88d6a9b95
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55
phase: claimed
candidate_head: b4c9f55890
target_branch: master
target_branch_sha: 82d71b7702
last_activity: 2026-07-28T07:19:58Z
expires_at: 2026-07-28T07:29:58Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #954 issue: #953 reviewer_identity: sysadmin profile: prgs-merger session_id: 13484-bcb88d6a9b95 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr954-b4c9f55 phase: claimed candidate_head: b4c9f558901699e29fac77b4f748565bcdb5f272 target_branch: master target_branch_sha: 82d71b77028a7abd4f8ab4a4e4d89658a187f73d last_activity: 2026-07-28T07:19:58Z expires_at: 2026-07-28T07:29:58Z blocker: none

sysadmin merged commit aad5c8b423 into master

2026-07-28 02:21:11 -05:00

sysadmin commented

2026-07-28 02:21:15 -05:00

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

Status: APPLIED

actor: sysadmin
profile: prgs-merger
timestamp: 2026-07-28T07:21:15.333696+00:00
last terminal: approve on PR fix(author): unify the bootstrap and lock_issue issue-lock contract (Closes #953) (#954)
PR state: closed (merged=True)
merge_commit_sha: aad5c8b42361d380a8eeb07b94b90815e594c2c5
prior live_mutations_count: 2
prior profile_identity: prgs-reviewer

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

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-28T07:21:15.333696+00:00` - last terminal: `approve` on PR #954 - PR state: `closed` (merged=True) - merge_commit_sha: `aad5c8b42361d380a8eeb07b94b90815e594c2c5` - prior live_mutations_count: `2` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.

jcwalker3 referenced this pull request

2026-07-28 02:56:38 -05:00

Canary: five concurrent LLM clients with a negative concurrency suite #963

jcwalker3 referenced this pull request

2026-07-28 03:02:56 -05:00

Epic: Make author workflows transactional and safe for multiple LLM clients #887

jcwalker3 referenced this pull request

2026-07-28 03:07:51 -05:00

Epic: certify safe multi-LLM execution through securely isolated remote MCP #955

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

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

fix(author): unify the bootstrap and lock_issue issue-lock contract (Closes #953) #954

The defect

The canonical contract

How heartbeat, renewal, recovery, and create_pr now agree

Bootstrap fails closed before implementation

Legacy and malformed-lock handling

Target-specific recovery

The #447 guard was preserved

Test evidence

Proven pre-existing failures

Preserved state

Reviewer focus areas

REQUEST_CHANGES — PR #954 (Closes #953)

Independently confirmed

F1 (blocking) — the new author mutation omits the namespace mutation gate

F2 (blocking) — bootstrap's AC7 refusal returns a next action that cannot be executed

F3 (non-blocking) — dead provenance constant

F4 (non-blocking) — the two new tool functions have no executing coverage

Not addressed, and fine to leave

Canonical PR State

Author remediation of review 632 — PR #954, head b4c9f558901699e29fac77b4f748565bcdb5f272

F1 — how the namespace mutation wall was added

F2 — how AC7 guidance now follows actual post-compensation state

F3 — the dead constant was removed

#447 provenance enforcement is unchanged

F4 — coverage that executes the real native tool functions

Exact test evidence

Files changed since cdf0dae

Preserved state

Reviewer focus areas

Canonical PR State

APPROVE — PR #954 (Closes #953)

Scope of the remediation range

F1 — resolved, and the author was right that my prescription was insufficient

F2 — resolved, plus a genuine pre-existing defect on the same path

F3 — resolved

F4 — resolved

Contract-wide reconciliation

Non-blocking findings

Independent test evidence

Preserved state

Canonical PR State

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

How heartbeat, renewal, recovery, and `create_pr` now agree

Author remediation of review 632 — PR #954, head `b4c9f558901699e29fac77b4f748565bcdb5f272`

Files changed since `cdf0dae`