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

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

Open
jcwalker3 wants to merge 3 commits from fix/issue-709-decision-lock-cross-profile into master
pull from: fix/issue-709-decision-lock-cross-profile
merge into: Scaled-Tech-Consulting:master
Conversation 8 Commits 3 Files Changed 7
+4496 -27
jcwalker3 commented 2026-07-13 23:54:49 -05:00
Owner
Copy Link
Copy Source

Summary

Closes #709. Implements the workflow repair identified by Incident #700 comment 11489 / PR #703 comment 11495.

What this PR repairs (future behavior)

  1. AC1 — Exact decision cleanup: After merge, reconcile scans durable decision locks across profiles (not only the merger profile’s empty local lock) and clears locks whose terminal mutation targets the merged PR’s approval. Archives prior state before clear.
  2. AC2 — No silent overwrite: init_review_decision_lock refuses to replace unresolved terminal evidence with an empty init even when force=True.
  3. AC3 — Post-merge recovery: If cross-profile cleanup or audit fails after irreversible merge, records durable post_merge_decision_recovery (applied=false).
  4. AC4 — Audit publication: Successful cleanup is fully reconciled only when audit comment id is read back; comment failure → recovery-required, no false success.
  5. AC5 — Irrecoverable provenance: New gitea_record_irrecoverable_decision_lock_provenance records absence of proof only (applied=false, historical_cleanup_proven=false). Never fabricates historical cleanup. Confirmation-gated + operator_authorized.
  6. AC6–AC8: Idempotency, scoping, TTL exemption for recovery-critical kinds, regression tests.

Explicit non-claims

  • This PR does not fabricate PR #696 history and does not claim applied=true / last_terminal_pr=696 / pr_merged=true for past cleanup.
  • PR #703 remains blocked until: independent review + merge of this repair, and a sanctioned recovery execution (if still required) under the new path.
  • No special-casing of PR #696, #703, or Incident #700 in production code.

Files

File Role
stale_review_decision_lock.py Pure policy: init overwrite assess, post-merge recovery + irrecoverable records, formatters
mcp_session_state.py Cross-profile list/load; recovery-critical kinds + TTL exempt
gitea_mcp_server.py Init protection; post-merge cross-profile reconcile; cleanup AC4; irrecoverable tool
task_capability_map.py Capability entry for irrecoverable tool
tests/test_issue_709_decision_lock_cross_profile.py AC regression suite

Validation

pytest tests/test_issue_709_decision_lock_cross_profile.py tests/test_stale_review_decision_lock_cleanup.py -q
# 32 passed
pytest tests/ -q
# 2710 passed, 6 skipped, 1 warning, 161 subtests passed

Links

  • Incident #700 comment 11489
  • PR #703 comment 11495
  • Issues #594, #693, #709
  • PRs #696 (merged forensic context), #703 (remains unmerged / merge-blocked)

Next role

Independent REVIEWER — do not self-review or merge. Do not merge PR #703 from this workstream.

## Summary Closes **#709**. Implements the workflow repair identified by **Incident #700 comment 11489** / **PR #703 comment 11495**. ### What this PR repairs (future behavior) 1. **AC1 — Exact decision cleanup:** After merge, reconcile scans durable decision locks across profiles (not only the merger profile’s empty local lock) and clears locks whose terminal mutation targets the merged PR’s approval. Archives prior state before clear. 2. **AC2 — No silent overwrite:** `init_review_decision_lock` refuses to replace unresolved terminal evidence with an empty init even when `force=True`. 3. **AC3 — Post-merge recovery:** If cross-profile cleanup or audit fails after irreversible merge, records durable `post_merge_decision_recovery` (applied=false). 4. **AC4 — Audit publication:** Successful cleanup is fully reconciled only when audit comment id is read back; comment failure → recovery-required, no false success. 5. **AC5 — Irrecoverable provenance:** New `gitea_record_irrecoverable_decision_lock_provenance` records **absence of proof** only (`applied=false`, `historical_cleanup_proven=false`). Never fabricates historical cleanup. Confirmation-gated + operator_authorized. 6. **AC6–AC8:** Idempotency, scoping, TTL exemption for recovery-critical kinds, regression tests. ### Explicit non-claims - This PR **does not fabricate PR #696 history** and does **not** claim `applied=true` / `last_terminal_pr=696` / `pr_merged=true` for past cleanup. - **PR #703 remains blocked** until: independent review + merge of this repair, and a sanctioned recovery execution (if still required) under the new path. - No special-casing of PR #696, #703, or Incident #700 in production code. ### Files | File | Role | |------|------| | `stale_review_decision_lock.py` | Pure policy: init overwrite assess, post-merge recovery + irrecoverable records, formatters | | `mcp_session_state.py` | Cross-profile list/load; recovery-critical kinds + TTL exempt | | `gitea_mcp_server.py` | Init protection; post-merge cross-profile reconcile; cleanup AC4; irrecoverable tool | | `task_capability_map.py` | Capability entry for irrecoverable tool | | `tests/test_issue_709_decision_lock_cross_profile.py` | AC regression suite | ### Validation ```text pytest tests/test_issue_709_decision_lock_cross_profile.py tests/test_stale_review_decision_lock_cleanup.py -q # 32 passed pytest tests/ -q # 2710 passed, 6 skipped, 1 warning, 161 subtests passed ``` ### Links - Incident #700 comment **11489** - PR #703 comment **11495** - Issues **#594**, **#693**, **#709** - PRs **#696** (merged forensic context), **#703** (remains unmerged / merge-blocked) ### Next role **Independent REVIEWER** — do not self-review or merge. Do not merge PR #703 from this workstream.
jcwalker3 added 1 commit 2026-07-13 23:54:49 -05:00
fix(workflow): cross-profile decision-lock cleanup and irrecoverable provenance (#709) ec5cf67771 
Prevent merger-local empty decision locks from standing in for reviewer
terminal cleanup, refuse silent re-init overwrite of unresolved terminal
evidence, record post-merge recovery-required state when audit fails, and
add a truthful irrecoverable-provenance path that never claims applied=true.

Closes #709
sysadmin commented 2026-07-14 00:06:26 -05:00
Owner
Copy Link
Copy Source

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #710
issue: #709
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 40918-d0d14c123645
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710
phase: claimed
candidate_head: ec5cf67771
target_branch: master
target_branch_sha: 1eafb757a9
last_activity: 2026-07-14T05:06:25Z
expires_at: 2026-07-14T07:06:25Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #710 issue: #709 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 40918-d0d14c123645 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710 phase: claimed candidate_head: ec5cf677718b6a6a5fc9a5102b5ce1783592509a target_branch: master target_branch_sha: 1eafb757a91e4606ed9f82413e4122ad6bcf94fd last_activity: 2026-07-14T05:06:25Z expires_at: 2026-07-14T07:06:25Z blocker: none
sysadmin requested changes 2026-07-14 00:10:34 -05:00
Dismissed
sysadmin left a comment
Owner
Copy Link
Copy Source

Canonical PR State

STATE: changes_requested
WHO_IS_NEXT: author
NEXT_ACTION: Bind irrecoverable-provenance authorization to a non-forgeable artifact, implement and fail-close the merger acceptance path, and scope cross-profile cleanup to repository plus head; add regression tests and request a fresh independent review.
NEXT_PROMPT:

AUTHOR task PR #710 / Issue #709: address this REQUEST_CHANGES. (1) gitea_record_irrecoverable_decision_lock_provenance must stop trusting caller-supplied operator_authorized plus a pr-number-derivable confirmation string; bind authorization to native transport, reconciler capability, live head SHA, canonical incident evidence and a non-forgeable artifact; require expected_head_sha to equal the live PR head. (2) Implement a fail-closed merger consumption path for the recovery record; merger_may_accept is written but never read. (3) In _clear_decision_lock_for_profile / load_state_for_profile verify remote/org/repo (the computed filtered reasons are unused) and require head match in the fallback clear branch. Add regression tests each; re-run tests/; do not merge PR #703.

WHAT_HAPPENED: Independent reviewer confirmed scope (5 files, single commit, generic), ran focused (32 passed) and full (2710 passed, 6 skipped, 1 warning, 161 subtests) suites, and found three blocking defects in the #709 recovery/cleanup design.
WHY: AC5 requires recovery authorization that cannot be self-asserted and a precise, implemented merger acceptance condition; AC1/AC6 require cross-profile cleanup scoped to repository plus head. The implementation does not meet these.
ISSUE: #709
HEAD_SHA: ec5cf67771
REVIEW_STATUS: request_changes posted to gitea
MERGE_READY: no
BLOCKERS: F1 self-assertable operator_authorized; F2 no implemented merger acceptance path; F3 cross-repo isolation gap in post-merge cleanup.
VALIDATION: focused 32 passed; full 2710 passed, 6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py, not touched by this PR), 161 subtests passed.
LAST_UPDATED_BY: reviewer sysadmin / prgs-reviewer

