fix(author): unify the bootstrap and lock_issue issue-lock contract (Closes #953) #954
Merged
sysadmin
merged 4 commits from 2026-07-28 02:21:11 -05:00
fix/issue-953-bootstrap-lock-provenance into master
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  sysadminandClaude Opus 4.8
|
b4c9f55890 |
test(author): execute the native #953 tool functions and compensation paths (#953)
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]> |
||
|
sysadminandClaude Opus 4.8
|
1aa351718a |
fix(author): derive AC7 guidance from the state compensation actually leaves (#953)
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]> |
||
|
sysadminandClaude Opus 4.8
|
55d66c57e4 |
fix(author): gate bootstrap-lock recovery on the namespace mutation wall (#953)
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]> |
||
|
sysadminandClaude Opus 4.8
|
cdf0daefa9 |
fix(author): unify the bootstrap and lock_issue issue-lock contract (Closes #953)
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 |