Merger cleanup is profile-local and reviewer initialization can destroy terminal decision-lock evidence #709

New Issue

2026-07-13T23:48:59-05:00

jcwalker3 commented

2026-07-13 23:48:59 -05:00

Problem

The merge workflow can initialize and clean the merger profile’s empty decision lock while leaving the reviewer profile’s terminal lock untouched. A later reviewer session can then overwrite that unresolved reviewer evidence. If cleanup or audit publication is skipped, no durable fail-closed recovery record is guaranteed. This permanently destroys provenance and can leave an otherwise-approved PR blocked because truthful historical applied-cleanup evidence cannot be reconstructed.

Forensic evidence (do not fabricate history)

Incident #700 comment 11489 — complete forensic diagnosis
PR #703 comment 11495 — merge-blocked cross-reference
PR #696 — merged; reviewer decision-lock cleanup was skipped
PR #703 — later reviewer initialization overwrote leftover #696 state; remains open APPROVED (review 433) but merge blocked
Issue #594 (closed) — narrower; post-merge moot cleanup for active-profile locks only
Issue #693 (open) — partial overlap on recovery/audit gaps for open-PR stuck locks; does not fully own this bundle (cross-profile merger cleanup, overwrite protection, irrecoverable-evidence recovery)

No historical claim of applied=true + last_terminal_pr=696 + pr_merged=true is valid.

Defect classes

Merger cleanup is profile-local — targets only the merger profile’s decision lock (often empty init), not the reviewer lock holding terminal mutations for the approval being consumed.
Reviewer initialization overwrites unresolved terminal evidence — empty re-init can erase a prior terminal ledger without fail-closed archive/audit.
Missing cleanup audit is not fail-closed — cleanup/audit skip after irreversible merge does not guarantee a durable recovery-required record.
No truthful recovery when historical terminal evidence is already irrecoverably lost — no mechanism that documents absence of proof without fabricating applied cleanup.

Acceptance criteria

AC1 — Exact decision cleanup

Merge cleanup targets the reviewer decision/lock associated with the approval being consumed (verify repository, PR, head SHA, reviewer profile, review/session identity, terminal mutation before cleanup).

AC2 — No silent overwrite

Starting/initializing a reviewer session never overwrites unresolved terminal decision-lock evidence; fail closed or preserve/archive via auditable transition. Empty initialized lock must not erase a prior terminal ledger.

AC3 — Durable post-merge reconciliation

If PR merges but cleanup or audit publication fails, record durable recovery-required state (not undo merge). Survives restart; identifies merged PR, head, merge commit, target lock/profile, failed step, required recovery action.

AC4 — Audit publication

Successful cleanup is fully reconciled only after audit is published and read back (applied status, terminal PR, merged state, cleanup target profile/lock, operation/session IDs, timestamps, comment ID). Comment failure → retryable recovery-required without repeating unsafe mutations.

AC5 — Truthful irrecoverable-evidence recovery

Native, generic, tightly gated recovery for already-destroyed historical terminal evidence. Never emit applied=true or claim historical cleanup was proven. Distinct record (provenance_irrecoverable / operator_recovery_required). Scoped to repository, PR, exact head, incident/canonical evidence, operator authorization, safe-state verification. Auditable, idempotent, read back; not a broad approval/merge bypass. Define precise merger acceptance conditions. Do not hard-code PR #696, #703, or Incident #700.

AC6 — Anti-stomp and replay safety

Cleanup, audit retry, and irrecoverable recovery are idempotent. Stale sessions / wrong heads / repos / profiles / reviews fail closed. Authorization for one PR/head cannot unblock another.

AC7 — Compatibility

Existing valid lock/audit records remain readable; legacy empty locks handled conservatively; schema/version changes have explicit validation/migration.

AC8 — Regression coverage

Tests for all scenarios in the implementation plan (merger-local empty while reviewer terminal remains; exact reviewer cleanup after merge; init overwrite attempt; audit-comment failure; restart/retry; duplicates; wrong identity; irrecoverable recovery without fabrication; cross-PR reuse; legacy compatibility; no gate regressions).

Non-goals

Fabricating PR #696 historical cleanup provenance
Special-casing PR #696 / #703 in production code
Weakening approval, lease, decision-lock, audit, identity, native-transport, workspace, or anti-stomp enforcement
Merging PR #703 from this workstream

Links

Incident #700 comment 11489
PR #703 comment 11495
Issues #594, #693
PRs #696, #703