Findings (blocking)

F1 — Recovery authorization is self-assertable (AC5, adversarial check C1).

  • File: stale_review_decision_lock.py::build_irrecoverable_provenance_record and gitea_mcp_server.py::gitea_record_irrecoverable_decision_lock_provenance.
  • merger_may_accept = bool(operator_authorized) where operator_authorized is a plain caller-supplied boolean. The only other gates are a confirmation string that is fully derivable from the PR number (IRRECOVERABLE DECISION PROVENANCE PR <n>) and a gitea.read permission check. expected_head_sha and incident_ref are optional and never validated against the live PR head or canonical incident evidence.
  • Reproduced from a fresh non-pytest process: with operator_authorized=True, head_sha=None, incident_ref=None the record returns merger_may_accept=True. An arbitrary reconciler-capable (or any read+comment) caller can self-assert authorization. This is exactly the PR #696 / review-427 threat model.
  • Required correction: bind authorization to native transport, authenticated profile/capability, repository, PR, exact live head SHA, canonical incident evidence and a non-forgeable authorization artifact; require expected_head_sha and reject when it does not equal the live head.

F2 — Merger acceptance condition is described but not implemented (AC5, adversarial check C6).

  • No merger/merge/adopt code consumes merger_may_accept, the irrecoverable_decision_provenance record, or KIND_POST_MERGE_DECISION_RECOVERY. They are write-only forensic artifacts; acceptance_rule is only a descriptive string.
  • AC5 requires precise merger acceptance conditions that are actually implemented and fail-closed. As shipped, the recovery path cannot sanction unblocking PR #703 through any implemented consumer, so the acceptance criterion is incomplete.
  • Required correction: implement a fail-closed merger consumption path that re-verifies repository, PR, exact head, durability/readback and absence of a conflicting terminal lock before honoring the record, or descope AC5 to the record-only guarantee and document the missing consumer.

F3 — Cross-profile cleanup does not verify repository or head in its fallback (AC1, AC6, adversarial check C2).

  • Decision-lock files are keyed only by <kind>-<profile> (remote/org/repo intentionally excluded from the filename, validated on load).
  • mcp_session_state.load_state_for_profile(skip_identity_match=True) computes filtered (the remote/org/repo mismatch reasons) but never uses it; only expiry gates the return, so repository/remote/org are not enforced on cross-profile load.
  • gitea_mcp_server._clear_decision_lock_for_profile clears via a fallback branch when last_terminal_mutation.pr_number == pr_number, ignoring head SHA and repository. A same-profile decision lock recorded for a different repository's PR with the same number can be archived and cleared during a merge, destroying unrelated terminal evidence — the exact harm #709 targets.
  • Required correction: enforce remote/org/repo on load (use the filtered reasons) and require head-SHA match in the fallback clear branch.

Non-blocking observations

  • AC2 overwrite protection is correctly enforced in production init_review_decision_lock via assess_init_overwrite; force=True cannot destroy terminal evidence.
  • AC3/AC4 post-merge recovery and audit-readback fail-closed logic is sound; merge is never undone.
  • No special-casing of PR #696/#703/#700 in production code; #693 and #594 are referenced, not closed or repurposed.

[THREAD STATE LEDGER]

What is true now:

  • server-side decision state: request_changes posted to gitea at head ec5cf67771
  • local verdict/state: REQUEST_CHANGES prepared locally in isolated detached worktree at the pinned head

What changed:

  • one reviewer REQUEST_CHANGES review posted to PR #710; server-side state changed (a review was created)

What is blocked:

  • blocker classification: code blocker
  • PR #710 cannot merge until the author corrects F1, F2 and F3
  • exact failing gates: AC5 authorization binding, AC5 merger acceptance implementation, AC1/AC6 repository and head scoping

Who/what acts next:

  • next actor: author
  • required action: fix F1, F2 and F3 with regression tests, re-run the suite and request a fresh independent review

Do not do: do not merge PR #710 or PR #703; do not self-review; do not treat the irrecoverable record as proof of historical cleanup.

