From 86651a3d054cdde9b9490d9fe662faf156ffe9ba Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 9 Jul 2026 15:21:21 -0400 Subject: [PATCH] docs: expand #594 stale decision-lock cleanup guidance Add allowed/forbidden workflow steps to the review-merge skill and document the #594 cleanup gate on the Safety-and-Gates wiki page. --- docs/wiki/Safety-and-Gates.md | 6 +- .../workflows/review-merge-pr.md | 69 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/docs/wiki/Safety-and-Gates.md b/docs/wiki/Safety-and-Gates.md index 25c21fb..d8182a9 100644 --- a/docs/wiki/Safety-and-Gates.md +++ b/docs/wiki/Safety-and-Gates.md @@ -15,5 +15,7 @@ 5. **Explicit merge confirmation** — `gitea_merge_pr` requires `confirmation="MERGE PR "`. 6. **No self-review / self-merge** — Authenticated user must differ from PR author for approve/merge. 7. **Review decision lock** — Live review mutations require validation-phase dry-run and `gitea_mark_final_review_decision`. -8. **Redaction** — Tokens, passwords, and keychain material never appear in tool output. -9. **Wiki publication (#224)** — `docs/wiki/` and the sync helper are prerequisites only. Closing a wiki issue requires live Gitea Wiki proof on the repo Wiki tab. See [Runbooks](Runbooks.md#wiki-publication-readiness-gate-224). \ No newline at end of file +8. **Terminal review hard-stop (#332)** — After a terminal live review mutation, only same-PR merge after `approve` may continue. Durable locks (#559) must not be deleted by hand. +9. **Stale decision-lock cleanup (#594)** — `gitea_cleanup_stale_review_decision_lock` may clear a durable #332 lock **only** when the last terminal mutation's PR is live-state **merged or closed**, identity/profile gates pass, and apply uses a reviewer-capable profile. Open/ambiguous locks stay fail-closed. Successful cleanup records a durable audit trail. +10. **Redaction** — Tokens, passwords, and keychain material never appear in tool output. +11. **Wiki publication (#224)** — `docs/wiki/` and the sync helper are prerequisites only. Closing a wiki issue requires live Gitea Wiki proof on the repo Wiki tab. See [Runbooks](Runbooks.md#wiki-publication-readiness-gate-224). \ No newline at end of file diff --git a/skills/llm-project-workflow/workflows/review-merge-pr.md b/skills/llm-project-workflow/workflows/review-merge-pr.md index d27c961..e0b3996 100644 --- a/skills/llm-project-workflow/workflows/review-merge-pr.md +++ b/skills/llm-project-workflow/workflows/review-merge-pr.md @@ -826,6 +826,75 @@ The final report must identify: * whether same-PR merge continuation was allowed * whether the run stopped as required +## 26A-1. Stale durable review-decision lock cleanup (#594) + +After #559, the review-decision lock is durable under +`~/.cache/gitea-tools/session-state/` (for example +`review_decision_lock-prgs-reviewer.json`). That durability can outlive the work +it protects: a terminal `approve` / `request_changes` on a PR that is already +**merged or closed** still hard-stops later unrelated reviews under #332. + +**Do not** delete session-state files by hand. Use the canonical MCP path. + +### When cleanup is allowed + +Use `gitea_cleanup_stale_review_decision_lock` when: + +1. `gitea_mark_final_review_decision` / live review is blocked by + `terminal review mutation already consumed ... (#332)`. +2. Live Gitea state for the **last terminal mutation's PR** is **merged** or + **closed** (no same-PR merge sequence remains). +3. Active profile identity matches the durable lock (reviewer profile with + `gitea.pr.review` for `apply=true`). +4. Optional pin: pass `expected_terminal_pr` to the prior terminal PR number + (for example `586` when resuming after a merged #586 lock). + +Recommended sequence: + +```text +gitea_whoami +gitea_resolve_task_capability(task="cleanup_stale_review_decision_lock") +gitea_cleanup_stale_review_decision_lock(apply=false, remote=..., org=..., repo=...) +# inspect is_moot / cleanup_allowed / last_terminal_pr +gitea_cleanup_stale_review_decision_lock( + apply=true, + expected_terminal_pr=, + remote=..., org=..., repo=..., +) +gitea_resolve_task_capability(task="review_pr") +gitea_load_review_workflow() +# continue formal mark + submit for the *new* PR +``` + +Successful `apply=true` clears memory + durable store and returns an audit +record (and posts a durable PR comment on the terminal PR when +`post_audit_comment` is true and comment permission allows). + +Same-profile auto-expire: after `gitea_merge_pr` succeeds for the PR that this +profile just approved, the decision lock for that profile is cleared +automatically. Cross-profile locks (for example reviewer approve, merger merge) +still require `gitea_cleanup_stale_review_decision_lock`. + +### When cleanup is forbidden + +Refuse / fail-closed — keep #332 hard-stop — when: + +* the last terminal PR is still **open** (active review or merge sequence may + still apply) +* live PR state cannot be fetched (ambiguous) +* no lock or no terminal live mutation exists +* profile identity does not match the lock +* apply requested without authenticated identity or `gitea.pr.review` +* `expected_terminal_pr` does not match the last terminal PR + +Do **not** use cleanup to: + +* bypass #332 on an open PR +* replace `gitea_authorize_review_correction` for a mistaken review on a still + active PR (#211) +* auto-approve or auto-merge any PR +* justify `rm` of session-state files + ## 26B. Per-PR reviewer lease (#407) Parallel reviewer sessions are allowed only when each session holds a distinct,