## Problem The merge workflow can initialize and clean the **merger profile’s empty decision lock** while leaving the **reviewer profile’s terminal lock** untouched. A later reviewer session can then **overwrite that unresolved reviewer evidence**. If cleanup or audit publication is skipped, no durable fail-closed recovery record is guaranteed. This permanently destroys provenance and can leave an otherwise-approved PR blocked because truthful historical applied-cleanup evidence cannot be reconstructed. ## Forensic evidence (do not fabricate history) - **Incident #700 comment 11489** — complete forensic diagnosis - **PR #703 comment 11495** — merge-blocked cross-reference - **PR #696** — merged; reviewer decision-lock cleanup was skipped - **PR #703** — later reviewer initialization overwrote leftover #696 state; remains open APPROVED (review 433) but **merge blocked** - **Issue #594** (closed) — narrower; post-merge moot cleanup for active-profile locks only - **Issue #693** (open) — partial overlap on recovery/audit gaps for **open-PR** stuck locks; does **not** fully own this bundle (cross-profile merger cleanup, overwrite protection, irrecoverable-evidence recovery) **No historical claim of `applied=true` + `last_terminal_pr=696` + `pr_merged=true` is valid.** ## Defect classes 1. **Merger cleanup is profile-local** — targets only the merger profile’s decision lock (often empty init), not the reviewer lock holding terminal mutations for the approval being consumed. 2. **Reviewer initialization overwrites unresolved terminal evidence** — empty re-init can erase a prior terminal ledger without fail-closed archive/audit. 3. **Missing cleanup audit is not fail-closed** — cleanup/audit skip after irreversible merge does not guarantee a durable recovery-required record. 4. **No truthful recovery** when historical terminal evidence is already irrecoverably lost — no mechanism that documents absence of proof without fabricating applied cleanup. ## Acceptance criteria ### AC1 — Exact decision cleanup Merge cleanup targets the reviewer decision/lock associated with the approval being consumed (verify repository, PR, head SHA, reviewer profile, review/session identity, terminal mutation before cleanup). ### AC2 — No silent overwrite Starting/initializing a reviewer session never overwrites unresolved terminal decision-lock evidence; fail closed or preserve/archive via auditable transition. Empty initialized lock must not erase a prior terminal ledger. ### AC3 — Durable post-merge reconciliation If PR merges but cleanup or audit publication fails, record durable recovery-required state (not undo merge). Survives restart; identifies merged PR, head, merge commit, target lock/profile, failed step, required recovery action. ### AC4 — Audit publication Successful cleanup is fully reconciled only after audit is published and read back (applied status, terminal PR, merged state, cleanup target profile/lock, operation/session IDs, timestamps, comment ID). Comment failure → retryable recovery-required without repeating unsafe mutations. ### AC5 — Truthful irrecoverable-evidence recovery Native, generic, tightly gated recovery for already-destroyed historical terminal evidence. Never emit `applied=true` or claim historical cleanup was proven. Distinct record (`provenance_irrecoverable` / `operator_recovery_required`). Scoped to repository, PR, exact head, incident/canonical evidence, operator authorization, safe-state verification. Auditable, idempotent, read back; not a broad approval/merge bypass. Define precise merger acceptance conditions. **Do not hard-code PR #696, #703, or Incident #700.** ### AC6 — Anti-stomp and replay safety Cleanup, audit retry, and irrecoverable recovery are idempotent. Stale sessions / wrong heads / repos / profiles / reviews fail closed. Authorization for one PR/head cannot unblock another. ### AC7 — Compatibility Existing valid lock/audit records remain readable; legacy empty locks handled conservatively; schema/version changes have explicit validation/migration. ### AC8 — Regression coverage Tests for all scenarios in the implementation plan (merger-local empty while reviewer terminal remains; exact reviewer cleanup after merge; init overwrite attempt; audit-comment failure; restart/retry; duplicates; wrong identity; irrecoverable recovery without fabrication; cross-PR reuse; legacy compatibility; no gate regressions). ## Non-goals - Fabricating PR #696 historical cleanup provenance - Special-casing PR #696 / #703 in production code - Weakening approval, lease, decision-lock, audit, identity, native-transport, workspace, or anti-stomp enforcement - Merging PR #703 from this workstream ## Links - Incident #700 comment **11489** - PR #703 comment **11495** - Issues **#594**, **#693** - PRs **#696**, **#703**

jcwalker3 added the status:ready workflow-hardening anti-stomp safety type:bug labels 2026-07-13 23:48:59 -05:00

jcwalker3 referenced this issue from a commit

2026-07-13 23:54:41 -05:00

