Complete lease fencing and align remaining task classes (Issue #790 Slice C) #793

New Issue

2026-07-22T03:28:20-05:00

jcwalker3 commented

2026-07-22 03:28:20 -05:00

Scope transfer

Slice C of Issue #790, transferred to its own issue so that closing #790 through the Slice A pull request does not imply this work is done. Issue #790 tracks and is completed by Slice A only. The acceptance criteria below are carried verbatim from the controller reassessment recorded in Issue #790 comment 13958.

Problem

Three gaps remain after Slice A.

Fencing is write-side only. issue_lock_store.bind_session_lock performs a lock_generation compare-and-swap inside the per-issue flock, and Slice A added the same compare-and-swap to the heartbeat writer. But issue_lock_store.verify_lock_for_mutation — the gate every author mutation passes through — checks freshness, issue number, branch, and worktree, and does not check the generation. A session holding a stale in-memory lock record therefore still passes the read side after another session has reclaimed or retired the work.

The Issue #760 renewal path predates heartbeats. It grants renewal to the exact recorded owner of an expired lease with no heartbeat requirement at all, which was correct when it landed because heartbeats did not exist. Now that they do, historical ownership is not evidence of present activity, and the boundary has to move.

The shared lifecycle covers one task class. Slice A migrated author_issue_work and deliberately declared but did not rewire reviewer, merger, conflict-fix, and control-plane leases. Their durations are recorded in lease_policy with heartbeat_lifecycle_active false, and a Slice A test asserts the declared values still match the constants those modules own, so the two cannot drift apart before this issue migrates the call sites.

Acceptance criteria

AC-C1. verify_lock_for_mutation verifies lock_generation in addition to its existing freshness, issue, branch, and worktree checks, so a session holding a stale in-memory lock record cannot pass the read-side gate after another session has reclaimed or retired the work. This is #790 AC-N4.

AC-C2. The Issue #760 exact-owner renewal path is re-scoped to require a fresh heartbeat within the grace window. A lease past the missed-heartbeat reclaim threshold is not renewable on historical ownership evidence alone and must use the sanctioned reclaim or re-adoption path under generation fencing, which mints a new generation and a genuine heartbeat. This is #790 AC-N5.

AC-C3. The remediation delivered by PR #791 remains in force until this replacement is implemented and covered end to end. It is not removed, weakened, or short-circuited before its replacement is proven.

AC-C4. Reviewer, merger, and conflict-fix leases read their durations from lease_policy rather than module-local constants, and adopt the shared acquire, renew, expire, reclaim, release, and audit semantics. Task classes may keep different values; they may not keep different lifecycles.

AC-C5. The control-plane lease default in control_plane_db is aligned to the same policy source, with explicit compatibility behavior for lease rows written under the previous default.

AC-C6. A late heartbeat from a superseded session cannot revive or overwrite ownership after another session has reclaimed the task, across every migrated task class. Heartbeat, adoption, release, and reclaim operations are atomic and idempotent.

AC-C7. Native MCP integration tests cover every downstream gate this work touches, driving the real tools against a real repository and real durable state. Assessor-only coverage is explicitly insufficient: it did not catch either of the two sanction discard points found in review #499 on PR #791. This is #790 AC-N6 applied to this slice.

AC-C8. Existing behavior is preserved: Issue #753 dead-session recovery, Issue #755 owning-pull-request evidence, Issue #772 generation compare-and-swap, Issue #747 reviewer sliding TTL semantics where values are unchanged, and the Slice A heartbeat lifecycle and legacy-lock protection.

Required tests

A stale in-memory lock record fails verify_lock_for_mutation after another session advances the generation.
A current record passes, so the added check does not block ordinary author work.
Exact-owner renewal succeeds while the owner is heartbeating within the grace window.
Exact-owner renewal is refused once the lease is past the missed-heartbeat threshold, and the refusal names the sanctioned reclaim path.
The Issue #760 acceptance criteria that survive the re-scope still hold, and the two Issue #760 suites are updated rather than deleted.
Reviewer, merger, and conflict-fix leases resolve their durations from the policy module, proven by overriding the policy and observing the change.
The Slice A drift-guard test still passes after the call sites are migrated, proving the migration changed no value.
Control-plane lease rows written under the previous default receive deterministic compatibility treatment.
A late heartbeat from a superseded session is rejected for each migrated task class.
Duplicate heartbeats are idempotent.
Read-after-write verifies the resulting lease and audit state.
Native MCP integration coverage per AC-C7.

Dependencies

Depends on Issue #790 Slice A landing first, for lease_policy and the heartbeat lifecycle this extends. Independent of Slice B; the two may proceed in either order. Touches issue_lock_store expiry and conflict assessment, so it must not be allocated concurrently with Slice B or with any other work on those surfaces.

Non-goals

Do not implement Slice B. Do not weaken worktree cleanliness, ancestry, publication, or binding checks. Do not make pid liveness an ownership authority. Do not use one identical duration for every task class merely because the lifecycle is shared.

Canonical issue state

STATE: ready-for-allocation
WHO_IS_NEXT: controller
NEXT_ACTION: Allocate after Issue #790 Slice A lands.
NEXT_PROMPT: Implement lease fencing completion and task-class alignment per AC-C1 through AC-C8, in a separately bounded author task, after Slice A has landed on master and with no concurrent allocation on the issue_lock_store expiry surfaces.

## Scope transfer Slice C of Issue #790, transferred to its own issue so that closing #790 through the Slice A pull request does not imply this work is done. Issue #790 tracks and is completed by Slice A only. The acceptance criteria below are carried verbatim from the controller reassessment recorded in Issue #790 comment 13958. ## Problem Three gaps remain after Slice A. **Fencing is write-side only.** `issue_lock_store.bind_session_lock` performs a `lock_generation` compare-and-swap inside the per-issue flock, and Slice A added the same compare-and-swap to the heartbeat writer. But `issue_lock_store.verify_lock_for_mutation` — the gate every author mutation passes through — checks freshness, issue number, branch, and worktree, and does not check the generation. A session holding a stale in-memory lock record therefore still passes the read side after another session has reclaimed or retired the work. **The Issue #760 renewal path predates heartbeats.** It grants renewal to the exact recorded owner of an expired lease with no heartbeat requirement at all, which was correct when it landed because heartbeats did not exist. Now that they do, historical ownership is not evidence of present activity, and the boundary has to move. **The shared lifecycle covers one task class.** Slice A migrated `author_issue_work` and deliberately declared but did not rewire reviewer, merger, conflict-fix, and control-plane leases. Their durations are recorded in `lease_policy` with `heartbeat_lifecycle_active` false, and a Slice A test asserts the declared values still match the constants those modules own, so the two cannot drift apart before this issue migrates the call sites. ## Acceptance criteria **AC-C1.** `verify_lock_for_mutation` verifies `lock_generation` in addition to its existing freshness, issue, branch, and worktree checks, so a session holding a stale in-memory lock record cannot pass the read-side gate after another session has reclaimed or retired the work. This is #790 AC-N4. **AC-C2.** The Issue #760 exact-owner renewal path is re-scoped to require a fresh heartbeat within the grace window. A lease past the missed-heartbeat reclaim threshold is not renewable on historical ownership evidence alone and must use the sanctioned reclaim or re-adoption path under generation fencing, which mints a new generation and a genuine heartbeat. This is #790 AC-N5. **AC-C3.** The remediation delivered by PR #791 remains in force until this replacement is implemented and covered end to end. It is not removed, weakened, or short-circuited before its replacement is proven. **AC-C4.** Reviewer, merger, and conflict-fix leases read their durations from `lease_policy` rather than module-local constants, and adopt the shared acquire, renew, expire, reclaim, release, and audit semantics. Task classes may keep different values; they may not keep different lifecycles. **AC-C5.** The control-plane lease default in `control_plane_db` is aligned to the same policy source, with explicit compatibility behavior for lease rows written under the previous default. **AC-C6.** A late heartbeat from a superseded session cannot revive or overwrite ownership after another session has reclaimed the task, across every migrated task class. Heartbeat, adoption, release, and reclaim operations are atomic and idempotent. **AC-C7.** Native MCP integration tests cover every downstream gate this work touches, driving the real tools against a real repository and real durable state. Assessor-only coverage is explicitly insufficient: it did not catch either of the two sanction discard points found in review #499 on PR #791. This is #790 AC-N6 applied to this slice. **AC-C8.** Existing behavior is preserved: Issue #753 dead-session recovery, Issue #755 owning-pull-request evidence, Issue #772 generation compare-and-swap, Issue #747 reviewer sliding TTL semantics where values are unchanged, and the Slice A heartbeat lifecycle and legacy-lock protection. ## Required tests 1. A stale in-memory lock record fails `verify_lock_for_mutation` after another session advances the generation. 2. A current record passes, so the added check does not block ordinary author work. 3. Exact-owner renewal succeeds while the owner is heartbeating within the grace window. 4. Exact-owner renewal is refused once the lease is past the missed-heartbeat threshold, and the refusal names the sanctioned reclaim path. 5. The Issue #760 acceptance criteria that survive the re-scope still hold, and the two Issue #760 suites are updated rather than deleted. 6. Reviewer, merger, and conflict-fix leases resolve their durations from the policy module, proven by overriding the policy and observing the change. 7. The Slice A drift-guard test still passes after the call sites are migrated, proving the migration changed no value. 8. Control-plane lease rows written under the previous default receive deterministic compatibility treatment. 9. A late heartbeat from a superseded session is rejected for each migrated task class. 10. Duplicate heartbeats are idempotent. 11. Read-after-write verifies the resulting lease and audit state. 12. Native MCP integration coverage per AC-C7. ## Dependencies Depends on Issue #790 Slice A landing first, for `lease_policy` and the heartbeat lifecycle this extends. Independent of Slice B; the two may proceed in either order. Touches `issue_lock_store` expiry and conflict assessment, so it must not be allocated concurrently with Slice B or with any other work on those surfaces. ## Non-goals Do not implement Slice B. Do not weaken worktree cleanliness, ancestry, publication, or binding checks. Do not make pid liveness an ownership authority. Do not use one identical duration for every task class merely because the lifecycle is shared. ## Canonical issue state ```text STATE: ready-for-allocation WHO_IS_NEXT: controller NEXT_ACTION: Allocate after Issue #790 Slice A lands. NEXT_PROMPT: Implement lease fencing completion and task-class alignment per AC-C1 through AC-C8, in a separately bounded author task, after Slice A has landed on master and with no concurrent allocation on the issue_lock_store expiry surfaces. ```