Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 75 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

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

New Issue
Closed
opened 2026-07-16 14:32:38 -05:00 by jcwalker3 · 2 comments
jcwalker3 commented 2026-07-16 14:32:38 -05:00
Owner
Copy Link
Copy Source

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
## 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 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
Author
Owner
Copy Link
Copy Source

Issue claim heartbeat

<!-- gitea-issue-claim-heartbeat:v1 --> **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 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
Author
Owner
Copy Link
Copy Source

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.**
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
allocator
Controller-owned work allocator
 anti-stomp
Prevent concurrent LLM session stomping
 architecture
Architecture / structural design
 bug
chore
codex
OpenAI Codex client / workflow session surface
 concurrency
Concurrent session safety
 contamination
Workflow or session contamination incident
 control-plane
MCP control-plane coordination and allocation authority
 dashboard
MCP operational dashboard/queue view
 database
Internal coordination storage (SQLite/Postgres)
 design
Design / investigation, no implementation
 documentation
Docs / runbooks
 enhancement
New feature or improvement
 gitea
Gitea MCP workflow
 glitchtip
GlitchTip integration
 important
incident
Operational or process incident requiring durable audit trail
 incident-bridge
Sentry-to-Gitea incident bridging
 integration
Integration testing
 jenkins
Jenkins integration
 labels
Label taxonomy management
 leases
Lease adopt/release/expire lifecycle
 mcp
MCP server / tooling
 mcp-health
MCP namespace and runtime health
 mcp-menu
MCP menu surface
 multi-project
Work spanning multiple monitoring projects or Gitea repos
 mutating
Mutating action; requires gating
 nice-to-have
observability
Observability, metrics, traces, error reporting
 portability
Cross-platform / portability
 preflight
Shared preflight gates before mutation
 protected-branch
Protected branch / stable-branch policy concern
 queue
Work queue visibility and allocation
 read-only
Read-only, no mutation
 reconnect
MCP client reconnect/reload recovery path
 recovery
Recovery paths for stale/foreign leases
 refactor
Code refactor / restructure
 release
Release / versioning
 reliability
Reliability / failure handling
 resumable-review
Persist and resume prepared review verdicts across sessions
 reviewer
Reviewer workflow tooling
 roadmap
Roadmap / umbrella issue
 safety
Safety rails and fail-closed mutation guards
 security
Security / trust boundary
 self-hosted
Self-hosted infrastructure integration
 sentry
Sentry error monitoring integration
 stale-runtime
Stale backend daemon / runtime-vs-master parity failures
 status:blocked
Issue is blocked
 status:in-progress
Issue is being worked on
 status:pr-open
Issue has an open pull request
 status:ready
Issue is ready for work
 terminal-lock
Terminal review lock (#332) path
 testing
Tests / test coverage
 tracker
Issue tracker hygiene / meta
 type:bug
Bug or defect
 type:feature
Feature or enhancement
 type:feature
Feature or enhancement
 type:guardrail
Safety gate or guardrail
 visibility
Workflow state visibility for LLMs/operators
 workflow
Cross-tool workflow
 workflow-hardening
LLM workflow coordination hardening
 workflow-hardening
LLM workflow coordination hardening
No labels mcp recovery reliability reviewer safety status:pr-open terminal-lock type:bug workflow-hardening
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jcwalker3 sysadmin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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