## Canonical PR State STATE: changes_requested WHO_IS_NEXT: author NEXT_ACTION: Bind irrecoverable-provenance authorization to a non-forgeable artifact, implement and fail-close the merger acceptance path, and scope cross-profile cleanup to repository plus head; add regression tests and request a fresh independent review. NEXT_PROMPT: ```text AUTHOR task PR #710 / Issue #709: address this REQUEST_CHANGES. (1) gitea_record_irrecoverable_decision_lock_provenance must stop trusting caller-supplied operator_authorized plus a pr-number-derivable confirmation string; bind authorization to native transport, reconciler capability, live head SHA, canonical incident evidence and a non-forgeable artifact; require expected_head_sha to equal the live PR head. (2) Implement a fail-closed merger consumption path for the recovery record; merger_may_accept is written but never read. (3) In _clear_decision_lock_for_profile / load_state_for_profile verify remote/org/repo (the computed filtered reasons are unused) and require head match in the fallback clear branch. Add regression tests each; re-run tests/; do not merge PR #703. ``` WHAT_HAPPENED: Independent reviewer confirmed scope (5 files, single commit, generic), ran focused (32 passed) and full (2710 passed, 6 skipped, 1 warning, 161 subtests) suites, and found three blocking defects in the #709 recovery/cleanup design. WHY: AC5 requires recovery authorization that cannot be self-asserted and a precise, implemented merger acceptance condition; AC1/AC6 require cross-profile cleanup scoped to repository plus head. The implementation does not meet these. ISSUE: #709 HEAD_SHA: ec5cf677718b6a6a5fc9a5102b5ce1783592509a REVIEW_STATUS: request_changes posted to gitea MERGE_READY: no BLOCKERS: F1 self-assertable operator_authorized; F2 no implemented merger acceptance path; F3 cross-repo isolation gap in post-merge cleanup. VALIDATION: focused 32 passed; full 2710 passed, 6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py, not touched by this PR), 161 subtests passed. LAST_UPDATED_BY: reviewer sysadmin / prgs-reviewer ### Findings (blocking) **F1 — Recovery authorization is self-assertable (AC5, adversarial check C1).** - File: `stale_review_decision_lock.py::build_irrecoverable_provenance_record` and `gitea_mcp_server.py::gitea_record_irrecoverable_decision_lock_provenance`. - `merger_may_accept = bool(operator_authorized)` where `operator_authorized` is a plain caller-supplied boolean. The only other gates are a `confirmation` string that is fully derivable from the PR number (`IRRECOVERABLE DECISION PROVENANCE PR <n>`) and a `gitea.read` permission check. `expected_head_sha` and `incident_ref` are optional and never validated against the live PR head or canonical incident evidence. - Reproduced from a fresh non-pytest process: with `operator_authorized=True`, `head_sha=None`, `incident_ref=None` the record returns `merger_may_accept=True`. An arbitrary reconciler-capable (or any read+comment) caller can self-assert authorization. This is exactly the PR #696 / review-427 threat model. - Required correction: bind authorization to native transport, authenticated profile/capability, repository, PR, exact live head SHA, canonical incident evidence and a non-forgeable authorization artifact; require `expected_head_sha` and reject when it does not equal the live head. **F2 — Merger acceptance condition is described but not implemented (AC5, adversarial check C6).** - No merger/merge/adopt code consumes `merger_may_accept`, the `irrecoverable_decision_provenance` record, or `KIND_POST_MERGE_DECISION_RECOVERY`. They are write-only forensic artifacts; `acceptance_rule` is only a descriptive string. - AC5 requires precise merger acceptance conditions that are actually implemented and fail-closed. As shipped, the recovery path cannot sanction unblocking PR #703 through any implemented consumer, so the acceptance criterion is incomplete. - Required correction: implement a fail-closed merger consumption path that re-verifies repository, PR, exact head, durability/readback and absence of a conflicting terminal lock before honoring the record, or descope AC5 to the record-only guarantee and document the missing consumer. **F3 — Cross-profile cleanup does not verify repository or head in its fallback (AC1, AC6, adversarial check C2).** - Decision-lock files are keyed only by `<kind>-<profile>` (remote/org/repo intentionally excluded from the filename, validated on load). - `mcp_session_state.load_state_for_profile(skip_identity_match=True)` computes `filtered` (the remote/org/repo mismatch reasons) but never uses it; only expiry gates the return, so repository/remote/org are not enforced on cross-profile load. - `gitea_mcp_server._clear_decision_lock_for_profile` clears via a fallback branch when `last_terminal_mutation.pr_number == pr_number`, ignoring head SHA and repository. A same-profile decision lock recorded for a different repository's PR with the same number can be archived and cleared during a merge, destroying unrelated terminal evidence — the exact harm #709 targets. - Required correction: enforce remote/org/repo on load (use the `filtered` reasons) and require head-SHA match in the fallback clear branch. ### Non-blocking observations - AC2 overwrite protection is correctly enforced in production `init_review_decision_lock` via `assess_init_overwrite`; `force=True` cannot destroy terminal evidence. - AC3/AC4 post-merge recovery and audit-readback fail-closed logic is sound; merge is never undone. - No special-casing of PR #696/#703/#700 in production code; #693 and #594 are referenced, not closed or repurposed. [THREAD STATE LEDGER] What is true now: - server-side decision state: request_changes posted to gitea at head ec5cf677718b6a6a5fc9a5102b5ce1783592509a - local verdict/state: REQUEST_CHANGES prepared locally in isolated detached worktree at the pinned head What changed: - one reviewer REQUEST_CHANGES review posted to PR #710; server-side state changed (a review was created) What is blocked: - blocker classification: code blocker - PR #710 cannot merge until the author corrects F1, F2 and F3 - exact failing gates: AC5 authorization binding, AC5 merger acceptance implementation, AC1/AC6 repository and head scoping Who/what acts next: - next actor: author - required action: fix F1, F2 and F3 with regression tests, re-run the suite and request a fresh independent review Do not do: do not merge PR #710 or PR #703; do not self-review; do not treat the irrecoverable record as proof of historical cleanup.
sysadmin commented 2026-07-14 00:11:02 -05:00
Owner
Copy Link
Copy Source

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #710
issue: #709
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 40918-d0d14c123645
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710
phase: released
candidate_head: ec5cf67771
target_branch: master
target_branch_sha: 1eafb757a9
last_activity: 2026-07-14T05:11:02Z
expires_at: 2026-07-14T07:11:02Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #710 issue: #709 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 40918-d0d14c123645 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710 phase: released candidate_head: ec5cf677718b6a6a5fc9a5102b5ce1783592509a target_branch: master target_branch_sha: 1eafb757a91e4606ed9f82413e4122ad6bcf94fd last_activity: 2026-07-14T05:11:02Z expires_at: 2026-07-14T07:11:02Z blocker: manual-release
jcwalker3 added 1 commit 2026-07-14 00:36:43 -05:00
fix(workflow): non-forgeable irrecoverable auth, merger consumer, exact-scope cleanup (#709) 9cb12ee0f4 
Address formal review 434 REQUEST_CHANGES on PR #710:
- F1: replace caller operator_authorized with server-side HMAC auth artifacts
- F2: implement fail-closed merger consumption for prior-provenance only
- F3: enforce remote/org/repo/head on cross-profile load and clear

Co-Authored-By: Grok 4.5 (xAI) <noreply@x.ai>
jcwalker3 commented 2026-07-14 00:37:31 -05:00
Author
Owner
Copy Link
Copy Source

Canonical Issue State

STATE:
author-remediation-complete; ready-for-fresh-review

WHO_IS_NEXT:
reviewer

NEXT_ACTION:
Independent prgs-reviewer formal review of PR #710 at head 9cb12ee0f442a89535b6f81712367f1112a3e59e; re-check F1/F2/F3; do not merge PR #703; do not treat review 434 as resolved.

NEXT_PROMPT:

Role: REVIEWER (prgs-reviewer)
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
Issue: #709 | PR: #710
Pinned head: 9cb12ee0f442a89535b6f81712367f1112a3e59e
Prior review 434: REQUEST_CHANGES at ec5cf677… (stale; do not dismiss manually)

1. gitea_whoami + resolve review_pr; neutral runtime; acquire PR #710 lease first.
2. Re-verify F1 (non-forgeable auth), F2 (merger consumer), F3 (exact-scope load/clear).
3. Confirm no historical cleanup fabrication; no special-cased session IDs in production code.
4. One formal verdict via native MCP. Do not merge #710 or #703.

WHAT_HAPPENED:
Author remediations for review 434 pushed as new commit 9cb12ee (does not rewrite ec5cf677).

F1 mapping (self-assertable auth → server artifact)

  • Code: new irrecoverable_provenance.py; gitea_issue_irrecoverable_provenance_authorization; record tool rejects operator_authorized; requires HMAC auth + live head + incident_issue/comment_id; capability gitea.decision_lock.irrecoverable_recovery (or reconciler equivalence); confirmation is intent only.
  • Invariant: no caller Boolean / confirmation / gitea.read-only acceptable recovery; fresh non-pytest process cannot mint merger_may_accept=True.
  • Tests: TestF1AuthorizationNotSelfAssertable, TestIrrecoverableToolF1; subprocess proof.
  • Results: focused suite green (see VALIDATION).

F2 mapping (merger consumer)

  • Code: gitea_consume_irrecoverable_decision_lock_provenance; assess_merger_consumption; merge Gate 8b report+consume; durable idempotent mark_consumed.
  • Invariant: resolves only prior-provenance blocker; cannot bypass approval/CR/lease/mergeability/runtime/workspace/anti-stomp; historical_cleanup_proven always false.
  • Tests: TestF2MergerConsumer.
  • Results: green.

F3 mapping (exact-scope)

  • Code: load_state_for_profile enforces remote/org/repo (and missing identity); path traversal/symlink refused; _clear_decision_lock_for_profile requires head+repo scope — no PR-number-only fallback.
  • Invariant: same PR number other remote/org/repo/wrong head never selected or cleared.
  • Tests: TestF3ExactScopeEnforcement, TestClearProfileHelperF3.
  • Results: green.

Files: irrecoverable_provenance.py (new); gitea_mcp_server.py; mcp_session_state.py; stale_review_decision_lock.py; task_capability_map.py; tests/test_issue_709_decision_lock_cross_profile.py.

WHY:
Review 434 blocked AC5 authorization binding, AC5 merger acceptance, and AC1/AC6 repository+head scoping.

RELATED_PRS:
PR #710 (this PR, head 9cb12ee); PR #703 remains open unmerged untouched.

BLOCKERS:
none for fresh review; review 434 not claimed resolved; do not merge #703 from this stream.

VALIDATION:
pytest tests/test_issue_709_decision_lock_cross_profile.py: 46 passed; related native/decision-lock suite: 151 passed; full tests/: 2738 passed, 6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py), 161 subtests; head 9cb12ee0f442a89535b6f81712367f1112a3e59e; root checkout preserved.

