[review][bug] Expired old-head decision lock blocks fresh review on open PR with no configured recovery path #720

New Issue

2026-07-16T14:32:38-05:00

jcwalker3 commented

2026-07-16 14:32:38 -05:00

Problem

An open PR can retain a durable KIND_DECISION_LOCK ledger for an old-head terminal review. After the generic session-state four-hour TTL elapses, that ledger still blocks a prepared independent re-review at a corrected new head, even when assessment reports fresh_review_on_current_head_allowed: true.

Moot cleanup remains forbidden while the PR is open (#594). Irrecoverable recovery requires gitea.decision_lock.irrecoverable_recovery, but no configured ordinary profile holds that permission, so there is no reachable sanctioned recovery path.

Confirmed reproduction (PR #616)

Fact	Value
PR	#616 remains open
Prior formal review	Review 443 `REQUEST_CHANGES` at old head `a0fffae576673ba7df7456b32e6aec916581bdfb`
Author remediation head	`a6a2243aad9c3e385fc70509f4941a0f0ec33162`
Stale prior review	Old-head review became stale after remediation
Independent re-review	Passed: 9 focused tests and 41 documentation tests
Prepared verdict	`APPROVE` prepared but could not be submitted
Durable ledger age	Older than the four-hour default TTL
Mark final decision	`gitea_mark_final_review_decision` failed with "session state expired after 4h"
Fresh-head allowance	`fresh_review_on_current_head_allowed` was true but unreachable (TTL rejection wins first)
Moot cleanup	Forbidden because the PR remained open
Expired evidence reporting	Expired evidence could surface as no loadable lock even though the ledger remained on disk
Irrecoverable recovery	Required `gitea.decision_lock.irrecoverable_recovery`
Profile gap	No ordinary author / reviewer / merger / general reconciler profile held that permission → no configured recovery path

Defect classes

KIND_DECISION_LOCK lacks dedicated lifecycle semantics and is treated like generic session state with a four-hour TTL that is unsuitable for terminal review provenance.
Ordering bug: TTL / "session state expired" rejection runs before the head-scoped fresh-review allowance (fresh_review_on_current_head_allowed) can take effect.
Reporting bug: expired-but-present lock evidence can be reported as if no lock loads, understating on-disk terminal history.
Recovery gap: open-PR old-head expiry cannot use #594 moot cleanup; exceptional recovery is permission-gated to a capability no operational profile is configured to use.
Historical evidence must remain durable; recovery must not erase old-head review 443 / Gitea history.

Required behavior

Give KIND_DECISION_LOCK explicit lifecycle semantics instead of an unsuitable generic four-hour TTL.
Preserve historical old-head review evidence (e.g. review 443 at a0fffae…).
Permit a normally gated independent review at a corrected new head (e.g. a6a2243…) with a fresh lease and new terminal verdict.
Make fresh_review_on_current_head_allowed effective before TTL rejection when the live head differs from the locked old head.
Report expired lock evidence accurately (on-disk present vs truly absent; expired vs loadable; head mismatch).
Provide a sanctioned recovery path for already-affected records (open PR, advanced head, age over TTL, stale prior review).
Exceptional recovery must use a dedicated immutable controller profile and namespace with server-minted, incident-bound authorization and a durable audit record.
Do not grant gitea.decision_lock.irrecoverable_recovery (or equivalent) to ordinary author, reviewer, merger, or general reconciler profiles.
Add regression coverage for: open PR + advanced head + stale prior review + age over four hours + fresh lease + new terminal verdict.
Preserve merged/closed moot-cleanup behavior (#594) and return structured actionable errors (never silent no-op or bare "no lock" when evidence remains).

Acceptance criteria

KIND_DECISION_LOCK has explicit lifecycle semantics; generic four-hour TTL is not the sole lifecycle for terminal decision ledgers.
Historical old-head review evidence is preserved (Gitea review + durable local provenance as designed).
A normally gated independent review is permitted at a corrected new head of an open PR.
When assessment would set fresh_review_on_current_head_allowed: true, that path is reachable before generic TTL rejection blocks gitea_mark_final_review_decision / formal submit.
Expired lock evidence is reported accurately (present-on-disk, expired, locked head vs live head, cleanup/recovery next action).
Already-affected records have a sanctioned recovery path that does not require manual session-state file deletion.
Exceptional recovery uses a dedicated immutable controller profile/namespace, server-minted incident-bound authorization, and an audit record.
Ordinary author, reviewer, merger, and general reconciler profiles do not receive the irrecoverable recovery permission.
Regression tests cover open PR, advanced head, stale prior review, age over four hours, fresh lease, and new terminal verdict.
Merged/closed moot-cleanup behavior is preserved; failures return structured actionable errors.

Related (not true duplicates)

Issue	Why related	Why not the same work
#332 (closed)	Terminal hard-stop after live review mutation	Session multi-PR budget; not open-PR TTL + fresh-head allowance
#594 (closed)	Moot cleanup after merged/closed PRs	Explicitly preserves #332 for open PRs; cannot clear this case
#620 (closed)	Head-scoped fresh re-review after REQUEST_CHANGES	Introduced/intended fresh-head allowance; does not fix TTL-first rejection or missing recovery permission
#693 (open)	No canonical recovery for stuck open-PR decision locks	Broader recovery/audit gaps; not this confirmed TTL + `fresh_review_on_current_head_allowed` unreachable path
#709 (closed)	Cross-profile merger cleanup / init overwrite of terminal evidence	Different failure mode (profile-local cleanup / overwrite)
#717 (open)	TTL causes silent overwrite of unresolved terminal ledger after merge (#710)	Overwrite-on-init integrity for merged work; not open-PR fresh formal review blocked by expired old-head lock
#718 (open)	Merger lease expiry / adopt `internal_error`	Lease path, not decision-lock TTL
#719 (open)	Thin incident handoff for review 443 / PR #616 recovery	Operational recovery ticket only; not this product defect + AC set

Non-goals

Mutating PR #616, review 443, leases, preserved commits, or forensic evidence from this filing alone
Weakening #332 for same-head duplicate terminal mutations
Granting irrecoverable recovery to ordinary author/reviewer/merger/reconciler profiles
Manual rm of session-state files as a normal workflow
Touching chore/preserve-local-master-54753d4 or external forensic artifacts

Filing metadata

Remote: prgs / Scaled-Tech-Consulting / Gitea-Tools
Control master at filing prep: 77746b08fc19e20abb95e4171784e321cb88817a
Namespace: native gitea-tools / profile prgs-author / identity jcwalker3
Incident cross-ref: #719 (recovery handoff); product fix tracked here

## Problem An **open** PR can retain a durable `KIND_DECISION_LOCK` ledger for an **old-head** terminal review. After the generic session-state **four-hour TTL** elapses, that ledger still blocks a prepared independent re-review at a **corrected new head**, even when assessment reports `fresh_review_on_current_head_allowed: true`. Moot cleanup remains forbidden while the PR is open (#594). Irrecoverable recovery requires `gitea.decision_lock.irrecoverable_recovery`, but **no configured ordinary profile holds that permission**, so there is **no reachable sanctioned recovery path**. ## Confirmed reproduction (PR #616) | Fact | Value | |------|--------| | PR | **#616** remains **open** | | Prior formal review | Review **443** `REQUEST_CHANGES` at old head `a0fffae576673ba7df7456b32e6aec916581bdfb` | | Author remediation head | `a6a2243aad9c3e385fc70509f4941a0f0ec33162` | | Stale prior review | Old-head review became stale after remediation | | Independent re-review | Passed: **9** focused tests and **41** documentation tests | | Prepared verdict | `APPROVE` prepared but **could not be submitted** | | Durable ledger age | Older than the **four-hour** default TTL | | Mark final decision | `gitea_mark_final_review_decision` failed with **"session state expired after 4h"** | | Fresh-head allowance | `fresh_review_on_current_head_allowed` was **true** but **unreachable** (TTL rejection wins first) | | Moot cleanup | **Forbidden** because the PR remained open | | Expired evidence reporting | Expired evidence could surface as **no loadable lock** even though the ledger **remained on disk** | | Irrecoverable recovery | Required `gitea.decision_lock.irrecoverable_recovery` | | Profile gap | **No** ordinary author / reviewer / merger / general reconciler profile held that permission → **no configured recovery path** | ## Defect classes 1. **`KIND_DECISION_LOCK` lacks dedicated lifecycle semantics** and is treated like generic session state with a four-hour TTL that is unsuitable for terminal review provenance. 2. **Ordering bug:** TTL / "session state expired" rejection runs **before** the head-scoped fresh-review allowance (`fresh_review_on_current_head_allowed`) can take effect. 3. **Reporting bug:** expired-but-present lock evidence can be reported as if no lock loads, understating on-disk terminal history. 4. **Recovery gap:** open-PR old-head expiry cannot use #594 moot cleanup; exceptional recovery is permission-gated to a capability **no operational profile is configured to use**. 5. **Historical evidence** must remain durable; recovery must not erase old-head review 443 / Gitea history. ## Required behavior 1. Give `KIND_DECISION_LOCK` **explicit lifecycle semantics** instead of an unsuitable generic four-hour TTL. 2. Preserve historical old-head review evidence (e.g. review 443 at `a0fffae…`). 3. Permit a normally gated independent review at a corrected new head (e.g. `a6a2243…`) with a fresh lease and new terminal verdict. 4. Make `fresh_review_on_current_head_allowed` **effective before** TTL rejection when the live head differs from the locked old head. 5. Report expired lock evidence **accurately** (on-disk present vs truly absent; expired vs loadable; head mismatch). 6. Provide a **sanctioned recovery path** for already-affected records (open PR, advanced head, age over TTL, stale prior review). 7. Exceptional recovery must use a **dedicated immutable controller profile and namespace** with **server-minted, incident-bound authorization** and a durable **audit record**. 8. **Do not** grant `gitea.decision_lock.irrecoverable_recovery` (or equivalent) to ordinary author, reviewer, merger, or general reconciler profiles. 9. Add regression coverage for: open PR + advanced head + stale prior review + age over four hours + fresh lease + new terminal verdict. 10. Preserve merged/closed **moot-cleanup** behavior (#594) and return **structured actionable errors** (never silent no-op or bare "no lock" when evidence remains). ## Acceptance criteria 1. `KIND_DECISION_LOCK` has explicit lifecycle semantics; generic four-hour TTL is not the sole lifecycle for terminal decision ledgers. 2. Historical old-head review evidence is preserved (Gitea review + durable local provenance as designed). 3. A normally gated independent review is permitted at a corrected new head of an open PR. 4. When assessment would set `fresh_review_on_current_head_allowed: true`, that path is reachable **before** generic TTL rejection blocks `gitea_mark_final_review_decision` / formal submit. 5. Expired lock evidence is reported accurately (present-on-disk, expired, locked head vs live head, cleanup/recovery next action). 6. Already-affected records have a sanctioned recovery path that does not require manual session-state file deletion. 7. Exceptional recovery uses a dedicated immutable controller profile/namespace, server-minted incident-bound authorization, and an audit record. 8. Ordinary author, reviewer, merger, and general reconciler profiles **do not** receive the irrecoverable recovery permission. 9. Regression tests cover open PR, advanced head, stale prior review, age over four hours, fresh lease, and new terminal verdict. 10. Merged/closed moot-cleanup behavior is preserved; failures return structured actionable errors. ## Related (not true duplicates) | Issue | Why related | Why not the same work | |-------|-------------|------------------------| | #332 (closed) | Terminal hard-stop after live review mutation | Session multi-PR budget; not open-PR TTL + fresh-head allowance | | #594 (closed) | Moot cleanup after merged/closed PRs | Explicitly preserves #332 for **open** PRs; cannot clear this case | | #620 (closed) | Head-scoped fresh re-review after REQUEST_CHANGES | Introduced/intended fresh-head allowance; does not fix TTL-first rejection or missing recovery permission | | #693 (open) | No canonical recovery for stuck open-PR decision locks | Broader recovery/audit gaps; not this confirmed TTL + `fresh_review_on_current_head_allowed` unreachable path | | #709 (closed) | Cross-profile merger cleanup / init overwrite of terminal evidence | Different failure mode (profile-local cleanup / overwrite) | | #717 (open) | TTL causes silent overwrite of unresolved terminal ledger after merge (#710) | Overwrite-on-init integrity for **merged** work; not open-PR fresh formal review blocked by expired old-head lock | | #718 (open) | Merger lease expiry / adopt `internal_error` | Lease path, not decision-lock TTL | | #719 (open) | Thin incident handoff for review 443 / PR #616 recovery | Operational recovery ticket only; not this product defect + AC set | ## Non-goals * Mutating PR #616, review 443, leases, preserved commits, or forensic evidence from this filing alone * Weakening #332 for same-head duplicate terminal mutations * Granting irrecoverable recovery to ordinary author/reviewer/merger/reconciler profiles * Manual `rm` of session-state files as a normal workflow * Touching `chore/preserve-local-master-54753d4` or external forensic artifacts ## Filing metadata * Remote: `prgs` / `Scaled-Tech-Consulting` / `Gitea-Tools` * Control master at filing prep: `77746b08fc19e20abb95e4171784e321cb88817a` * Namespace: native `gitea-tools` / profile `prgs-author` / identity `jcwalker3` * Incident cross-ref: #719 (recovery handoff); product fix tracked here

jcwalker3 added the mcp reliability status:ready workflow-hardening recovery terminal-lock reviewer safety type:bug labels 2026-07-16 14:32:38 -05:00

jcwalker3 added status:in-progress and removed status:ready labels 2026-07-16 14:44:48 -05:00

jcwalker3 commented

2026-07-16 14:44:48 -05:00

Issue claim heartbeat

kind: claim
issue: [review][bug] Expired old-head decision lock blocks fresh review on open PR with no configured recovery path (#720)
branch: fix/issue-720-expired-decision-lock
phase: claimed
profile: prgs-author
pr: none
blocker: none
next_action: create worktree and begin implementation

**Issue claim heartbeat** - kind: claim - issue: #720 - branch: fix/issue-720-expired-decision-lock - phase: claimed - profile: prgs-author - pr: none - blocker: none - next_action: create worktree and begin implementation

jcwalker3 referenced this issue from a commit

2026-07-16 14:47:59 -05:00

fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720)

jcwalker3 referenced a pull request that will close this issue

2026-07-16 14:48:12 -05:00

fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720) #721

jcwalker3 added status:pr-open and removed status:in-progress labels 2026-07-16 14:48:13 -05:00

jcwalker3 commented

2026-07-16 14:48:26 -05:00

Canonical author handoff — Issue #720 / PR #721

STATE: Author implementation complete; awaiting independent review
WHO_IS_NEXT: reviewer
NEXT_ACTION: Independent review of PR #721 at pinned head (do not self-review/merge)
NEXT_PROMPT:
  Review PR #721 (Closes #720) at expected_head_sha=80f59b334e6671b08006725292c08a8e8b6c823f
  from a reviewer namespace/profile distinct from jcwalker3/prgs-author.
WHAT_HAPPENED: Implemented recovery-critical lifecycle for KIND_DECISION_LOCK so expired old-head evidence no longer blocks fresh formal review on a newer open-PR head; PR #721 opened.
WHY: Generic 4h session-state TTL rejected decision ledgers before #620 fresh_review_on_current_head_allowed could apply (PR #616 / review 443 reproduction).
RELATED_PRS: 721 (implementation), 616 (concrete reproduction; not modified)
IMMUTABLE_HEAD: 80f59b334e6671b08006725292c08a8e8b6c823f
BRANCH: fix/issue-720-expired-decision-lock
WORKTREE: branches/issue-720-expired-decision-lock
BASE: master @ 77746b08fc19e20abb95e4171784e321cb88817a
VALIDATION: focused 228 passed; full suite 2898 passed, 6 skipped, 1 warning, 173 subtests passed
BLOCKERS: None
LAST_UPDATED_BY: prgs-author / jcwalker3

Delivered

KIND_DECISION_LOCK ∈ RECOVERY_CRITICAL_KINDS (terminal provenance survives generic TTL)
inspect_state_envelope + cleanup disk_inspect for accurate on-disk assessment
Regression suite tests/test_issue_720_expired_decision_lock.py
Same-head #332, #620 head-scope, #594 moot cleanup, #709 irrecoverable auth preserved
No ordinary-profile grant of gitea.decision_lock.irrecoverable_recovery
No edits to live session-state, forensic files, PR #616, review 443, or preserve branch

Do not self-review or merge.

## Canonical author handoff — Issue #720 / PR #721 ```text STATE: Author implementation complete; awaiting independent review WHO_IS_NEXT: reviewer NEXT_ACTION: Independent review of PR #721 at pinned head (do not self-review/merge) NEXT_PROMPT: Review PR #721 (Closes #720) at expected_head_sha=80f59b334e6671b08006725292c08a8e8b6c823f from a reviewer namespace/profile distinct from jcwalker3/prgs-author. WHAT_HAPPENED: Implemented recovery-critical lifecycle for KIND_DECISION_LOCK so expired old-head evidence no longer blocks fresh formal review on a newer open-PR head; PR #721 opened. WHY: Generic 4h session-state TTL rejected decision ledgers before #620 fresh_review_on_current_head_allowed could apply (PR #616 / review 443 reproduction). RELATED_PRS: 721 (implementation), 616 (concrete reproduction; not modified) IMMUTABLE_HEAD: 80f59b334e6671b08006725292c08a8e8b6c823f BRANCH: fix/issue-720-expired-decision-lock WORKTREE: branches/issue-720-expired-decision-lock BASE: master @ 77746b08fc19e20abb95e4171784e321cb88817a VALIDATION: focused 228 passed; full suite 2898 passed, 6 skipped, 1 warning, 173 subtests passed BLOCKERS: None LAST_UPDATED_BY: prgs-author / jcwalker3 ``` ### Delivered - `KIND_DECISION_LOCK` ∈ `RECOVERY_CRITICAL_KINDS` (terminal provenance survives generic TTL) - `inspect_state_envelope` + cleanup `disk_inspect` for accurate on-disk assessment - Regression suite `tests/test_issue_720_expired_decision_lock.py` - Same-head #332, #620 head-scope, #594 moot cleanup, #709 irrecoverable auth preserved - No ordinary-profile grant of `gitea.decision_lock.irrecoverable_recovery` - No edits to live session-state, forensic files, PR #616, review 443, or preserve branch **Do not self-review or merge.**

sysadmin referenced this issue

2026-07-16 16:56:46 -05:00

fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720) #721

sysadmin referenced this issue

2026-07-16 17:01:03 -05:00

fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720) #721

sysadmin referenced this issue

2026-07-16 17:02:01 -05:00

fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720) #721

jcwalker3 referenced this issue

2026-07-16 17:33:45 -05:00

incident: unreviewed direct-to-master commit 970e68b remapped 10 reviewer tasks to merger, disabling the repository's formal review capability #722