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 base82d71b77: 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 at92615f474band 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]>
This commit is contained in:
@@ -0,0 +1,148 @@
|
||||
# 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.
|
||||
|
||||
## 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.
|
||||
|
||||
### 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.
|
||||
Reference in New Issue
Block a user