LAST_UPDATED_BY:
jcwalker3 / prgs-author

## Canonical Issue State STATE: author-remediation-complete; ready-for-fresh-review WHO_IS_NEXT: reviewer NEXT_ACTION: Independent prgs-reviewer formal review of PR #710 at head 9cb12ee0f442a89535b6f81712367f1112a3e59e; re-check F1/F2/F3; do not merge PR #703; do not treat review 434 as resolved. NEXT_PROMPT: ```text Role: REVIEWER (prgs-reviewer) Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools Issue: #709 | PR: #710 Pinned head: 9cb12ee0f442a89535b6f81712367f1112a3e59e Prior review 434: REQUEST_CHANGES at ec5cf677… (stale; do not dismiss manually) 1. gitea_whoami + resolve review_pr; neutral runtime; acquire PR #710 lease first. 2. Re-verify F1 (non-forgeable auth), F2 (merger consumer), F3 (exact-scope load/clear). 3. Confirm no historical cleanup fabrication; no special-cased session IDs in production code. 4. One formal verdict via native MCP. Do not merge #710 or #703. ``` WHAT_HAPPENED: Author remediations for review 434 pushed as new commit 9cb12ee (does not rewrite ec5cf677). ### F1 mapping (self-assertable auth → server artifact) - **Code:** new `irrecoverable_provenance.py`; `gitea_issue_irrecoverable_provenance_authorization`; record tool rejects `operator_authorized`; requires HMAC auth + live head + incident_issue/comment_id; capability `gitea.decision_lock.irrecoverable_recovery` (or reconciler equivalence); confirmation is intent only. - **Invariant:** no caller Boolean / confirmation / gitea.read-only acceptable recovery; fresh non-pytest process cannot mint merger_may_accept=True. - **Tests:** TestF1AuthorizationNotSelfAssertable, TestIrrecoverableToolF1; subprocess proof. - **Results:** focused suite green (see VALIDATION). ### F2 mapping (merger consumer) - **Code:** `gitea_consume_irrecoverable_decision_lock_provenance`; `assess_merger_consumption`; merge Gate 8b report+consume; durable idempotent `mark_consumed`. - **Invariant:** resolves only prior-provenance blocker; cannot bypass approval/CR/lease/mergeability/runtime/workspace/anti-stomp; historical_cleanup_proven always false. - **Tests:** TestF2MergerConsumer. - **Results:** green. ### F3 mapping (exact-scope) - **Code:** `load_state_for_profile` enforces remote/org/repo (and missing identity); path traversal/symlink refused; `_clear_decision_lock_for_profile` requires head+repo scope — no PR-number-only fallback. - **Invariant:** same PR number other remote/org/repo/wrong head never selected or cleared. - **Tests:** TestF3ExactScopeEnforcement, TestClearProfileHelperF3. - **Results:** green. Files: irrecoverable_provenance.py (new); gitea_mcp_server.py; mcp_session_state.py; stale_review_decision_lock.py; task_capability_map.py; tests/test_issue_709_decision_lock_cross_profile.py. WHY: Review 434 blocked AC5 authorization binding, AC5 merger acceptance, and AC1/AC6 repository+head scoping. RELATED_PRS: PR #710 (this PR, head 9cb12ee); PR #703 remains open unmerged untouched. BLOCKERS: none for fresh review; review 434 not claimed resolved; do not merge #703 from this stream. VALIDATION: pytest tests/test_issue_709_decision_lock_cross_profile.py: 46 passed; related native/decision-lock suite: 151 passed; full tests/: 2738 passed, 6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py), 161 subtests; head 9cb12ee0f442a89535b6f81712367f1112a3e59e; root checkout preserved. LAST_UPDATED_BY: jcwalker3 / prgs-author
sysadmin commented 2026-07-14 01:02:02 -05:00
Owner
Copy Link
Copy Source

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #710
issue: #709
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 19475-9d7deb5c696b
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710-rereview-9cb12ee
phase: claimed
candidate_head: 9cb12ee0f4
target_branch: master
target_branch_sha: 1eafb757a9
last_activity: 2026-07-14T06:02:02Z
expires_at: 2026-07-14T08:02:02Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #710 issue: #709 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 19475-9d7deb5c696b worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710-rereview-9cb12ee phase: claimed candidate_head: 9cb12ee0f442a89535b6f81712367f1112a3e59e target_branch: master target_branch_sha: 1eafb757a91e4606ed9f82413e4122ad6bcf94fd last_activity: 2026-07-14T06:02:02Z expires_at: 2026-07-14T08:02:02Z blocker: none
sysadmin requested changes 2026-07-14 01:04:36 -05:00
sysadmin left a comment
Owner
Copy Link
Copy Source

