feat(lease): make the author task heartbeat load-bearing (Closes #790, Slice A) #794

Open
jcwalker3 wants to merge 1 commits from fix/issue-790-slice-a-heartbeat-policy into master
Owner

Closes #790

Follow-up: #792
Follow-up: #793

This pull request implements Slice A only

Issue #790 was reduced to Slice A by the scope transfer recorded in its comment 14010, so the closing reference above is accurate. The rest of the original scope was moved to dedicated issues before this pull request was opened:

  • #792 — Slice B: terminal author-lease retirement, the reconciler-only retirement operation, generation-compare-and-swap terminal status, the two-minute configurable race-drain, full revalidation before destructive cleanup.
  • #793 — Slice C: lock_generation verification in verify_lock_for_mutation, the Issue #760 renewal re-scope, remaining task-class policy alignment, control-plane lease alignment.

Merging this closes #790 correctly. It does not complete #792 or #793, and #790 should not be reopened to represent them.

Problem

issue_lock_store.assess_lock_freshness parsed last_heartbeat_at and then never consulted it. Liveness was decided by an absolute four-hour expires_at and by process-identifier liveness, and the recorded identifier is the long-lived MCP daemon rather than the authoring task, so an abandoned claim stayed live for the full four hours. A tree-wide search found the field written in exactly one place, gitea_mcp_server._build_author_issue_work_lease, and advanced by nothing anywhere.

Issue #787 / PR #789 hit this. Issue #760 / PR #791 hit it again: its lease was minted at 04:19:08Z, never heartbeated, its recorded pid 39849 stayed alive as a daemon, and reconciliation was still blocked at 06:39:43Z — more than five hours after the work landed on master.

What changed

A1 — one authoritative policy source. New lease_policy declares every duration for every task class: author initial and sliding TTL 10 minutes, heartbeat cadence 2, stale warning 5, missed-heartbeat grace 10, absolute cap 8 hours, recovery grace 10, terminal race-drain 2. It ships first so the first heartbeat and TTL behavior to run reads from it (AC-N7). The duplicated four-hour literal is deleted from both issue_lock_store and gitea_mcp_server. Reviewer, merger, and conflict-fix classes are declared but not rewired; a test asserts the declared numbers still match the constants reviewer_pr_lease and pr_work_lease own, so the two cannot drift apart before #793 migrates those call sites.

A2 — load-bearing freshness, with two intentional asymmetries. An alive process identifier never establishes freshness anywhere (AC-N2): it is recorded as evidence and no branch returns live because of it. A dead one still marks a lease stale, and that band still precedes every heartbeat evaluation, so Issue #753 dead-session recovery keys on exactly the classification it always did. New bands stale_missed_heartbeat and stale_absolute_cap are classified in branch_cleanup_guard instead of falling through to unknown-status, and still block unless the ownership record proves reclaim_allowed is True. A heartbeat lease carrying no heartbeat is contradictory and fails closed. assess_expired_lock_reclaim accepts a lapsed heartbeat as reclaim grounds for heartbeat-lifecycle leases only — under this lifecycle the heartbeat is the liveness proof, and additionally demanding a dead process would reinstate the original defect.

A3 and A4 — task-session identity and the writer. mint_task_session_id produces an ownership key containing no process identifier, because the daemon identifier is reused by every task it serves and identifies none of them. heartbeat_session_lock writes inside the existing per-issue flock under the Issue #772 generation compare-and-swap, verifying exact issue, branch, realpath-normalized worktree, claimant username, claimant profile, and recorded session identifier. It cannot acquire, take over, or revive: a lease past its grace is refused and must use the reclaim path, so a session that stopped proving liveness cannot restore ownership retroactively. New gitea_heartbeat_issue_lock gates on the authority lock_issue already requires, being a strictly narrower operation.

A5 — legacy compatibility (AC-N8). An explicit lifecycle_version marker, never a timestamp comparison, discriminates legacy from heartbeat leases. A legacy lock has last_heartbeat_at matching created_at forever precisely because nothing advanced it, and a freshly minted heartbeat lease has them matching too, so that equality carries no information in either direction. Legacy locks keep their recorded absolute expiry and are never evaluated against the short grace, so deploying this cannot make an existing claim instantly reclaimable. They leave that state only through terminal retirement (#792) or rebind_legacy_lock, which re-verifies the exact owner and mints a genuine identifier and first heartbeat while preserving the original claim under legacy_origin. Rebinding a lapsed legacy lease is refused; that case belongs to Issue #760 renewal or Issue #601 reclaim.

A6 — native coverage. Review #499 proved assessor-level tests miss discard points, so tests/test_issue_790_heartbeat_mcp_path.py drives the real tools against a real git repository and a real durable lock.

Files changed

File Change
lease_policy.py new; the single authoritative duration source
issue_lock_store.py lifecycle helpers, session-id minting, rewritten freshness, reclaim on lapsed heartbeat, heartbeat_session_lock, rebind_legacy_lock
gitea_mcp_server.py policy-derived lease with lifecycle marker and session id; new gitea_heartbeat_issue_lock tool
branch_cleanup_guard.py classify the two new stale bands
task_capability_map.py heartbeat_issue_lock capability entry
docs/mcp-tool-inventory.md register the new tool in the documented inventory
tests/test_issue_790_lease_policy.py new, 40 cases
tests/test_issue_790_heartbeat_mcp_path.py new, 15 cases

1974 insertions, 32 deletions.

Non-regressions preserved

Issue #760 exact-owner renewal is untouched and stays in force until #793 replaces it end to end. Issue #753 dead-session recovery, Issue #755 owning-pull-request evidence, Issue #772 generation compare-and-swap, and Issue #747 reviewer sliding TTL all keep their existing behavior, each covered by its own suite below.

Tests and results

Run with venv/bin/python -m pytest from the allocated worktree.

  • New suites — 55 passed (test_issue_790_lease_policy.py 40, test_issue_790_heartbeat_mcp_path.py 15).
  • Lock and lease regression set (test_issue_lock_store, test_lease_lifecycle, test_issue_753_dead_pid_lock_recovery, test_issue_755_owning_pr_recovery, test_issue_760_exact_owner_lease_renewal, test_issue_760_mcp_renewal_path, test_issue_768_strict_descendant_recovery, test_issue_772_unpublished_claim_recovery, test_lock_issue_mcp_registration, test_issue_lock_worktree, test_issue_lock_adoption, test_issue_work_duplicate_gate, test_branch_cleanup_guard, test_task_capability_role_invariants, test_issue_claim_heartbeat, test_worktrees) — 383 passed, 98 subtests passed.
  • Full suite — 4295 passed, 11 failed, 6 skipped, 499 subtests passed.

Baseline comparison: a clean detached worktree at branches/baseline-master-issue-790, pinned to master 620ed6e9a9550b8da2ceb82d9ab8744e8920490f, reports 11 failed, 4240 passed, 6 skipped, 493 subtests passed running the identical command. The eleven failing node identifiers are byte-for-byte the ones on this branch: six in test_commit_payloads.py, two in test_issue_702_review_findings_f1_f6.py, and one each in test_mcp_server.py::TestPreflightVerification, test_post_merge_moot_lease.py, and test_reconciler_supersession_close.py. None touch lease, lock-freshness, or heartbeat code. The 55-test delta between 4240 and 4295 is exactly the new suites, so this branch adds no new failures. Both worktrees reported a zero-entry git status --porcelain before and after validation.

Risk

Behavioral change to the author lock lifecycle, so the risk concentrates in two places and both are covered.

Shortening the author TTL from 240 to 10 minutes could in principle make deployed claims reclaimable at restart. It cannot: every lock already on disk lacks the lifecycle marker and is therefore held to its recorded absolute expiry, and the short grace applies only to leases minted after this lands.

Loosening reclaim could in principle weaken a guard. The new reclaim ground is reachable only for heartbeat-lifecycle leases, never for a legacy lock, and every other precondition — cleanliness, ancestry, publication, worktree binding, the duplicate-work gate, the compare-and-swap — is unchanged.

One consequence worth stating plainly for review: an author session that runs longer than 10 minutes without calling gitea_heartbeat_issue_lock will have its claim become reclaimable. That is the intended behavior, and it is why the heartbeat cadence is 2 minutes, giving five missed beats before the grace lapses.

Known limitations

The heartbeat is a tool an author session must call; nothing in this slice schedules it automatically. Reviewer, merger, conflict-fix, and control-plane leases still run their own lifecycles until #793. verify_lock_for_mutation still does not check the generation, which is also #793.

Provenance

  • Issue: #790, reduced to Slice A by comment 14010
  • Follow-ups: #792 (Slice B), #793 (Slice C)
  • Branch: fix/issue-790-slice-a-heartbeat-policy
  • Worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-790-slice-a-heartbeat-policy
  • Commit: 243f52dc7959c362242ebea321fafa3950f13083
  • Base: master at 620ed6e9a9550b8da2ceb82d9ab8744e8920490f
Closes #790 Follow-up: #792 Follow-up: #793 ## This pull request implements Slice A only Issue #790 was reduced to Slice A by the scope transfer recorded in its comment 14010, so the closing reference above is accurate. The rest of the original scope was moved to dedicated issues **before** this pull request was opened: - **#792 — Slice B:** terminal author-lease retirement, the reconciler-only retirement operation, generation-compare-and-swap terminal status, the two-minute configurable race-drain, full revalidation before destructive cleanup. - **#793 — Slice C:** `lock_generation` verification in `verify_lock_for_mutation`, the Issue #760 renewal re-scope, remaining task-class policy alignment, control-plane lease alignment. Merging this closes #790 correctly. It does **not** complete #792 or #793, and #790 should not be reopened to represent them. ## Problem `issue_lock_store.assess_lock_freshness` parsed `last_heartbeat_at` and then never consulted it. Liveness was decided by an absolute four-hour `expires_at` and by process-identifier liveness, and the recorded identifier is the long-lived MCP daemon rather than the authoring task, so an abandoned claim stayed live for the full four hours. A tree-wide search found the field written in exactly one place, `gitea_mcp_server._build_author_issue_work_lease`, and advanced by nothing anywhere. Issue #787 / PR #789 hit this. Issue #760 / PR #791 hit it again: its lease was minted at `04:19:08Z`, never heartbeated, its recorded pid 39849 stayed alive as a daemon, and reconciliation was still blocked at `06:39:43Z` — more than five hours after the work landed on master. ## What changed **A1 — one authoritative policy source.** New `lease_policy` declares every duration for every task class: author initial and sliding TTL 10 minutes, heartbeat cadence 2, stale warning 5, missed-heartbeat grace 10, absolute cap 8 hours, recovery grace 10, terminal race-drain 2. It ships first so the first heartbeat and TTL behavior to run reads from it (AC-N7). The duplicated four-hour literal is deleted from both `issue_lock_store` and `gitea_mcp_server`. Reviewer, merger, and conflict-fix classes are declared but not rewired; a test asserts the declared numbers still match the constants `reviewer_pr_lease` and `pr_work_lease` own, so the two cannot drift apart before #793 migrates those call sites. **A2 — load-bearing freshness, with two intentional asymmetries.** An alive process identifier never establishes freshness anywhere (AC-N2): it is recorded as evidence and no branch returns live because of it. A dead one still marks a lease stale, and that band still precedes every heartbeat evaluation, so Issue #753 dead-session recovery keys on exactly the classification it always did. New bands `stale_missed_heartbeat` and `stale_absolute_cap` are classified in `branch_cleanup_guard` instead of falling through to unknown-status, and still block unless the ownership record proves `reclaim_allowed is True`. A heartbeat lease carrying no heartbeat is contradictory and fails closed. `assess_expired_lock_reclaim` accepts a lapsed heartbeat as reclaim grounds for heartbeat-lifecycle leases only — under this lifecycle the heartbeat is the liveness proof, and additionally demanding a dead process would reinstate the original defect. **A3 and A4 — task-session identity and the writer.** `mint_task_session_id` produces an ownership key containing no process identifier, because the daemon identifier is reused by every task it serves and identifies none of them. `heartbeat_session_lock` writes inside the existing per-issue flock under the Issue #772 generation compare-and-swap, verifying exact issue, branch, realpath-normalized worktree, claimant username, claimant profile, and recorded session identifier. It cannot acquire, take over, or revive: a lease past its grace is refused and must use the reclaim path, so a session that stopped proving liveness cannot restore ownership retroactively. New `gitea_heartbeat_issue_lock` gates on the authority `lock_issue` already requires, being a strictly narrower operation. **A5 — legacy compatibility (AC-N8).** An explicit `lifecycle_version` marker, never a timestamp comparison, discriminates legacy from heartbeat leases. A legacy lock has `last_heartbeat_at` matching `created_at` forever precisely because nothing advanced it, and a freshly minted heartbeat lease has them matching too, so that equality carries no information in either direction. Legacy locks keep their recorded absolute expiry and are never evaluated against the short grace, so deploying this cannot make an existing claim instantly reclaimable. They leave that state only through terminal retirement (#792) or `rebind_legacy_lock`, which re-verifies the exact owner and mints a genuine identifier and first heartbeat while preserving the original claim under `legacy_origin`. Rebinding a lapsed legacy lease is refused; that case belongs to Issue #760 renewal or Issue #601 reclaim. **A6 — native coverage.** Review #499 proved assessor-level tests miss discard points, so `tests/test_issue_790_heartbeat_mcp_path.py` drives the real tools against a real git repository and a real durable lock. ## Files changed | File | Change | |---|---| | `lease_policy.py` | new; the single authoritative duration source | | `issue_lock_store.py` | lifecycle helpers, session-id minting, rewritten freshness, reclaim on lapsed heartbeat, `heartbeat_session_lock`, `rebind_legacy_lock` | | `gitea_mcp_server.py` | policy-derived lease with lifecycle marker and session id; new `gitea_heartbeat_issue_lock` tool | | `branch_cleanup_guard.py` | classify the two new stale bands | | `task_capability_map.py` | `heartbeat_issue_lock` capability entry | | `docs/mcp-tool-inventory.md` | register the new tool in the documented inventory | | `tests/test_issue_790_lease_policy.py` | new, 40 cases | | `tests/test_issue_790_heartbeat_mcp_path.py` | new, 15 cases | 1974 insertions, 32 deletions. ## Non-regressions preserved Issue #760 exact-owner renewal is untouched and stays in force until #793 replaces it end to end. Issue #753 dead-session recovery, Issue #755 owning-pull-request evidence, Issue #772 generation compare-and-swap, and Issue #747 reviewer sliding TTL all keep their existing behavior, each covered by its own suite below. ## Tests and results Run with `venv/bin/python -m pytest` from the allocated worktree. - New suites — **55 passed** (`test_issue_790_lease_policy.py` 40, `test_issue_790_heartbeat_mcp_path.py` 15). - Lock and lease regression set (`test_issue_lock_store`, `test_lease_lifecycle`, `test_issue_753_dead_pid_lock_recovery`, `test_issue_755_owning_pr_recovery`, `test_issue_760_exact_owner_lease_renewal`, `test_issue_760_mcp_renewal_path`, `test_issue_768_strict_descendant_recovery`, `test_issue_772_unpublished_claim_recovery`, `test_lock_issue_mcp_registration`, `test_issue_lock_worktree`, `test_issue_lock_adoption`, `test_issue_work_duplicate_gate`, `test_branch_cleanup_guard`, `test_task_capability_role_invariants`, `test_issue_claim_heartbeat`, `test_worktrees`) — **383 passed, 98 subtests passed**. - Full suite — **4295 passed, 11 failed, 6 skipped, 499 subtests passed**. Baseline comparison: a clean detached worktree at `branches/baseline-master-issue-790`, pinned to master `620ed6e9a9550b8da2ceb82d9ab8744e8920490f`, reports **11 failed, 4240 passed, 6 skipped, 493 subtests passed** running the identical command. The eleven failing node identifiers are byte-for-byte the ones on this branch: six in `test_commit_payloads.py`, two in `test_issue_702_review_findings_f1_f6.py`, and one each in `test_mcp_server.py::TestPreflightVerification`, `test_post_merge_moot_lease.py`, and `test_reconciler_supersession_close.py`. None touch lease, lock-freshness, or heartbeat code. The 55-test delta between 4240 and 4295 is exactly the new suites, so this branch adds no new failures. Both worktrees reported a zero-entry `git status --porcelain` before and after validation. ## Risk Behavioral change to the author lock lifecycle, so the risk concentrates in two places and both are covered. Shortening the author TTL from 240 to 10 minutes could in principle make deployed claims reclaimable at restart. It cannot: every lock already on disk lacks the lifecycle marker and is therefore held to its recorded absolute expiry, and the short grace applies only to leases minted after this lands. Loosening reclaim could in principle weaken a guard. The new reclaim ground is reachable only for heartbeat-lifecycle leases, never for a legacy lock, and every other precondition — cleanliness, ancestry, publication, worktree binding, the duplicate-work gate, the compare-and-swap — is unchanged. One consequence worth stating plainly for review: an author session that runs longer than 10 minutes without calling `gitea_heartbeat_issue_lock` will have its claim become reclaimable. That is the intended behavior, and it is why the heartbeat cadence is 2 minutes, giving five missed beats before the grace lapses. ## Known limitations The heartbeat is a tool an author session must call; nothing in this slice schedules it automatically. Reviewer, merger, conflict-fix, and control-plane leases still run their own lifecycles until #793. `verify_lock_for_mutation` still does not check the generation, which is also #793. ## Provenance - Issue: #790, reduced to Slice A by comment 14010 - Follow-ups: #792 (Slice B), #793 (Slice C) - Branch: `fix/issue-790-slice-a-heartbeat-policy` - Worktree: `/Users/jasonwalker/Development/Gitea-Tools/branches/issue-790-slice-a-heartbeat-policy` - Commit: `243f52dc7959c362242ebea321fafa3950f13083` - Base: master at `620ed6e9a9550b8da2ceb82d9ab8744e8920490f`
jcwalker3 added 1 commit 2026-07-22 03:31:26 -05:00
Slice A of Issue #790, per the controller reassessment in comment 13958. Does
not close the issue: terminal retirement (Slice B) and the read-side generation
check plus the #760 renewal re-scope (Slice C) are deliberately not implemented.

The defect. `issue_lock_store.assess_lock_freshness` parsed `last_heartbeat_at`
and then never consulted it. Liveness was decided by an absolute four-hour
`expires_at` and by PID liveness, and the recorded PID is the long-lived MCP
daemon rather than the authoring task, so an abandoned claim stayed live for the
full four hours. A tree-wide search found the field written in exactly one place
and advanced by nothing. Issue #787 / PR #789 hit this; Issue #760 / PR #791 hit
it again, blocking reconciliation for over five hours after its work had landed.

A1 — central policy. New `lease_policy` declares every duration for every task
class in one place: author initial/sliding TTL 10 minutes, heartbeat cadence 2,
stale warning 5, missed-heartbeat grace 10, absolute cap 8 hours, recovery grace
10, terminal race-drain 2. It ships first so the first heartbeat and TTL
behavior to run reads from it (AC-N7). The duplicated four-hour literal is gone
from both `issue_lock_store` and `gitea_mcp_server`. Reviewer, merger, and
conflict-fix classes are declared but not rewired — Slice C moves those call
sites — and a test asserts the declaration still equals the constants #747 and
`pr_work_lease` own, so the two cannot drift apart unnoticed.

A2 — load-bearing freshness, with two deliberate asymmetries. An alive PID never
establishes freshness anywhere (AC-N2); it is recorded as evidence and no branch
returns live because of it. A dead PID still marks a lease stale, and that band
still precedes every heartbeat evaluation, so #753 dead-session recovery keys on
exactly the classification it always did. New bands `stale_missed_heartbeat` and
`stale_absolute_cap` are classified in `branch_cleanup_guard` rather than
falling through to unknown-status, and still block unless the ownership record
proves `reclaim_allowed is True`. A heartbeat lease carrying no heartbeat is
contradictory and fails closed. `assess_expired_lock_reclaim` accepts a lapsed
heartbeat as reclaim grounds for heartbeat-lifecycle leases only: under this
lifecycle the heartbeat is the liveness proof, and also requiring a dead PID
would reinstate the original defect.

A3/A4 — task-session identity and the writer. `mint_task_session_id` produces an
ownership key containing no process identifier, since the daemon PID is reused
by every task it serves and identifies none of them. `heartbeat_session_lock`
writes inside the existing per-issue flock under the #772 generation
compare-and-swap, verifying exact issue, branch, realpath-normalized worktree,
claimant username, claimant profile, and recorded session identifier. It cannot
acquire, take over, or revive: a lease past its grace is refused and must use
the reclaim path, so a session that stopped proving liveness cannot restore
ownership retroactively. New `gitea_heartbeat_issue_lock` gates on the same
authority as `lock_issue`, being strictly narrower.

A5 — legacy compatibility (AC-N8). The explicit `lifecycle_version` marker, never
a timestamp comparison, discriminates legacy from heartbeat leases: a legacy lock
has `last_heartbeat_at == created_at` forever precisely because nothing advanced
it, and a freshly minted heartbeat lease has them equal too, so the equality
carries no information in either direction. Legacy locks keep their recorded
absolute expiry and are never evaluated against the short grace, so deployment
cannot make an existing claim instantly reclaimable. They leave that state only
by terminal retirement (Slice B) or by `rebind_legacy_lock`, which re-verifies
the exact owner and mints a genuine identifier and first heartbeat while
preserving the original claim under `legacy_origin`. Rebinding a lapsed legacy
lease is refused; that belongs to #760 renewal or #601 reclaim.

A6 — native coverage. Review #499 proved assessor-level tests miss discard
points, so `tests/test_issue_790_heartbeat_mcp_path.py` drives the real tools
against a real git repository and a real durable lock: lock creation and
read-back, policy window, freshness, survival of `verify_lock_for_mutation`,
invariance of the duplicate-work and linked-open-PR gates, CAS rejection,
foreign-session and foreign-claimant refusal, alive-PID-only refusal, missed
heartbeat, legacy protection on deployment, and legacy rebinding.

Tests. New suites 55 passed. Lock and lease regression set (issue_lock_store,
lease_lifecycle, #753, #755, #760 x2, #768, #772, lock registration, worktree,
adoption, duplicate gate, branch cleanup guard, capability invariants, claim
heartbeat, worktrees) 383 passed with 98 subtests. Full suite 4295 passed, 11
failed, 6 skipped, 499 subtests passed, against a clean master baseline worktree
at 620ed6e9 that reports 11 failed and 4240 passed — the same eleven node IDs.
The 55-test delta is exactly the new suites; no new failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011u6GKSJwwrrYjguPjs1aK5
Author
Owner

CTH: Author Handoff

Status: published_awaiting_independent_review
Next owner: reviewer (prgs-reviewer / sysadmin)
Current blocker: none
Decision: PR #794 implements Slice A of #790 only; ready for independent review
Proof: head 243f52dc7959c362242ebea321fafa3950f13083 on fix/issue-790-slice-a-heartbeat-policy; base master; Closes #790; follow-ups #792/#793 are out of scope
Next action: independent formal review at exact head 243f52dc7959c362242ebea321fafa3950f13083
Ready-to-paste prompt: see NEXT_PROMPT below

Canonical Issue State

STATE: pr_open_awaiting_review
WHO_IS_NEXT: reviewer
NEXT_ACTION: formal independent review of PR #794 at head 243f52dc79
NEXT_PROMPT:

Review PR #794 (Closes #790, Slice A) on prgs Scaled-Tech-Consulting/Gitea-Tools as prgs-reviewer.
Pin head 243f52dc7959c362242ebea321fafa3950f13083 on fix/issue-790-slice-a-heartbeat-policy targeting master.
Confirm scope is Slice A only (policy + load-bearing heartbeat + session id + legacy marker); #792/#793 remain separate.
Re-run focused lease suites and full suite; compare to master baseline failures.
Submit one formal APPROVE or REQUEST_CHANGES; do not merge.

WHAT_HAPPENED: Canonical author handoff posted for already-published PR #794 (incomplete handoff). Implementation and validation evidence are in the PR body.
WHY: Queue hygiene — open PR lacked CTH handoff so reviewer ownership was unclear.
RELATED_PRS: #794 (this PR); follow-ups #792, #793
BLOCKERS: none
VALIDATION: See PR body — new suites 55 passed; lock/lease regression 383 passed; full suite 4295 passed / 11 baseline fail vs master 4240/11.
LAST_UPDATED_BY: jcwalker3 (prgs-author)
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
ISSUE: #790
PR: #794
HEAD_SHA: 243f52dc79
BASE_BRANCH: master
ACTING_ROLE: author
ACTING_IDENTITY: jcwalker3

<!-- cth:v1 --> ## CTH: Author Handoff Status: published_awaiting_independent_review Next owner: reviewer (prgs-reviewer / sysadmin) Current blocker: none Decision: PR #794 implements Slice A of #790 only; ready for independent review Proof: head `243f52dc7959c362242ebea321fafa3950f13083` on `fix/issue-790-slice-a-heartbeat-policy`; base master; Closes #790; follow-ups #792/#793 are out of scope Next action: independent formal review at exact head `243f52dc7959c362242ebea321fafa3950f13083` Ready-to-paste prompt: see NEXT_PROMPT below ## Canonical Issue State STATE: pr_open_awaiting_review WHO_IS_NEXT: reviewer NEXT_ACTION: formal independent review of PR #794 at head 243f52dc7959c362242ebea321fafa3950f13083 NEXT_PROMPT: ```text Review PR #794 (Closes #790, Slice A) on prgs Scaled-Tech-Consulting/Gitea-Tools as prgs-reviewer. Pin head 243f52dc7959c362242ebea321fafa3950f13083 on fix/issue-790-slice-a-heartbeat-policy targeting master. Confirm scope is Slice A only (policy + load-bearing heartbeat + session id + legacy marker); #792/#793 remain separate. Re-run focused lease suites and full suite; compare to master baseline failures. Submit one formal APPROVE or REQUEST_CHANGES; do not merge. ``` WHAT_HAPPENED: Canonical author handoff posted for already-published PR #794 (incomplete handoff). Implementation and validation evidence are in the PR body. WHY: Queue hygiene — open PR lacked CTH handoff so reviewer ownership was unclear. RELATED_PRS: #794 (this PR); follow-ups #792, #793 BLOCKERS: none VALIDATION: See PR body — new suites 55 passed; lock/lease regression 383 passed; full suite 4295 passed / 11 baseline fail vs master 4240/11. LAST_UPDATED_BY: jcwalker3 (prgs-author) REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools ISSUE: #790 PR: #794 HEAD_SHA: 243f52dc7959c362242ebea321fafa3950f13083 BASE_BRANCH: master ACTING_ROLE: author ACTING_IDENTITY: jcwalker3
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #794
issue: #790
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 89687-276bb9edcbbf
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-794
phase: claimed
candidate_head: 243f52dc79
target_branch: master
target_branch_sha: 620ed6e9a9
last_activity: 2026-07-22T10:52:21Z
expires_at: 2026-07-22T11:02:21Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #794 issue: #790 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 89687-276bb9edcbbf worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-794 phase: claimed candidate_head: 243f52dc7959c362242ebea321fafa3950f13083 target_branch: master target_branch_sha: 620ed6e9a9550b8da2ceb82d9ab8744e8920490f last_activity: 2026-07-22T10:52:21Z expires_at: 2026-07-22T11:02:21Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #794
issue: #790
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50914-eb6bb39eb08a
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-794
phase: claimed
candidate_head: 243f52dc79
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T11:05:47Z
expires_at: 2026-07-22T11:15:47Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #794 issue: #790 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50914-eb6bb39eb08a worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-794 phase: claimed candidate_head: 243f52dc7959c362242ebea321fafa3950f13083 target_branch: master target_branch_sha: none last_activity: 2026-07-22T11:05:47Z expires_at: 2026-07-22T11:15:47Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #794
issue: #790
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 50914-eb6bb39eb08a
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-794
phase: validation
candidate_head: 243f52dc79
target_branch: master
target_branch_sha: none
last_activity: 2026-07-22T11:07:43Z
expires_at: 2026-07-22T11:17:43Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #794 issue: #790 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 50914-eb6bb39eb08a worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-794 phase: validation candidate_head: 243f52dc7959c362242ebea321fafa3950f13083 target_branch: master target_branch_sha: none last_activity: 2026-07-22T11:07:43Z expires_at: 2026-07-22T11:17:43Z blocker: none
sysadmin requested changes 2026-07-22 06:08:00 -05:00
sysadmin left a comment
Owner

Canonical PR State

STATE: changes-requested
WHO_IS_NEXT: author
NEXT_ACTION: Fix assess_same_issue_lease_conflict so heartbeat-lifecycle non-live bands (stale_missed_heartbeat, stale_absolute_cap) enter the reclaim path even when expires_at is still in the future; add regression tests; re-request review.
NEXT_PROMPT:

Address PR #794 review: in issue_lock_store.assess_same_issue_lease_conflict, for heartbeat-lifecycle leases enter the existing reclaim branch when not is_lease_live (or status in stale_missed_heartbeat/stale_absolute_cap), not only when is_lease_expired. Keep legacy on absolute expires_at. Add tests for absolute-cap + future expires + foreign reclaim, grace-lapsed + future expires (TTL>grace) + foreign reclaim, and live lease still blocks foreign. Keep Slice A scope. Re-run test_issue_790_* and lock regression suites.

WHAT_HAPPENED: Independent review of PR #794 (Slice A author heartbeat policy) by sysadmin/prgs-reviewer.
WHY: assess_expired_lock_reclaim allows reclaim for the new stale bands, but assess_same_issue_lease_conflict still keys foreign reclaim on is_lease_expired (expires_at only). Absolute-cap with a recent heartbeat, and TTL>grace env overrides, leave foreign reclaim blocked as active.
ISSUE: #790
HEAD_SHA: 243f52dc79
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: no
BLOCKERS: conflict gate does not honor heartbeat-lifecycle non-live reclaim bands when expires_at is still future
VALIDATION: pytest test_issue_790_lease_policy + test_issue_790_heartbeat_mcp_path = 55 passed; lock/lease/#753/#760/branch_cleanup/capability regression = 173 passed; manual probes confirmed absolute-cap and TTL>grace conflict-gate hole
LAST_UPDATED_BY: sysadmin (prgs-reviewer)


Independent review detail

Verdict: REQUEST_CHANGES

One correctness hole in the acquisition conflict gate relative to the new freshness/reclaim semantics. Everything else in Slice A looks solid.

What looks good

  • Scope discipline (Slice A only; #792/#793 carved out).
  • lease_policy as AC-N7 source; env overrides fail closed; lifecycle marker (AC-N8) not timestamp heuristics.
  • Freshness asymmetries (AC-N2): alive PID never grants live; dead PID still stale for #753.
  • heartbeat_session_lock under flock + generation CAS; no acquire/takeover/revive; auto-rebind of legacy.
  • Tests: 55 new + 173 regression green; native MCP path coverage present.

Default-path missed-heartbeat reclaim works when expires_at and grace align (defaults 10/10).

Blocking: conflict gate still keys off expires_at only

assess_expired_lock_reclaim allows reclaim for stale_missed_heartbeat and stale_absolute_cap without a dead PID.

assess_same_issue_lease_conflict only enters that reclaim branch when is_lease_expired (expires_at only). When freshness is non-live and expires_at is still future, foreign acquisition is rejected as active.

Reproduced on this head:

  1. Absolute cap (defaults): created >8h ago, heartbeated now → stale_absolute_cap, live=False, reclaim_allowed=True, is_lease_expired=False → foreign conflict returns active-lease block.
  2. TTL/grace divergence via independent env overrides: same hole for stale_missed_heartbeat with future expires_at.

assess_foreign_lock_overwrite already uses is_lease_live; the same-issue path is inconsistent. gitea_lock_issue raises before bind.

Suggested fix: for heartbeat-lifecycle leases, enter reclaim/renewal when not is_lease_live (or status in the new bands), not only when is_lease_expired. Keep legacy on absolute expires_at. Add regression tests for the three cases above.

Non-blocking

  1. No author skill/runbook call sites for gitea_heartbeat_issue_lock (known limitation; operational thrash risk).
  2. workflow_dashboard._partition_leases does not name the new stale bands.
  3. lock_issue nests task_session_id under work_lease only.
  4. Base is 620ed6e; master has moved — re-run new suites on current master before merge.

Happy to re-review once the conflict-gate wiring matches the new reclaim bands.

## Canonical PR State STATE: changes-requested WHO_IS_NEXT: author NEXT_ACTION: Fix assess_same_issue_lease_conflict so heartbeat-lifecycle non-live bands (stale_missed_heartbeat, stale_absolute_cap) enter the reclaim path even when expires_at is still in the future; add regression tests; re-request review. NEXT_PROMPT: ```text Address PR #794 review: in issue_lock_store.assess_same_issue_lease_conflict, for heartbeat-lifecycle leases enter the existing reclaim branch when not is_lease_live (or status in stale_missed_heartbeat/stale_absolute_cap), not only when is_lease_expired. Keep legacy on absolute expires_at. Add tests for absolute-cap + future expires + foreign reclaim, grace-lapsed + future expires (TTL>grace) + foreign reclaim, and live lease still blocks foreign. Keep Slice A scope. Re-run test_issue_790_* and lock regression suites. ``` WHAT_HAPPENED: Independent review of PR #794 (Slice A author heartbeat policy) by sysadmin/prgs-reviewer. WHY: assess_expired_lock_reclaim allows reclaim for the new stale bands, but assess_same_issue_lease_conflict still keys foreign reclaim on is_lease_expired (expires_at only). Absolute-cap with a recent heartbeat, and TTL>grace env overrides, leave foreign reclaim blocked as active. ISSUE: #790 HEAD_SHA: 243f52dc7959c362242ebea321fafa3950f13083 REVIEW_STATUS: REQUEST_CHANGES MERGE_READY: no BLOCKERS: conflict gate does not honor heartbeat-lifecycle non-live reclaim bands when expires_at is still future VALIDATION: pytest test_issue_790_lease_policy + test_issue_790_heartbeat_mcp_path = 55 passed; lock/lease/#753/#760/branch_cleanup/capability regression = 173 passed; manual probes confirmed absolute-cap and TTL>grace conflict-gate hole LAST_UPDATED_BY: sysadmin (prgs-reviewer) --- ## Independent review detail ### Verdict: REQUEST_CHANGES One correctness hole in the acquisition conflict gate relative to the new freshness/reclaim semantics. Everything else in Slice A looks solid. ### What looks good - Scope discipline (Slice A only; #792/#793 carved out). - lease_policy as AC-N7 source; env overrides fail closed; lifecycle marker (AC-N8) not timestamp heuristics. - Freshness asymmetries (AC-N2): alive PID never grants live; dead PID still stale for #753. - heartbeat_session_lock under flock + generation CAS; no acquire/takeover/revive; auto-rebind of legacy. - Tests: 55 new + 173 regression green; native MCP path coverage present. Default-path missed-heartbeat reclaim works when expires_at and grace align (defaults 10/10). ### Blocking: conflict gate still keys off expires_at only assess_expired_lock_reclaim allows reclaim for stale_missed_heartbeat and stale_absolute_cap without a dead PID. assess_same_issue_lease_conflict only enters that reclaim branch when is_lease_expired (expires_at only). When freshness is non-live and expires_at is still future, foreign acquisition is rejected as active. Reproduced on this head: 1. Absolute cap (defaults): created >8h ago, heartbeated now → stale_absolute_cap, live=False, reclaim_allowed=True, is_lease_expired=False → foreign conflict returns active-lease block. 2. TTL/grace divergence via independent env overrides: same hole for stale_missed_heartbeat with future expires_at. assess_foreign_lock_overwrite already uses is_lease_live; the same-issue path is inconsistent. gitea_lock_issue raises before bind. Suggested fix: for heartbeat-lifecycle leases, enter reclaim/renewal when not is_lease_live (or status in the new bands), not only when is_lease_expired. Keep legacy on absolute expires_at. Add regression tests for the three cases above. ### Non-blocking 1. No author skill/runbook call sites for gitea_heartbeat_issue_lock (known limitation; operational thrash risk). 2. workflow_dashboard._partition_leases does not name the new stale bands. 3. lock_issue nests task_session_id under work_lease only. 4. Base is 620ed6e; master has moved — re-run new suites on current master before merge. Happy to re-review once the conflict-gate wiring matches the new reclaim bands.
You are not authorized to merge this pull request.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/issue-790-slice-a-heartbeat-policy:fix/issue-790-slice-a-heartbeat-policy
git checkout fix/issue-790-slice-a-heartbeat-policy
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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