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]>
210 lines
10 KiB
Markdown
210 lines
10 KiB
Markdown
# The canonical author issue-lock contract (#953)
|
|
|
|
Every author issue lock has exactly one shape. Both writers —
|
|
`gitea_bootstrap_author_issue_worktree` and `gitea_lock_issue` — build it
|
|
through `author_lock_contract.build_canonical_issue_lock`, and every reader
|
|
consumes that same shape.
|
|
|
|
Before #953 the two writers disagreed. `gitea_lock_issue` wrote the canonical
|
|
record; bootstrap wrote a thinner one with the claimant at the lock top level,
|
|
`lease_id: null`, and no `work_lease`, `lock_provenance`, or expiry. Because
|
|
every reader was written against the canonical shape, a lock that bootstrap
|
|
reported as successfully created could not be heartbeated, renewed, re-locked,
|
|
or accepted by `gitea_create_pr`. Each of those gates was individually correct;
|
|
the defect was that two writers disagreed about what a lock *is*.
|
|
|
|
## Required ordering
|
|
|
|
**Finalize the lock before writing any implementation bytes.** This ordering is
|
|
what keeps recovery cheap: while the worktree is still base-equivalent, a lock
|
|
problem can be fixed by simply calling `gitea_lock_issue` again. Once the branch
|
|
carries commits, base-equivalence is gone and the ordinary re-lock path is no
|
|
longer available.
|
|
|
|
1. `gitea_whoami` — resolve identity and profile.
|
|
2. `gitea_resolve_task_capability(task='work_issue')`.
|
|
3. `gitea_bootstrap_author_issue_worktree` — creates the branch, the registered
|
|
worktree under `branches/`, and a **canonical** lock. It reads the lock back
|
|
and verifies it structurally before reporting success; a partial lock fails
|
|
closed here, with the missing fields named, and never reports
|
|
`implementation_allowed: true`.
|
|
4. `gitea_heartbeat_issue_lock` — prove the lock is usable, using the
|
|
`task_session_id` bootstrap returned.
|
|
5. Implement, commit, push.
|
|
6. `gitea_create_pr`.
|
|
|
|
If bootstrap returns `success: false` with
|
|
`reason_code: incomplete_issue_lock_contract`, **do not implement**. Its
|
|
`exact_next_action` names the executable recovery step. Bootstrap's reported
|
|
next action always matches the state it actually returned.
|
|
|
|
### What that refusal leaves behind
|
|
|
|
The AC7 refusal runs `run_compensating_recovery` *before* it reports, so the
|
|
advice has to describe the post-rollback state rather than the shape of the lock
|
|
that provoked it. Recommending incomplete-lock recovery for artifacts the
|
|
rollback already deleted would produce `no_durable_lock` and then
|
|
`worktree_invalid` — two refusals for a state a plain retry fixes.
|
|
|
|
The refusal therefore carries `compensating_recovery` and
|
|
`post_compensation_state`, and derives `exact_next_action` from what was
|
|
observed on disk. `cleanup_state` is one of:
|
|
|
|
| `cleanup_state` | Meaning | Next action |
|
|
| --- | --- | --- |
|
|
| `complete` | lock, branch, and worktree all removed | resolve `missing_fields` and re-run `gitea_bootstrap_author_issue_worktree` |
|
|
| `partial` | rollback ran; some artifacts survive, by design or because a step errored | scoped to exactly what survives — see below |
|
|
| `failed` | rollback never completed, so nothing is proven removed | `gitea_inspect_issue_lock_contract` (read-only) before anything else |
|
|
|
|
Within `partial`, the surviving set decides the action:
|
|
|
|
| Survives | Next action |
|
|
| --- | --- |
|
|
| lock + branch + worktree | `gitea_recover_incomplete_bootstrap_lock` for that exact issue, branch, and worktree |
|
|
| branch + worktree (lock released) | `gitea_lock_issue` — no implementation bytes were written, so the worktree is still base-equivalent |
|
|
| lock only (worktree removed) | `gitea_inspect_issue_lock_contract`; the surviving lock must be released by its recorded owner before bootstrap is retried |
|
|
| branch only | `gitea_inspect_issue_lock_contract`, then re-run bootstrap, which adopts the existing branch |
|
|
|
|
`failed_rollback_steps` names any rollback step that errored, and the returned
|
|
action says so rather than presenting the surviving state as intentional.
|
|
|
|
> The lock half of that rollback was dead code until #953 review 632 F2:
|
|
> `run_compensating_recovery` called `issue_lock_store.release_session_lock`,
|
|
> which did not exist, inside a bare `except Exception: pass`. Every rollback
|
|
> removed the branch and worktree and silently left the lock — the exact
|
|
> uninspectable, unrecoverable state this issue exists to eliminate. The
|
|
> function now exists, releases only a lock whose recorded `owner_session`
|
|
> matches, and its failures are recorded rather than swallowed.
|
|
|
|
## The contract
|
|
|
|
A canonical lock carries every field in
|
|
`author_lock_contract.REQUIRED_LOCK_FIELDS`:
|
|
|
|
| Field | Meaning |
|
|
| --- | --- |
|
|
| `remote`, `org`, `repo`, `issue_number` | repository and issue identity |
|
|
| `branch_name`, `worktree_path` | the binding this claim owns |
|
|
| `work_lease` | the canonical lease block, below |
|
|
| `lock_provenance` | sanctioned source, minted server-side |
|
|
| `lock_generation` | monotonic; every write advances it |
|
|
|
|
`work_lease` carries every field in
|
|
`author_lock_contract.REQUIRED_WORK_LEASE_FIELDS`, notably:
|
|
|
|
| Field | Meaning |
|
|
| --- | --- |
|
|
| `claimant.{username,profile}` | **canonical** claimant placement |
|
|
| `expires_at` | sliding TTL from `lease_policy` |
|
|
| `last_heartbeat_at`, `heartbeat_count` | liveness evidence |
|
|
| `task_session_id` | the ownership fencing token — never null |
|
|
| `lifecycle_version` | `heartbeat-v1`; its absence is what makes a lock legacy |
|
|
|
|
### Claimant placement and legacy compatibility
|
|
|
|
`work_lease.claimant` is canonical. A top-level `claimant` is the legacy
|
|
placement written by pre-#953 bootstrap and is still **read** — through the one
|
|
shared reader, `issue_lock_store.lock_claimant` — so an existing lock is not
|
|
refused for "not recording a claimant" when it plainly records one.
|
|
|
|
Tolerating the placement is not a widening. Every caller still compares the
|
|
values against server-resolved identity and profile, so a legacy placement
|
|
grants nothing the canonical placement would not. When both are present, the
|
|
`work_lease` copy wins: after an upgrade, a stale top-level copy must never
|
|
decide ownership.
|
|
|
|
### Expiration is explicit
|
|
|
|
A lock with no recorded expiry is **not** "not yet expired". `is_lease_expired`
|
|
returns `False` for it, which used to make such a lock permanently non-expiring
|
|
*and* permanently ineligible for #760 exact-owner renewal, which only ever
|
|
assesses an expired lease. `author_lock_contract.expiration_state` names the
|
|
real fact: `recorded`, `missing`, or `unparseable`. A `missing` expiry makes the
|
|
lock eligible for the recovery path below rather than stranding it.
|
|
|
|
## Recovering an existing incomplete bootstrap lock
|
|
|
|
For locks already written by the old bootstrap — including those whose branches
|
|
already carry legitimate committed and pushed work — use:
|
|
|
|
```text
|
|
gitea_inspect_issue_lock_contract(issue_number, branch_name, worktree_path, remote=...)
|
|
gitea_recover_incomplete_bootstrap_lock(issue_number, branch_name, worktree_path, expected_head, remote=...)
|
|
```
|
|
|
|
`gitea_inspect_issue_lock_contract` is strictly read-only: it performs no lock,
|
|
lease, branch, worktree, issue, or pull-request mutation. Use it first to see
|
|
which fields are missing and what the recommended action is; pass `dry_run=True`
|
|
to the recovery tool to preview the decision without writing.
|
|
|
|
`gitea_recover_incomplete_bootstrap_lock` upgrades that one lock to the
|
|
canonical contract. Before writing anything it verifies:
|
|
|
|
* repository (`remote`, `org`, `repo`) and issue number
|
|
* claimant username **and** profile against the server-resolved values — a
|
|
matching username alone is never accepted
|
|
* branch, worktree path, worktree existence, and worktree registration
|
|
* the worktree is on the recorded branch
|
|
* the observed head equals the caller's `expected_head`
|
|
* the existing lock's generation and provenance state
|
|
* the absence of healthy foreign ownership
|
|
|
|
What it deliberately does **not** do:
|
|
|
|
* it never moves, resets, or rewinds the branch, and never requires
|
|
base-equivalence — preserving the committed work is the entire point;
|
|
* it never pushes and never creates a pull request;
|
|
* it touches only the single lock file for that exact remote/org/repo/issue;
|
|
* it accepts no caller-supplied provenance and no caller-supplied authorization
|
|
flag — both are minted server-side.
|
|
|
|
A recovered lock records a `bootstrap_lock_recovery` block holding both sides of
|
|
the transition — prior contract, prior missing fields, prior generation, prior
|
|
owning session, the replacement `task_session_id`, and the preserved head — so a
|
|
recovered claim never reads as an original one.
|
|
|
|
### Gates, in order
|
|
|
|
`gitea_recover_incomplete_bootstrap_lock` is an author-only durable-lock
|
|
mutation and carries the same three gates as every comparable author operation,
|
|
in this order:
|
|
|
|
1. `role_session_router.check_author_mutation_after_reviewer_stop` — no author
|
|
fallback after a reviewer `wrong_role_stop`.
|
|
2. `_namespace_mutation_block(task, remote=remote, author_role_exclusive=True)` —
|
|
the namespace wall. It refuses a reviewer-bound session and, because this
|
|
task's required permission is `gitea.issue.comment` (which merger,
|
|
controller, and reconciler profiles also hold), additionally requires the
|
|
active profile's derived role kind to be exactly `author`. A refusal carries
|
|
`namespace_block: true` and emits a `BLOCKED` audit record naming the
|
|
namespace and profile.
|
|
3. `_profile_permission_block` — operation, provenance, and session-context
|
|
gates.
|
|
|
|
Exact-owner claimant matching inside `assess_bootstrap_lock_recovery` runs
|
|
*after* all three. It is a further layer, never a substitute for them: on its
|
|
own it refuses one step too late and leaves the audit trail silent about the
|
|
attempt.
|
|
|
|
### Refusals
|
|
|
|
| `refusal_code` | Meaning |
|
|
| --- | --- |
|
|
| `no_durable_lock` | nothing to recover |
|
|
| `already_canonical` | lock is fine; rewriting would invalidate a live heartbeat token |
|
|
| `foreign_claimant` | recorded claimant is not the active identity/profile pair |
|
|
| `healthy_foreign_lock` | a live foreign-owned lock; takeover is not a recovery path |
|
|
| `identity_unresolved` | identity or profile could not be resolved |
|
|
| `binding_mismatch` | repository, issue, branch, or worktree does not match |
|
|
| `worktree_invalid` | worktree missing, unregistered, or on another branch |
|
|
| `head_mismatch` | the worktree moved under the caller |
|
|
|
|
## The #447 create-PR provenance guard is unchanged
|
|
|
|
`issue_lock_provenance.assess_lock_file_for_create_pr` still requires both a
|
|
sanctioned `lock_provenance` and a `work_lease`, and the sanctioned source set
|
|
was **not** widened. Bootstrap writes through
|
|
`issue_lock_provenance.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.
|