Canonical PR State

STATE: changes_requested
WHO_IS_NEXT: author
NEXT_ACTION: Provision a durable non-ephemeral HMAC key so recovery authorizations survive restart and verify in the merger process; make incident evidence authoritative (author identity + canonical content/digest) and restrict minting to the dedicated capability; close the same-repo legacy no-head clear in the primary approve-match branch. Add regression tests and request a fresh independent review.
NEXT_PROMPT:

AUTHOR task PR #710 / Issue #709: address this REQUEST_CHANGES at head 9cb12ee.
(F4) irrecoverable_provenance._process_secret mints an ephemeral per-process secrets.token_bytes(32) (pytest uses a fixed constant). The HMAC-signed authorization is minted in the reconciler-capable process and verified in the merger process / after restart, where _process_secret returns a different key, so hmac.compare_digest always fails in production and the recovery record can never be consumed (AC3/AC5). Provision the key from a durable secret source (env/keychain) loaded at startup, fail closed if absent/invalid, add a key id/version in the signed scope, reject unknown versions.
(F5) assess_incident_evidence accepts any existing non-empty comment; validate the comment author against an authorized identity and require canonical structured content/digest bound to the exact scope; treat edited/deleted as fail closed. Restrict minting to gitea.decision_lock.irrecoverable_recovery (drop the broad reconciler-role/issue.comment equivalence) so ordinary comment permission cannot self-mint recovery authority.
(F3-residual) stale_review_decision_lock.lock_targets_merged_pr_approval returns True on pr_number+approve when the terminal head is unrecorded; make the primary clear branch require a recorded head equal to expected_head_sha (or refuse recovery_required), matching the secondary fallback.
Add regression tests each (cross-process/restart key, arbitrary/edited/wrong-author incident comment, dedicated-capability-only mint, same-repo legacy no-head lock not cleared); re-run tests/; do not merge PR #703.

WHAT_HAPPENED: Independent reviewer re-review at head 9cb12ee. Confirmed scope (6 files incl new irrecoverable_provenance.py, single remediation commit atop ec5cf677 which remains an ancestor/non-rewritten, generic, no #696/#703/#700 special-casing). Ran focused suite (60 passed) and full suite (2738 passed, 6 skipped, 1 warning, 161 subtests) independently. Prior review-434 F1/F2/F3 are largely remediated, but the new HMAC design and the incident-evidence/mint-authority surface introduce fresh blockers.
WHY: AC3/AC5 require recovery authorization that is durable across restart and consumable by the merger, incident evidence that is authoritative and scope-bound, and mint authority limited to the dedicated capability; AC1/AC6 require no PR-number-only clear even for legacy locks.
ISSUE: #709
HEAD_SHA: 9cb12ee0f4
REVIEW_STATUS: request_changes posted to gitea
MERGE_READY: no
BLOCKERS: F4 ephemeral per-process HMAC key breaks cross-process/restart consumption; F5 non-authoritative incident evidence + reconciler-equivalence self-mint; F3-residual same-repo legacy no-head approve clear.
VALIDATION: focused 60 passed; full 2738 passed, 6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py:8, not touched by this PR), 161 subtests passed.
LAST_UPDATED_BY: reviewer sysadmin / prgs-reviewer

Review-434 disposition

  • F1 (self-assertable operator_authorized): CORRECTED. operator_authorized is deprecated -> explicit deny (gitea_mcp_server.py:5370-5378); minting requires a server-side HMAC-signed artifact bound to remote/org/repo/PR/live-head/incident under native transport; confirmation is human intent only.
  • F2 (merger consumer): STRUCTURALLY IMPLEMENTED. gitea_consume_irrecoverable_decision_lock_provenance + inline Gate 8b read merger_may_accept, consume idempotently, and do not bypass approval/CR/mergeability/lease/anti-stomp gates (assess_merger_consumption fails closed on any). Ordering is consumed-before-merge but head-bound + idempotent + wrapped, so a failed merge does not burn the record. HOWEVER see F4 — the consumer cannot succeed in production because HMAC verification fails cross-process.
  • F3 (exact scope): MOSTLY CORRECTED. load_state_for_profile now enforces remote/org/repo (returns None), rejects traversal/symlink/legacy-missing-field; _clear_decision_lock_for_profile secondary fallback requires head+repo. One same-repo residual remains (F3-residual).

New / residual blockers

F4 — HMAC signing key is ephemeral per-process; recovery authorization cannot survive restart or cross-process merger consumption (AC3, AC5; HMAC review key-origin/persistence).

  • File: irrecoverable_provenance.py::_process_secret (51-58), _sign_scope (140-159), verify_authorization_artifact (490-517); consumed at gitea_mcp_server.py Gate 8b (6919-6946) and gitea_consume_irrecoverable_decision_lock_provenance (5751-5778).
  • Behavior: _PROCESS_AUTH_SECRET = secrets.token_bytes(32) minted per process; pytest short-circuits to b"pytest-irrecoverable-auth-v1". The authorization artifact is signed by the minting process and persisted to session-state, then verified by the merger process or after an MCP restart. Because the key is process-local and never persisted/shared, hmac.compare_digest only passes inside the exact process instance that minted it. In production the merger namespace is a different PID than the reconciler-capable process that mints, so assess_merger_consumption's auth check always fails and the record can never be consumed; any restart likewise invalidates every persisted authorization. No durable/env/keychain key origin exists and startup does not fail closed on an absent secret — it silently generates one.
  • Repro: mint under process key K1, reset _PROCESS_AUTH_SECRET (simulating restart/other process), verify -> "authorization server_signature invalid". The full suite is green only because pytest uses a shared fixed key, masking the production path.
  • Violated AC: AC3 (durable recovery survives restart), AC5 (implementable/consumable merger acceptance).
  • Required correction: derive the HMAC key from a durable secret, non-repository, non-caller source (env/keychain) loaded at startup; fail closed if absent/invalid; add explicit key id/version in the signed scope and reject unknown versions; keep constant-time compare.
  • Required test: mint-under-K1 / verify-under-K2 fails and verify-under-durable-key passes; restart-simulation proves a persisted artifact still verifies; missing/invalid key -> startup fail-closed.