fix(workflow): cross-profile decision-lock cleanup and irrecoverable provenance (#709)

jcwalker3 referenced a pull request that will close this issue

2026-07-13 23:54:49 -05:00

fix(workflow): cross-profile decision-lock cleanup and irrecoverable provenance (Closes #709) #710

jcwalker3 added status:pr-open and removed status:ready labels 2026-07-13 23:54:49 -05:00

jcwalker3 commented

2026-07-13 23:54:58 -05:00

Canonical Issue State

STATE:
ready-for-review

WHO_IS_NEXT:
reviewer

NEXT_ACTION:
Independent prgs-reviewer formal review of PR #710 at head ec5cf67771 against #709 AC1–AC8; do not merge PR #703; do not self-review as author.

NEXT_PROMPT:

Role: REVIEWER (prgs-reviewer)
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
Issue: #709 | PR: #710
Pinned head: ec5cf677718b6a6a5fc9a5102b5ce1783592509a

1. gitea_whoami + resolve review_pr on prgs-reviewer; neutral runtime; acquire PR #710 lease first.
2. Verify AC1–AC8: cross-profile post-merge cleanup, init overwrite protection, recovery-required records, audit readback, irrecoverable provenance (applied=false only), anti-stomp/idempotency, tests.
3. Confirm no fabrication of PR #696 history and no special-casing of #696/#703/#700 in production code.
4. Submit exactly one formal verdict via native MCP. Do not merge. Do not merge PR #703.

WHAT_HAPPENED:
Author created #709 (dedicated; #693 partial only), implemented AC1–AC8 in branches/fix-issue-709-decision-lock-cross-profile, full suite 2710 passed / 6 skipped, pushed ec5cf67, opened PR #710 Closes #709. Linked Incident #700 comment 11489 and PR #703 comment 11495.

WHY:
Merger profile-local empty locks cannot stand in for reviewer terminal cleanup; silent re-init overwrite and missing audit must fail closed with truthful recovery paths.

RELATED_PRS:
PR #710 (this fix); PR #703 remains open merge-blocked; PR #696 forensic context only.

BLOCKERS:
none for review start; PR #703 merge remains blocked until this repair lands and any sanctioned recovery is executed.

VALIDATION:
pytest focused 32 passed; full suite 2710 passed, 6 skipped, 1 warning, 161 subtests passed; head ec5cf677718b6a6a5fc9a5102b5ce1783592509a; root control checkout not mutated for this work.

LAST_UPDATED_BY:
jcwalker3 / prgs-author

## Canonical Issue State STATE: ready-for-review WHO_IS_NEXT: reviewer NEXT_ACTION: Independent prgs-reviewer formal review of PR #710 at head ec5cf677718b6a6a5fc9a5102b5ce1783592509a against #709 AC1–AC8; do not merge PR #703; do not self-review as author. NEXT_PROMPT: ```text Role: REVIEWER (prgs-reviewer) Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools Issue: #709 | PR: #710 Pinned head: ec5cf677718b6a6a5fc9a5102b5ce1783592509a 1. gitea_whoami + resolve review_pr on prgs-reviewer; neutral runtime; acquire PR #710 lease first. 2. Verify AC1–AC8: cross-profile post-merge cleanup, init overwrite protection, recovery-required records, audit readback, irrecoverable provenance (applied=false only), anti-stomp/idempotency, tests. 3. Confirm no fabrication of PR #696 history and no special-casing of #696/#703/#700 in production code. 4. Submit exactly one formal verdict via native MCP. Do not merge. Do not merge PR #703. ``` WHAT_HAPPENED: Author created #709 (dedicated; #693 partial only), implemented AC1–AC8 in branches/fix-issue-709-decision-lock-cross-profile, full suite 2710 passed / 6 skipped, pushed ec5cf67, opened PR #710 Closes #709. Linked Incident #700 comment 11489 and PR #703 comment 11495. WHY: Merger profile-local empty locks cannot stand in for reviewer terminal cleanup; silent re-init overwrite and missing audit must fail closed with truthful recovery paths. RELATED_PRS: PR #710 (this fix); PR #703 remains open merge-blocked; PR #696 forensic context only. BLOCKERS: none for review start; PR #703 merge remains blocked until this repair lands and any sanctioned recovery is executed. VALIDATION: pytest focused 32 passed; full suite 2710 passed, 6 skipped, 1 warning, 161 subtests passed; head ec5cf677718b6a6a5fc9a5102b5ce1783592509a; root control checkout not mutated for this work. LAST_UPDATED_BY: jcwalker3 / prgs-author