F5 — Incident evidence not authoritative + reconciler mint-equivalence permits self-minted recovery authority (AC5; incident-evidence & mint-authority review).

  • File: irrecoverable_provenance.py::assess_incident_evidence (253-325), assess_capability_for_irrecoverable_recovery (162-229).
  • Behavior: assess_incident_evidence accepts ANY live comment whose id matches and body is non-empty (URL check is soft — only fails when a wrong-org URL is present). It never validates the comment author against an authorized identity nor requires canonical structured content/digest linked to the exact recovery scope. Mint authority additionally accepts "reconciler_profile_equivalence" (is_reconciler_profile + gitea.issue.comment) instead of the dedicated capability. A reconciler-capable caller can create its own issue comment and cite it as canonical incident evidence to mint authorization — i.e. ordinary issue-comment permission yields its own recovery authority, which AC5 explicitly forbids.
  • Required correction: validate incident comment author against an authorized identity/allowlist; require an expected structured payload or digest bound to remote/org/repo/PR/head; treat edited/deleted comments as fail closed; restrict minting to gitea.decision_lock.irrecoverable_recovery (remove or tightly bound the reconciler role-name equivalence).
  • Required test: arbitrary/wrong-author/edited/unrelated incident comment fails; reconciler-without-dedicated-capability cannot mint; only the dedicated capability mints.

F3-residual — primary approve-match still allows a PR-number-only clear of same-repo legacy locks with no recorded head (AC1, AC6).

  • File: stale_review_decision_lock.py::lock_targets_merged_pr_approval (513-519); consumed by gitea_mcp_server.py::_clear_decision_lock_for_profile primary branch (4626-4689).
  • Behavior: the predicate is if locked and not heads_equal(locked, expected): return False then return True — so when the terminal mutation has no recorded head, it returns True on pr_number + action=="approve" alone, and the primary branch archives+clears without a head check. This is inconsistent with the secondary fallback (4640-4659), which refuses the same legacy-missing-head case as recovery_required. Bounded to the same remote/org/repo (cross-repo is blocked by the scoped load), but is exactly the legacy-incomplete-lock destruction #709 targets.
  • Required correction: require a recorded head equal to expected_head_sha in the primary approve-match (or refuse recovery_required when the head is unrecorded), matching the secondary branch.
  • Required test: same-repo legacy approve lock, same PR number, no recorded head -> NOT cleared.

Non-blocking observations

  • HMAC field-binding tests re-mint fresh artifacts rather than mutating a validly-signed artifact, so they do not prove the signature cryptographically binds repo/PR/head/incident; add per-field-mutation + bit-flip + cross-repo/cross-environment signature-transplant tests (these will be needed by F4).
  • No HMAC key-handling tests (missing/default/wrong/rotated key/version); no expired-authorization rejection test; no edited/wrong-author incident-comment negative test.
  • Correctly implemented and credited: F1 caller-boolean rejection; applied and historical_cleanup_proven always false (build + refusal checks); Gate 8b consumed-before-merge idempotency; AC2 init overwrite protection via assess_init_overwrite (force=True cannot destroy terminal evidence). No special-casing of PR #696/#703/#700 in production code; #693/#594 referenced, not closed/repurposed.

[THREAD STATE LEDGER]

What is true now:

  • server-side decision state: request_changes posted to gitea at head 9cb12ee0f4
  • local verdict/state: REQUEST_CHANGES prepared in an isolated detached reviewer worktree pinned at 9cb12ee; focused 60 / full 2738 passed independently
  • prior review 434 remains stale and undismissed at ec5cf677 (not manually altered)

What changed:

  • one reviewer REQUEST_CHANGES review created on PR #710 at head 9cb12ee; server-side review state changed

What is blocked:

  • blocker classification: code blocker
  • PR #710 cannot merge until F4, F5 and F3-residual are corrected with regression tests
  • exact failing gates: AC3/AC5 durable and consumable authorization (F4), AC5 authoritative incident evidence + dedicated mint authority (F5), AC1/AC6 no PR-number-only legacy clear (F3-residual)

Who/what acts next:

  • next actor: author (jcwalker3)
  • required action: fix F4, F5, F3-residual with regression tests, re-run tests/, request a fresh independent review

Do not do: do not merge PR #710 or PR #703; do not self-review; do not treat the irrecoverable record as proof of historical cleanup; do not manually dismiss review 434.

## Canonical PR State STATE: changes_requested WHO_IS_NEXT: author NEXT_ACTION: Provision a durable non-ephemeral HMAC key so recovery authorizations survive restart and verify in the merger process; make incident evidence authoritative (author identity + canonical content/digest) and restrict minting to the dedicated capability; close the same-repo legacy no-head clear in the primary approve-match branch. Add regression tests and request a fresh independent review. NEXT_PROMPT: ```text AUTHOR task PR #710 / Issue #709: address this REQUEST_CHANGES at head 9cb12ee. (F4) irrecoverable_provenance._process_secret mints an ephemeral per-process secrets.token_bytes(32) (pytest uses a fixed constant). The HMAC-signed authorization is minted in the reconciler-capable process and verified in the merger process / after restart, where _process_secret returns a different key, so hmac.compare_digest always fails in production and the recovery record can never be consumed (AC3/AC5). Provision the key from a durable secret source (env/keychain) loaded at startup, fail closed if absent/invalid, add a key id/version in the signed scope, reject unknown versions. (F5) assess_incident_evidence accepts any existing non-empty comment; validate the comment author against an authorized identity and require canonical structured content/digest bound to the exact scope; treat edited/deleted as fail closed. Restrict minting to gitea.decision_lock.irrecoverable_recovery (drop the broad reconciler-role/issue.comment equivalence) so ordinary comment permission cannot self-mint recovery authority. (F3-residual) stale_review_decision_lock.lock_targets_merged_pr_approval returns True on pr_number+approve when the terminal head is unrecorded; make the primary clear branch require a recorded head equal to expected_head_sha (or refuse recovery_required), matching the secondary fallback. Add regression tests each (cross-process/restart key, arbitrary/edited/wrong-author incident comment, dedicated-capability-only mint, same-repo legacy no-head lock not cleared); re-run tests/; do not merge PR #703. ``` WHAT_HAPPENED: Independent reviewer re-review at head 9cb12ee. Confirmed scope (6 files incl new irrecoverable_provenance.py, single remediation commit atop ec5cf677 which remains an ancestor/non-rewritten, generic, no #696/#703/#700 special-casing). Ran focused suite (60 passed) and full suite (2738 passed, 6 skipped, 1 warning, 161 subtests) independently. Prior review-434 F1/F2/F3 are largely remediated, but the new HMAC design and the incident-evidence/mint-authority surface introduce fresh blockers. WHY: AC3/AC5 require recovery authorization that is durable across restart and consumable by the merger, incident evidence that is authoritative and scope-bound, and mint authority limited to the dedicated capability; AC1/AC6 require no PR-number-only clear even for legacy locks. ISSUE: #709 HEAD_SHA: 9cb12ee0f442a89535b6f81712367f1112a3e59e REVIEW_STATUS: request_changes posted to gitea MERGE_READY: no BLOCKERS: F4 ephemeral per-process HMAC key breaks cross-process/restart consumption; F5 non-authoritative incident evidence + reconciler-equivalence self-mint; F3-residual same-repo legacy no-head approve clear. VALIDATION: focused 60 passed; full 2738 passed, 6 skipped, 1 warning (pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py:8, not touched by this PR), 161 subtests passed. LAST_UPDATED_BY: reviewer sysadmin / prgs-reviewer ### Review-434 disposition - F1 (self-assertable operator_authorized): CORRECTED. operator_authorized is deprecated -> explicit deny (gitea_mcp_server.py:5370-5378); minting requires a server-side HMAC-signed artifact bound to remote/org/repo/PR/live-head/incident under native transport; confirmation is human intent only. - F2 (merger consumer): STRUCTURALLY IMPLEMENTED. gitea_consume_irrecoverable_decision_lock_provenance + inline Gate 8b read merger_may_accept, consume idempotently, and do not bypass approval/CR/mergeability/lease/anti-stomp gates (assess_merger_consumption fails closed on any). Ordering is consumed-before-merge but head-bound + idempotent + wrapped, so a failed merge does not burn the record. HOWEVER see F4 — the consumer cannot succeed in production because HMAC verification fails cross-process. - F3 (exact scope): MOSTLY CORRECTED. load_state_for_profile now enforces remote/org/repo (returns None), rejects traversal/symlink/legacy-missing-field; _clear_decision_lock_for_profile secondary fallback requires head+repo. One same-repo residual remains (F3-residual). ### New / residual blockers **F4 — HMAC signing key is ephemeral per-process; recovery authorization cannot survive restart or cross-process merger consumption (AC3, AC5; HMAC review key-origin/persistence).** - File: irrecoverable_provenance.py::_process_secret (51-58), _sign_scope (140-159), verify_authorization_artifact (490-517); consumed at gitea_mcp_server.py Gate 8b (6919-6946) and gitea_consume_irrecoverable_decision_lock_provenance (5751-5778). - Behavior: _PROCESS_AUTH_SECRET = secrets.token_bytes(32) minted per process; pytest short-circuits to b"pytest-irrecoverable-auth-v1". The authorization artifact is signed by the minting process and persisted to session-state, then verified by the merger process or after an MCP restart. Because the key is process-local and never persisted/shared, hmac.compare_digest only passes inside the exact process instance that minted it. In production the merger namespace is a different PID than the reconciler-capable process that mints, so assess_merger_consumption's auth check always fails and the record can never be consumed; any restart likewise invalidates every persisted authorization. No durable/env/keychain key origin exists and startup does not fail closed on an absent secret — it silently generates one. - Repro: mint under process key K1, reset _PROCESS_AUTH_SECRET (simulating restart/other process), verify -> "authorization server_signature invalid". The full suite is green only because pytest uses a shared fixed key, masking the production path. - Violated AC: AC3 (durable recovery survives restart), AC5 (implementable/consumable merger acceptance). - Required correction: derive the HMAC key from a durable secret, non-repository, non-caller source (env/keychain) loaded at startup; fail closed if absent/invalid; add explicit key id/version in the signed scope and reject unknown versions; keep constant-time compare. - Required test: mint-under-K1 / verify-under-K2 fails and verify-under-durable-key passes; restart-simulation proves a persisted artifact still verifies; missing/invalid key -> startup fail-closed. **F5 — Incident evidence not authoritative + reconciler mint-equivalence permits self-minted recovery authority (AC5; incident-evidence & mint-authority review).** - File: irrecoverable_provenance.py::assess_incident_evidence (253-325), assess_capability_for_irrecoverable_recovery (162-229). - Behavior: assess_incident_evidence accepts ANY live comment whose id matches and body is non-empty (URL check is soft — only fails when a wrong-org URL is present). It never validates the comment author against an authorized identity nor requires canonical structured content/digest linked to the exact recovery scope. Mint authority additionally accepts "reconciler_profile_equivalence" (is_reconciler_profile + gitea.issue.comment) instead of the dedicated capability. A reconciler-capable caller can create its own issue comment and cite it as canonical incident evidence to mint authorization — i.e. ordinary issue-comment permission yields its own recovery authority, which AC5 explicitly forbids. - Required correction: validate incident comment author against an authorized identity/allowlist; require an expected structured payload or digest bound to remote/org/repo/PR/head; treat edited/deleted comments as fail closed; restrict minting to gitea.decision_lock.irrecoverable_recovery (remove or tightly bound the reconciler role-name equivalence). - Required test: arbitrary/wrong-author/edited/unrelated incident comment fails; reconciler-without-dedicated-capability cannot mint; only the dedicated capability mints. **F3-residual — primary approve-match still allows a PR-number-only clear of same-repo legacy locks with no recorded head (AC1, AC6).** - File: stale_review_decision_lock.py::lock_targets_merged_pr_approval (513-519); consumed by gitea_mcp_server.py::_clear_decision_lock_for_profile primary branch (4626-4689). - Behavior: the predicate is `if locked and not heads_equal(locked, expected): return False` then `return True` — so when the terminal mutation has no recorded head, it returns True on pr_number + action=="approve" alone, and the primary branch archives+clears without a head check. This is inconsistent with the secondary fallback (4640-4659), which refuses the same legacy-missing-head case as recovery_required. Bounded to the same remote/org/repo (cross-repo is blocked by the scoped load), but is exactly the legacy-incomplete-lock destruction #709 targets. - Required correction: require a recorded head equal to expected_head_sha in the primary approve-match (or refuse recovery_required when the head is unrecorded), matching the secondary branch. - Required test: same-repo legacy approve lock, same PR number, no recorded head -> NOT cleared. ### Non-blocking observations - HMAC field-binding tests re-mint fresh artifacts rather than mutating a validly-signed artifact, so they do not prove the signature cryptographically binds repo/PR/head/incident; add per-field-mutation + bit-flip + cross-repo/cross-environment signature-transplant tests (these will be needed by F4). - No HMAC key-handling tests (missing/default/wrong/rotated key/version); no expired-authorization rejection test; no edited/wrong-author incident-comment negative test. - Correctly implemented and credited: F1 caller-boolean rejection; applied and historical_cleanup_proven always false (build + refusal checks); Gate 8b consumed-before-merge idempotency; AC2 init overwrite protection via assess_init_overwrite (force=True cannot destroy terminal evidence). No special-casing of PR #696/#703/#700 in production code; #693/#594 referenced, not closed/repurposed. [THREAD STATE LEDGER] What is true now: - server-side decision state: request_changes posted to gitea at head 9cb12ee0f442a89535b6f81712367f1112a3e59e - local verdict/state: REQUEST_CHANGES prepared in an isolated detached reviewer worktree pinned at 9cb12ee; focused 60 / full 2738 passed independently - prior review 434 remains stale and undismissed at ec5cf677 (not manually altered) What changed: - one reviewer REQUEST_CHANGES review created on PR #710 at head 9cb12ee; server-side review state changed What is blocked: - blocker classification: code blocker - PR #710 cannot merge until F4, F5 and F3-residual are corrected with regression tests - exact failing gates: AC3/AC5 durable and consumable authorization (F4), AC5 authoritative incident evidence + dedicated mint authority (F5), AC1/AC6 no PR-number-only legacy clear (F3-residual) Who/what acts next: - next actor: author (jcwalker3) - required action: fix F4, F5, F3-residual with regression tests, re-run tests/, request a fresh independent review Do not do: do not merge PR #710 or PR #703; do not self-review; do not treat the irrecoverable record as proof of historical cleanup; do not manually dismiss review 434.
sysadmin commented 2026-07-14 01:05:07 -05:00
Owner
Copy Link
Copy Source

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #710
issue: #709
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 19475-9d7deb5c696b
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710-rereview-9cb12ee
phase: released
candidate_head: 9cb12ee0f4
target_branch: master
target_branch_sha: 1eafb757a9
last_activity: 2026-07-14T06:05:06Z
expires_at: 2026-07-14T08:05:06Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #710 issue: #709 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 19475-9d7deb5c696b worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-710-rereview-9cb12ee phase: released candidate_head: 9cb12ee0f442a89535b6f81712367f1112a3e59e target_branch: master target_branch_sha: 1eafb757a91e4606ed9f82413e4122ad6bcf94fd last_activity: 2026-07-14T06:05:06Z expires_at: 2026-07-14T08:05:06Z blocker: manual-release
jcwalker3 added 1 commit 2026-07-14 01:13:53 -05:00
fix(workflow): durable HMAC, dedicated mint capability, head-exact approve match (#709) 2b359e0c26 
Address formal review 435 REQUEST_CHANGES on PR #710:
- F4: require durable GITEA_IRRECOVERABLE_AUTH_HMAC_KEY (fail closed; no ephemeral
  per-process secret); bind key_version into HMAC; cross-process verify works
- F5: dedicated gitea.decision_lock.irrecoverable_recovery only; reject reconciler
  equivalence; authoritative incident body + author + content_digest; reject
  self-authored incident evidence
- F3 residual: lock_targets_merged_pr_approval requires recorded-head match when
  expected_head_sha is provided (legacy no-head approve no longer primary-clears)

Co-Authored-By: Grok 4.5 (xAI) <noreply@x.ai>
jcwalker3 commented 2026-07-14 01:14:58 -05:00
Author
Owner
Copy Link
Copy Source

Canonical Issue State

STATE: author_remediation_pushed_awaiting_independent_rereview
WHO_IS_NEXT: reviewer
NEXT_ACTION: Fresh independent re-review of PR #710 at head 2b359e0 only; do not merge #710 or #703
NEXT_PROMPT:

Controller: dispatch independent REVIEWER (prgs-reviewer / sysadmin ≠ jcwalker3) to re-review PR #710 at head 2b359e0c260a524863378291f1c2d24516f7502a for issue #709. Verify review-435 F4 (durable HMAC + key_version + cross-process), F5 (dedicated capability only + authoritative incident digest/author), F3-residual (recorded-head primary approve-match). Do not reuse author worktree. Do not merge #710 or #703. Post formal verdict with reviewed_head_sha=2b359e0.

WHAT_HAPPENED: Author remediated review 435 REQUEST_CHANGES and pushed commit 2b359e0 on fix/issue-709-decision-lock-cross-profile. Isolated worktree branches/author-remediate-pr710-review435. Root master remains 1eafb75 untouched. F4: durable GITEA_IRRECOVERABLE_AUTH_HMAC_KEY required in production (fail closed; no ephemeral token_bytes); key_version bound into HMAC; cross-process mint/verify regression tests. F5: dedicated gitea.decision_lock.irrecoverable_recovery only (reconciler equivalence removed); incident evidence requires author identity, canonical marker/fields, content_digest, and rejects self-authored evidence. F3 residual: lock_targets_merged_pr_approval requires recorded-head match when expected_head_sha provided.
WHY: Review 435 blocked merge on production-dead HMAC, self-mintable recovery authority via reconciler equivalence + weak incident evidence, and primary clear of legacy no-head approve locks.
RELATED_PRS: #710 (this PR, head 2b359e0 open unmerged); #703 (open unmerged, no mutation this session); #709 (tracking issue)
BLOCKERS: Independent REVIEWER re-review not yet submitted at 2b359e0; review 435 still the last formal verdict until superseded by new-head review.
VALIDATION: Focused tests 70 passed (test_issue_709_decision_lock_cross_profile.py + test_stale_review_decision_lock_cleanup.py). Full tests/ 2748 passed, 6 skipped, 1 pre-existing StarletteDeprecationWarning at tests/test_webui_audit.py:8. PR #703 state open unmerged (updated_at unchanged from prior review session).
LAST_UPDATED_BY: jcwalker3 (prgs-author)

## Canonical Issue State STATE: author_remediation_pushed_awaiting_independent_rereview WHO_IS_NEXT: reviewer NEXT_ACTION: Fresh independent re-review of PR #710 at head 2b359e0 only; do not merge #710 or #703 NEXT_PROMPT: ```text Controller: dispatch independent REVIEWER (prgs-reviewer / sysadmin ≠ jcwalker3) to re-review PR #710 at head 2b359e0c260a524863378291f1c2d24516f7502a for issue #709. Verify review-435 F4 (durable HMAC + key_version + cross-process), F5 (dedicated capability only + authoritative incident digest/author), F3-residual (recorded-head primary approve-match). Do not reuse author worktree. Do not merge #710 or #703. Post formal verdict with reviewed_head_sha=2b359e0. ``` WHAT_HAPPENED: Author remediated review 435 REQUEST_CHANGES and pushed commit 2b359e0 on fix/issue-709-decision-lock-cross-profile. Isolated worktree branches/author-remediate-pr710-review435. Root master remains 1eafb75 untouched. F4: durable GITEA_IRRECOVERABLE_AUTH_HMAC_KEY required in production (fail closed; no ephemeral token_bytes); key_version bound into HMAC; cross-process mint/verify regression tests. F5: dedicated gitea.decision_lock.irrecoverable_recovery only (reconciler equivalence removed); incident evidence requires author identity, canonical marker/fields, content_digest, and rejects self-authored evidence. F3 residual: lock_targets_merged_pr_approval requires recorded-head match when expected_head_sha provided. WHY: Review 435 blocked merge on production-dead HMAC, self-mintable recovery authority via reconciler equivalence + weak incident evidence, and primary clear of legacy no-head approve locks. RELATED_PRS: #710 (this PR, head 2b359e0 open unmerged); #703 (open unmerged, no mutation this session); #709 (tracking issue) BLOCKERS: Independent REVIEWER re-review not yet submitted at 2b359e0; review 435 still the last formal verdict until superseded by new-head review. VALIDATION: Focused tests 70 passed (test_issue_709_decision_lock_cross_profile.py + test_stale_review_decision_lock_cleanup.py). Full tests/ 2748 passed, 6 skipped, 1 pre-existing StarletteDeprecationWarning at tests/test_webui_audit.py:8. PR #703 state open unmerged (updated_at unchanged from prior review session). LAST_UPDATED_BY: jcwalker3 (prgs-author)
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/issue-709-decision-lock-cross-profile:fix/issue-709-decision-lock-cross-profile
git checkout fix/issue-709-decision-lock-cross-profile
Sign in to join this conversation.
Reviewers
No Reviewers
sysadmin
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 Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jcwalker3 sysadmin
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#710
Delete Branch "fix/issue-709-decision-lock-cross-profile"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?