feat: enforce MCP restart class permissions #886

Merged
sysadmin merged 6 commits from feat/issue-663-restart-classes into master 2026-07-25 02:34:24 -05:00
Owner

Summary

  • define and document all nine MCP restart classes with least-privilege request permissions, expected blast radius, scoped drain requirements, approval, audit, and recovery behavior
  • enforce restart-class authorization, approval, fail-closed unknown-class handling, and target-scoped impact evaluation in the coordinator
  • derive permissions from the authenticated MCP role and include restart class data in coordinator and Web UI audit records
  • preserve narrower recovery behavior so reconnects and scoped restarts do not inherit a full-runtime drain

Validation

  • focused restart/governance suite: 122 passed, 16 subtests
  • full suite on this branch: 5110 passed, 6 skipped, 802 subtests, 23 failed
  • clean master baseline at 870843f999fb4bb8fc8c15ff7ede730448cdb3d4: 5099 passed, 6 skipped, 802 subtests, the same 23 tests failed
  • branch adds 11 passing tests and introduces no new full-suite failure signatures
  • git diff --check: passed

Scope and risk

The MCP restart request remains analysis-only; this change adds policy enforcement and audit metadata but does not add a process-kill or restart execution path. The principal risk is policy routing, covered by the nine-class permission and drain matrix tests plus existing coordinator, governance, Web UI, reconciliation, and preflight tests.

Closes #663

## Summary - define and document all nine MCP restart classes with least-privilege request permissions, expected blast radius, scoped drain requirements, approval, audit, and recovery behavior - enforce restart-class authorization, approval, fail-closed unknown-class handling, and target-scoped impact evaluation in the coordinator - derive permissions from the authenticated MCP role and include restart class data in coordinator and Web UI audit records - preserve narrower recovery behavior so reconnects and scoped restarts do not inherit a full-runtime drain ## Validation - focused restart/governance suite: 122 passed, 16 subtests - full suite on this branch: 5110 passed, 6 skipped, 802 subtests, 23 failed - clean `master` baseline at `870843f999fb4bb8fc8c15ff7ede730448cdb3d4`: 5099 passed, 6 skipped, 802 subtests, the same 23 tests failed - branch adds 11 passing tests and introduces no new full-suite failure signatures - `git diff --check`: passed ## Scope and risk The MCP restart request remains analysis-only; this change adds policy enforcement and audit metadata but does not add a process-kill or restart execution path. The principal risk is policy routing, covered by the nine-class permission and drain matrix tests plus existing coordinator, governance, Web UI, reconciliation, and preflight tests. Closes #663
jcwalker3 added 1 commit 2026-07-24 17:21:11 -05:00
Author
Owner

Canonical Issue State

STATE:
needs-review

WHO_IS_NEXT:
reviewer

NEXT_ACTION:
Review PR #886 at exact head 714190e02a against issue #663 and post a formal current-head review verdict.

NEXT_PROMPT:
Load the canonical gitea-workflow skill first. In prgs/Scaled-Tech-Consulting/Gitea-Tools, use a sanctioned reviewer identity distinct from jcwalker3/prgs-author. Recover PR #886 for issue #663, verify its live head is 714190e02a, inspect all acceptance criteria and validation evidence, run proportionate reviewer validation, and post a formal review verdict at the current head. Do not merge and do not operate as the author.

WHAT_HAPPENED:
The author implemented issue #663, committed and published the existing feature branch, and opened PR #886 targeting master.

WHY:
All issue acceptance criteria are implemented and the feature-specific validation passes; the remaining full-suite failures reproduce with the same signatures on the clean master baseline.

RELATED_DISCUSSION:
#663

RELATED_PRS:

BRANCH:
feat/issue-663-restart-classes

HEAD_SHA:
714190e02a

VALIDATION:
Focused restart/governance suite: 122 passed, 16 subtests. Full branch suite: 5110 passed, 6 skipped, 802 subtests, 23 failed. Clean master baseline: 5099 passed, 6 skipped, 802 subtests, the same exact 23 failures. Eleven new tests pass. git diff --check passed. PR read-back confirms open, mergeable, base master, author jcwalker3, and the pinned current head.

BLOCKERS:
none

LAST_UPDATED_BY:
jcwalker3 / prgs-author / 2026-07-24T22:22:03Z

Canonical Handoff

REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
ISSUE: 663
PR: 886
WORKFLOW_STATE: needs-review
HEAD_SHA: 714190e02a89636aa515320cddef9437d46dbc37
BASE_BRANCH: master
BASE_OR_MERGE_SHA: 870843f999fb4bb8fc8c15ff7ede730448cdb3d4
ACTING_ROLE: author
ACTING_IDENTITY: jcwalker3 (prgs-author)
COMPLETED_ACTIONS: Recovered assignment asn-5f3a3a1b475f45a4 and lease lease-d90ed50a525140ab; recovered the existing issue #663 branch/worktree lock; implemented all nine restart classes, least-privilege permissions, scoped drain/approval/audit enforcement, MCP and Web UI integration, documentation, and matrix tests; committed and published the branch; opened PR #886 targeting master.
VALIDATION_EVIDENCE: Focused restart/governance suite: 122 passed, 16 subtests; full branch suite: 5110 passed, 6 skipped, 802 subtests, 23 failed; clean master baseline at 870843f999fb4bb8fc8c15ff7ede730448cdb3d4: 5099 passed, 6 skipped, 802 subtests, the same exact 23 failures; 11 added tests pass and no new full-suite failure signature was introduced; git diff --check passed; PR read-back is open, mergeable, head feat/issue-663-restart-classes, base master, author jcwalker3, current head 714190e02a89636aa515320cddef9437d46dbc37.
MUTATION_LEDGER: Adopted the existing workflow lease; recovered and heartbeated the durable issue lock; edited and staged 7 issue-scoped files; created local commit 714190e02a89636aa515320cddef9437d46dbc37; published feat/issue-663-restart-classes with the sanctioned MCP publisher; created PR #886; transitioned issue #663 to status:pr-open; posted this PR-thread handoff.
BLOCKERS: none
NEXT_ACTOR: reviewer
NEXT_ACTION: Review PR #886 at exact head 714190e02a89636aa515320cddef9437d46dbc37 against issue #663 and post a formal current-head review verdict.
PROHIBITED_ACTIONS: Do not review using jcwalker3 or prgs-author; do not merge as reviewer; do not approve a changed head without re-review; do not force-push, rebase, manually push, or use direct Gitea APIs.
NEXT_PROMPT: Load the canonical gitea-workflow skill first. In prgs/Scaled-Tech-Consulting/Gitea-Tools, use a sanctioned reviewer identity distinct from jcwalker3/prgs-author. Recover PR #886 for issue #663, verify its live head is 714190e02a89636aa515320cddef9437d46dbc37, inspect all acceptance criteria and validation evidence, run proportionate reviewer validation, and post a formal review verdict at the current head. Do not merge and do not operate as the author.
WORKFLOW_FAILURE_ISSUES: #618
LAST_UPDATED: 2026-07-24T22:22:03Z
## Canonical Issue State STATE: needs-review WHO_IS_NEXT: reviewer NEXT_ACTION: Review PR #886 at exact head 714190e02a89636aa515320cddef9437d46dbc37 against issue #663 and post a formal current-head review verdict. NEXT_PROMPT: Load the canonical gitea-workflow skill first. In prgs/Scaled-Tech-Consulting/Gitea-Tools, use a sanctioned reviewer identity distinct from jcwalker3/prgs-author. Recover PR #886 for issue #663, verify its live head is 714190e02a89636aa515320cddef9437d46dbc37, inspect all acceptance criteria and validation evidence, run proportionate reviewer validation, and post a formal review verdict at the current head. Do not merge and do not operate as the author. WHAT_HAPPENED: The author implemented issue #663, committed and published the existing feature branch, and opened PR #886 targeting master. WHY: All issue acceptance criteria are implemented and the feature-specific validation passes; the remaining full-suite failures reproduce with the same signatures on the clean master baseline. RELATED_DISCUSSION: #663 RELATED_PRS: - #886 BRANCH: feat/issue-663-restart-classes HEAD_SHA: 714190e02a89636aa515320cddef9437d46dbc37 VALIDATION: Focused restart/governance suite: 122 passed, 16 subtests. Full branch suite: 5110 passed, 6 skipped, 802 subtests, 23 failed. Clean master baseline: 5099 passed, 6 skipped, 802 subtests, the same exact 23 failures. Eleven new tests pass. git diff --check passed. PR read-back confirms open, mergeable, base master, author jcwalker3, and the pinned current head. BLOCKERS: none LAST_UPDATED_BY: jcwalker3 / prgs-author / 2026-07-24T22:22:03Z <!-- sph:v1 --> ## Canonical Handoff ```text REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools ISSUE: 663 PR: 886 WORKFLOW_STATE: needs-review HEAD_SHA: 714190e02a89636aa515320cddef9437d46dbc37 BASE_BRANCH: master BASE_OR_MERGE_SHA: 870843f999fb4bb8fc8c15ff7ede730448cdb3d4 ACTING_ROLE: author ACTING_IDENTITY: jcwalker3 (prgs-author) COMPLETED_ACTIONS: Recovered assignment asn-5f3a3a1b475f45a4 and lease lease-d90ed50a525140ab; recovered the existing issue #663 branch/worktree lock; implemented all nine restart classes, least-privilege permissions, scoped drain/approval/audit enforcement, MCP and Web UI integration, documentation, and matrix tests; committed and published the branch; opened PR #886 targeting master. VALIDATION_EVIDENCE: Focused restart/governance suite: 122 passed, 16 subtests; full branch suite: 5110 passed, 6 skipped, 802 subtests, 23 failed; clean master baseline at 870843f999fb4bb8fc8c15ff7ede730448cdb3d4: 5099 passed, 6 skipped, 802 subtests, the same exact 23 failures; 11 added tests pass and no new full-suite failure signature was introduced; git diff --check passed; PR read-back is open, mergeable, head feat/issue-663-restart-classes, base master, author jcwalker3, current head 714190e02a89636aa515320cddef9437d46dbc37. MUTATION_LEDGER: Adopted the existing workflow lease; recovered and heartbeated the durable issue lock; edited and staged 7 issue-scoped files; created local commit 714190e02a89636aa515320cddef9437d46dbc37; published feat/issue-663-restart-classes with the sanctioned MCP publisher; created PR #886; transitioned issue #663 to status:pr-open; posted this PR-thread handoff. BLOCKERS: none NEXT_ACTOR: reviewer NEXT_ACTION: Review PR #886 at exact head 714190e02a89636aa515320cddef9437d46dbc37 against issue #663 and post a formal current-head review verdict. PROHIBITED_ACTIONS: Do not review using jcwalker3 or prgs-author; do not merge as reviewer; do not approve a changed head without re-review; do not force-push, rebase, manually push, or use direct Gitea APIs. NEXT_PROMPT: Load the canonical gitea-workflow skill first. In prgs/Scaled-Tech-Consulting/Gitea-Tools, use a sanctioned reviewer identity distinct from jcwalker3/prgs-author. Recover PR #886 for issue #663, verify its live head is 714190e02a89636aa515320cddef9437d46dbc37, inspect all acceptance criteria and validation evidence, run proportionate reviewer validation, and post a formal review verdict at the current head. Do not merge and do not operate as the author. WORKFLOW_FAILURE_ISSUES: #618 LAST_UPDATED: 2026-07-24T22:22:03Z ```
sysadmin approved these changes 2026-07-24 17:50:14 -05:00
Dismissed
sysadmin left a comment
Owner

Formal Review Report: APPROVED

STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: merger lease acquisition and landing on master

Verification

  • Scope: docs/mcp-restart-classes.md (+53), docs/mcp-restart-coordinator.md (+14), gitea_mcp_server.py (+30/-6), restart_coordinator.py (+384/-13), tests/test_restart_classes.py (+232), webui/sanctioned_restart.py (+20/-2), tests/test_webui_sanctioned_restart.py (+6).
  • Test Execution:
    • pytest tests/test_restart_classes.py tests/test_webui_sanctioned_restart.py tests/test_restart_coordinator.py -> 67 passed, 12 subtests
    • Full restart suite (104 tests) -> 104 passed in 0.86s
  • Security & Policy Enforcement:
    • All nine restart classes strictly defined with least-privilege permission requirements.
    • Target-scoped impact scoping for worker, role_runtime, and connector restarts prevents unnecessary full-runtime drains.
    • Fail-closed evaluation on unknown restart classes or unauthorized permissions.
    • Auditing metadata expanded to include restart_class across coordinator and Web UI ledgers.
## Formal Review Report: APPROVED STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: merger lease acquisition and landing on master ### Verification - Scope: `docs/mcp-restart-classes.md` (+53), `docs/mcp-restart-coordinator.md` (+14), `gitea_mcp_server.py` (+30/-6), `restart_coordinator.py` (+384/-13), `tests/test_restart_classes.py` (+232), `webui/sanctioned_restart.py` (+20/-2), `tests/test_webui_sanctioned_restart.py` (+6). - Test Execution: - `pytest tests/test_restart_classes.py tests/test_webui_sanctioned_restart.py tests/test_restart_coordinator.py` -> 67 passed, 12 subtests - Full restart suite (104 tests) -> 104 passed in 0.86s - Security & Policy Enforcement: - All nine restart classes strictly defined with least-privilege permission requirements. - Target-scoped impact scoping for worker, role_runtime, and connector restarts prevents unnecessary full-runtime drains. - Fail-closed evaluation on unknown restart classes or unauthorized permissions. - Auditing metadata expanded to include `restart_class` across coordinator and Web UI ledgers.
jcwalker3 added 1 commit 2026-07-24 21:06:23 -05:00
jcwalker3 added 1 commit 2026-07-24 21:27:18 -05:00
jcwalker3 added 1 commit 2026-07-24 22:34:35 -05:00
Author
Owner

pr: #886
branch: feat/issue-663-restart-classes
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes
profile: prgs-author
session_id: unknown
phase: claimed
head_before: 930dc24632
expires_at: 2026-07-25T07:17:24Z
reviewer_active: no

<!-- mcp-conflict-fix-lease:v1 --> pr: #886 branch: feat/issue-663-restart-classes worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes profile: prgs-author session_id: unknown phase: claimed head_before: 930dc24632ceb87f88241020ee4bf78ea39b6d16 expires_at: 2026-07-25T07:17:24Z reviewer_active: no
jcwalker3 added 1 commit 2026-07-25 00:17:45 -05:00
Brings PR #886 up to date with master @ 2f4dec8323
(8 commits behind), resolving the single conflicted file.

Conflict: gitea_mcp_server.py, both hunks inside gitea_request_mcp_restart.
Both sides were purely additive to the same tool, so both are kept in full:

- Branch side (#663, restart classes): parameters restart_class,
  target_session_id, target_role, target_connector; payload keys
  controller_approval_authorized, requester_role, requester_permissions.
- Master side (#661 via PR #882, drain-proof hard gate): parameters
  drain_proof_json, request_break_glass; the explanatory comment describing
  the apply-path hard gate and break-glass authorization.

No behaviour from either side was dropped, reordered, or reimplemented. Every
parameter from both sides is already consumed by the auto-merged function body
(restart_class and the three target_* arguments flow into the coordinator call;
drain_proof_json and request_break_glass drive the dry_run=False hard gate), so
the union is the only resolution that keeps the merged function coherent.

Validation on the merged tree:

  python -m pytest tests/test_drain_proof.py tests/test_restart_classes.py \
    tests/test_restart_coordinator.py tests/test_mcp_restart_paths.py \
    tests/test_mcp_restart_governance_docs.py tests/test_webui_sanctioned_restart.py \
    tests/test_issue_662_post_restart_reconcile.py -q
  # 165 passed, 68 subtests passed

py_compile on gitea_mcp_server.py passes and no conflict markers remain.

Closes #663

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Author
Owner

Conflict remediation complete — PR #886 no longer carries conflicts

Author-role conflict remediation for PR #886 (feat/issue-663-restart-classes), performed in the bound worktree with no rebase and no force-push.

Conflict-fix lease release

LEASE: conflict_fix
PR: #886
BRANCH: feat/issue-663-restart-classes
WORKTREE: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes
PROFILE: prgs-author
PHASE: released
HEAD_BEFORE: 930dc24632ceb87f88241020ee4bf78ea39b6d16
HEAD_AFTER: 9bc021e9c0e7212d976d6448ba873eb3bb775a75
CLAIM_COMMENT_ID: 16508
REVIEWER_ACTIVE: no

What conflicted

The branch was 8 commits behind master. Exactly one file conflicted — gitea_mcp_server.py — in two adjacent hunks, both inside the gitea_request_mcp_restart tool. Both sides were purely additive to the same function:

Side Adds
Branch (#663, restart classes) params restart_class, target_session_id, target_role, target_connector; payload keys controller_approval_authorized, requester_role, requester_permissions
Master (#661 via PR #882, which landed on master) params drain_proof_json, request_break_glass; the comment describing the apply-path hard gate and break-glass authorization

How it was resolved

Union of both sides, in full. Nothing was dropped, reordered, or reimplemented.

The union is not a stylistic choice — it is the only coherent resolution, because the auto-merge output for the function body already consumes every parameter from both sides. restart_class and the three target_* arguments flow into the restart_coordinator call; drain_proof_json and request_break_glass drive the dry_run=False hard gate and the break-glass branch. Dropping either side's parameters would leave the combined body referencing undefined names.

Verification evidence

  • No conflict markers remain anywhere in the tree (grep -rn '^<<<<<<< \|^>>>>>>> ' --include='*.py' returns empty).
  • python -m py_compile gitea_mcp_server.py passes.
  • master @ 2f4dec832327513118f2fe92b74da25d124a01cb is now an ancestor of the branch head; commits_behind is 0.
  • The push was a true fast-forward: 930dc24..9bc021e, non-force. The prior head 930dc246 remains an ancestor.
  • Focused suite over the combined tree, covering both feature sets:
python -m pytest tests/test_drain_proof.py tests/test_restart_classes.py \
  tests/test_restart_coordinator.py tests/test_mcp_restart_paths.py \
  tests/test_mcp_restart_governance_docs.py tests/test_webui_sanctioned_restart.py \
  tests/test_issue_662_post_restart_reconcile.py -q
# 165 passed, 68 subtests passed
  • Live post-push state from gitea_assess_pr_sync_status: mergeable: true, has_conflicts: false, commits_behind: 0.

Review status — a fresh independent review is required

The prior review verdict was recorded against 930dc24632ceb87f88241020ee4bf78ea39b6d16 and does not carry to the new head. gitea_assess_pr_sync_status reports approval_at_current_head: false, stale_approval: true, approval_valid_for_merge: false, recommended_next_action: fresh_review_required.

Canonical Issue State

STATE: awaiting-review

WHO_IS_NEXT: reviewer

NEXT_ACTION: Perform a fresh independent review of PR #886 at exact head 9bc021e9c0e7212d976d6448ba873eb3bb775a75, then record a verdict at that exact head.

BLOCKERS: none

VALIDATION: focused suite over the combined tree — 165 passed, 68 subtests passed; py_compile passes; zero conflict markers; live state mergeable: true, has_conflicts: false, commits_behind: 0.

LAST_UPDATED_BY: prgs-author (jcwalker3)

WHAT_HAPPENED: PR #886 stopped being mergeable after master advanced 8 commits. master @ 2f4dec8323 was integrated into feat/issue-663-restart-classes via a git merge commit inside the bound worktree, the single conflicted file gitea_mcp_server.py was resolved as a full union of both sides, and the result was pushed as fast-forward 930dc24..9bc021e.

WHY: #663 (restart classes) and #661 (drain-proof hard gate, via PR #882) both extended the same tool gitea_request_mcp_restart with new parameters and new payload keys. Git could not order the two additive edits to the same parameter list, so it raised a content conflict even though the two changes are independent and non-overlapping in behaviour.

RELATED_PRS: #886, #882

NEXT_PROMPT:

As prgs-reviewer on PR #886 at exact head 9bc021e9c0e7212d976d6448ba873eb3bb775a75: verify the nine restart-class permission matrix and fail-closed unknown-class handling still hold alongside the #661 drain-proof hard gate; confirm the union resolution in gitea_request_mcp_restart drops no behaviour from either side and that both the target_* scoping arguments and the drain_proof_json/request_break_glass gate remain effective; record APPROVE or REQUEST_CHANGES at that exact head; stop.

[THREAD STATE LEDGER]

What is true now

  • Server-side decision state: PR #886 is in open state at head 9bc021e9c0e7212d976d6448ba873eb3bb775a75; Gitea reports mergeable: true, has_conflicts: false, commits_behind: 0; approval_at_current_head: false and approval_valid_for_merge: false.
  • Local verdict/state: conflict remediation is complete and verified in the bound worktree; focused suite 165 passed, 68 subtests passed; py_compile passes; zero conflict markers remain.

What changed

  • master @ 2f4dec832327513118f2fe92b74da25d124a01cb was integrated into feat/issue-663-restart-classes via a git merge commit 9bc021e9c0e7212d976d6448ba873eb3bb775a75 with parents 930dc24 and 2f4dec8.
  • gitea_mcp_server.py was resolved as a full union of both sides inside gitea_request_mcp_restart; no other file conflicted.
  • The branch was pushed 930dc24..9bc021e as a fast-forward, non-force. No rebase occurred.
  • The prior review verdict at 930dc246 no longer applies to the current head.

What is blocked

  • Blocker classification: no blocker
  • Nothing is waiting on the author role. The conflict-fix lease claimed by comment 16508 is released by this comment.

Who/what acts next

  • Next actor: prgs-reviewer
  • Required action: perform a fresh independent review of PR #886 at exact head 9bc021e9c0e7212d976d6448ba873eb3bb775a75 and record a verdict at that exact head.
  • Do not do: do not reuse the prior review verdict recorded at 930dc24632ceb87f88241020ee4bf78ea39b6d16 for merge eligibility; do not rebase or force-push this branch; do not merge from the author or reviewer role.
## Conflict remediation complete — PR #886 no longer carries conflicts Author-role conflict remediation for PR #886 (`feat/issue-663-restart-classes`), performed in the bound worktree with no rebase and no force-push. ### Conflict-fix lease release ```text LEASE: conflict_fix PR: #886 BRANCH: feat/issue-663-restart-classes WORKTREE: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes PROFILE: prgs-author PHASE: released HEAD_BEFORE: 930dc24632ceb87f88241020ee4bf78ea39b6d16 HEAD_AFTER: 9bc021e9c0e7212d976d6448ba873eb3bb775a75 CLAIM_COMMENT_ID: 16508 REVIEWER_ACTIVE: no ``` ### What conflicted The branch was 8 commits behind `master`. Exactly one file conflicted — `gitea_mcp_server.py` — in two adjacent hunks, both inside the `gitea_request_mcp_restart` tool. Both sides were purely additive to the same function: | Side | Adds | |------|------| | Branch (#663, restart classes) | params `restart_class`, `target_session_id`, `target_role`, `target_connector`; payload keys `controller_approval_authorized`, `requester_role`, `requester_permissions` | | Master (#661 via PR #882, which landed on master) | params `drain_proof_json`, `request_break_glass`; the comment describing the apply-path hard gate and break-glass authorization | ### How it was resolved Union of both sides, in full. Nothing was dropped, reordered, or reimplemented. The union is not a stylistic choice — it is the only coherent resolution, because the auto-merge output for the function body already consumes every parameter from both sides. `restart_class` and the three `target_*` arguments flow into the `restart_coordinator` call; `drain_proof_json` and `request_break_glass` drive the `dry_run=False` hard gate and the break-glass branch. Dropping either side's parameters would leave the combined body referencing undefined names. ### Verification evidence - No conflict markers remain anywhere in the tree (`grep -rn '^<<<<<<< \|^>>>>>>> ' --include='*.py'` returns empty). - `python -m py_compile gitea_mcp_server.py` passes. - `master` @ `2f4dec832327513118f2fe92b74da25d124a01cb` is now an ancestor of the branch head; `commits_behind` is `0`. - The push was a true fast-forward: `930dc24..9bc021e`, non-force. The prior head `930dc246` remains an ancestor. - Focused suite over the combined tree, covering both feature sets: ```text python -m pytest tests/test_drain_proof.py tests/test_restart_classes.py \ tests/test_restart_coordinator.py tests/test_mcp_restart_paths.py \ tests/test_mcp_restart_governance_docs.py tests/test_webui_sanctioned_restart.py \ tests/test_issue_662_post_restart_reconcile.py -q # 165 passed, 68 subtests passed ``` - Live post-push state from `gitea_assess_pr_sync_status`: `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`. ### Review status — a fresh independent review is required The prior review verdict was recorded against `930dc24632ceb87f88241020ee4bf78ea39b6d16` and does not carry to the new head. `gitea_assess_pr_sync_status` reports `approval_at_current_head: false`, `stale_approval: true`, `approval_valid_for_merge: false`, `recommended_next_action: fresh_review_required`. ## Canonical Issue State STATE: awaiting-review WHO_IS_NEXT: reviewer NEXT_ACTION: Perform a fresh independent review of PR #886 at exact head `9bc021e9c0e7212d976d6448ba873eb3bb775a75`, then record a verdict at that exact head. BLOCKERS: none VALIDATION: focused suite over the combined tree — 165 passed, 68 subtests passed; `py_compile` passes; zero conflict markers; live state `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`. LAST_UPDATED_BY: prgs-author (jcwalker3) WHAT_HAPPENED: PR #886 stopped being mergeable after `master` advanced 8 commits. `master` @ `2f4dec8323` was integrated into `feat/issue-663-restart-classes` via a git merge commit inside the bound worktree, the single conflicted file `gitea_mcp_server.py` was resolved as a full union of both sides, and the result was pushed as fast-forward `930dc24..9bc021e`. WHY: #663 (restart classes) and #661 (drain-proof hard gate, via PR #882) both extended the same tool `gitea_request_mcp_restart` with new parameters and new payload keys. Git could not order the two additive edits to the same parameter list, so it raised a content conflict even though the two changes are independent and non-overlapping in behaviour. RELATED_PRS: #886, #882 NEXT_PROMPT: ```text As prgs-reviewer on PR #886 at exact head 9bc021e9c0e7212d976d6448ba873eb3bb775a75: verify the nine restart-class permission matrix and fail-closed unknown-class handling still hold alongside the #661 drain-proof hard gate; confirm the union resolution in gitea_request_mcp_restart drops no behaviour from either side and that both the target_* scoping arguments and the drain_proof_json/request_break_glass gate remain effective; record APPROVE or REQUEST_CHANGES at that exact head; stop. ``` [THREAD STATE LEDGER] **What is true now** - Server-side decision state: PR #886 is in open state at head `9bc021e9c0e7212d976d6448ba873eb3bb775a75`; Gitea reports `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`; `approval_at_current_head: false` and `approval_valid_for_merge: false`. - Local verdict/state: conflict remediation is complete and verified in the bound worktree; focused suite 165 passed, 68 subtests passed; `py_compile` passes; zero conflict markers remain. **What changed** - `master` @ `2f4dec832327513118f2fe92b74da25d124a01cb` was integrated into `feat/issue-663-restart-classes` via a git merge commit `9bc021e9c0e7212d976d6448ba873eb3bb775a75` with parents `930dc24` and `2f4dec8`. - `gitea_mcp_server.py` was resolved as a full union of both sides inside `gitea_request_mcp_restart`; no other file conflicted. - The branch was pushed `930dc24..9bc021e` as a fast-forward, non-force. No rebase occurred. - The prior review verdict at `930dc246` no longer applies to the current head. **What is blocked** - Blocker classification: no blocker - Nothing is waiting on the author role. The conflict-fix lease claimed by comment 16508 is released by this comment. **Who/what acts next** - Next actor: prgs-reviewer - Required action: perform a fresh independent review of PR #886 at exact head `9bc021e9c0e7212d976d6448ba873eb3bb775a75` and record a verdict at that exact head. - Do not do: do not reuse the prior review verdict recorded at `930dc24632ceb87f88241020ee4bf78ea39b6d16` for merge eligibility; do not rebase or force-push this branch; do not merge from the author or reviewer role.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #886
issue: #663
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: reviewer-886-9bc021e-fresh
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr886-head-9bc021e
phase: claimed
candidate_head: 9bc021e9c0
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T06:05:48Z
expires_at: 2026-07-25T06:15:48Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #886 issue: #663 reviewer_identity: sysadmin profile: prgs-reviewer session_id: reviewer-886-9bc021e-fresh worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr886-head-9bc021e phase: claimed candidate_head: 9bc021e9c0e7212d976d6448ba873eb3bb775a75 target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T06:05:48Z expires_at: 2026-07-25T06:15:48Z blocker: none
Owner

Formal Review Findings — REQUEST_CHANGES (verdict not recordable via the review API at this moment)

PR #886 · issue #663 · head 9bc021e9c0e7212d976d6448ba873eb3bb775a75 · base master @ 2f4dec832327513118f2fe92b74da25d124a01cb

Independent re-review at the exact current head. Review 578 (sysadmin, APPROVE verdict) was recorded at 714190e02a89636aa515320cddef9437d46dbc37; approval_at_current_head: false, so it was not reused and carries no weight here.

Union verification — the 9bc021e conflict resolution

9bc021e has parents 930dc246 (#663 branch) and 2f4dec83 (master); merge base 7af40fb5. One conflicted file, gitea_mcp_server.py, inside gitea_request_mcp_restart.

Both directions verified by patch comparison:

  • diff(master → 9bc021e) on that file reproduces the #663 side exactly: the four new parameters (restart_class, target_session_id, target_role, target_connector), the requester_role derivation, controller_approved, requester_permissions, all eight coordinator kwargs, and the three payload keys (controller_approval_authorized, requester_role, requester_permissions).
  • diff(930dc246 → 9bc021e) on that file reproduces the #661 side exactly: the drain_proof import, drain_proof_json / request_break_glass, the rewritten docstring, the hard-gate comment, and the whole if not dry_run: gate body.

Stronger result: every added/removed line of the #663 delta is identical between the previously reviewed 714190e and the current head — 739 changed lines, zero differences. The union dropped no behaviour from either side.

The #661 hard gate was re-verified at this head by direct probe. Apply requests deny on: absent proof (with restart_drain_gate_denied incident), unclean proof, tampered proof (signature mismatch), expired proof, and stale impact fingerprint. Break-glass authorization stays environment-derived and is never self-asserted — break_glass = request_break_glass and break_glass_authorized, the latter read only from GITEA_BREAKGLASS_RESTART_AUTHORIZATION.

The 15 commits 714190e..930dc246

These carry no net new content. Only 3 of the 15 are not already ancestors of master, and all 3 are catch-up Merge branch 'master' into feat/issue-663-restart-classes commits. All 6 non-merge commits — 0a78da3 (#854), 5deb66c and c74b8da (#682), 069a9af, 1948d3d, dac40ab (#640) — are already ancestors of master, having landed there via PRs #883, #884 and #885. The effective diff of this PR against master is exactly the seven #663 files.

Independent baseline (author figures not used)

Both runs performed in branches/ worktrees; a /tmp checkout produces phantom path-resolution failures and is not comparable.

tree result
base 2f4dec83 23 failed, 5190 passed, 6 skipped, 867 subtests
head 9bc021e 23 failed, 5201 passed, 6 skipped, 867 subtests

Identical 23-test failure set on both sides; +11 passing, no new failure signature. Focused restart/drain suites: base 117 passed / 68 subtests → head 128 passed / 68 subtests.

AC verification (independent probe, not the branch's own assertions)

  • Nine classes present and enforced; rolling_mcp_restart, full_mcp_restart and host_restart deny for author/reviewer/merger/reconciler and allow for controller/operator/admin.
  • Unknown class denies fail-closed: verdict=unsafe, allow_restart=False, reason unknown restart class ...; deny (fail closed).
  • Target-scoped classes fail closed without a target: worker_restart, role_runtime_restart, connector_restart each report target required for <class>.
  • AC3 holds: with a live foreign session, a live disruptive lease and a terminal lock all present, client_reconnect and session_reconnect stay verdict=safe while full_mcp_restart goes unsafe. Narrower classes do not inherit a full-runtime drain.

B1 — blocker — apply_authorized does not consult restart-class authorization

gitea_request_mcp_restart computes the apply decision purely from the drain gate:

payload["apply_gate"] = gate_payload
payload["apply_authorized"] = gate.allow

gate.allow is a function of the drain proof (or break-glass) only. It never reads report.allow_restart, permission_authorized, role_authorized or approval_satisfied. The two gates introduced by #663 and #661 are therefore disjoint rather than conjoined, and the merge is the first point at which they coexist.

Reproduced against this exact head. An author role requesting full_mcp_restart is denied by the class matrix:

verdict=unsafe allow_restart=False perm=False role=False approval=False
  missing required permission 'mcp.restart.full.request'
  role 'author' may not request full_mcp_restart
  approval requirement not satisfied: controller_approval_and_automated_gates

A drain proof minted against that same denied report is clean, because _evaluate_checks derives from mutations, critical sections and disruptive leases and never inspects the authorization outcome. The gate then returns:

GATE: allow=True verdict=allow
report.allow_restart = False
tool would set apply_authorized = True

The payload simultaneously states allow_restart: False with an explicit "role 'author' may not request full_mcp_restart" reason and apply_authorized: True. In a change whose purpose is least-privilege restart authorization, that is a self-contradictory authorization contract, and apply_authorized is the field a later execution child is documented to consume.

Reachability today is limited but not nil. No production caller mints proofs — build_drain_proof has no non-test caller and the signing secret is per-process os.urandom(32), so a proof cannot be forged through MCP. The break-glass path, however, needs no proof: with GITEA_BREAKGLASS_RESTART_AUTHORIZATION present, any role calling restart_class="host_restart", request_break_glass=True, dry_run=False obtains apply_authorized: True while the matrix denies the class. One environment variable collapses the whole nine-class matrix for the apply decision, including the class whose policy demands controller_approval_plus_infrastructure_operator.

Nothing restarts today (apply_supported is always False and no executor exists), so this is a latent contract defect rather than a live escalation — but it should not ship in this shape.

Suggested fix, in gitea_request_mcp_restart:

payload["apply_authorized"] = bool(gate.allow and report.allow_restart)

and surface the class-authorization reasons in apply_gate when they are what denies, so a denial is attributable.

B2 — blocker — docs/mcp-restart-coordinator.md now describes pre-#661 behaviour

The merge left the coordinator document describing the tool as it existed before the drain gate arrived.

  • The documented signature block omits drain_proof_json and request_break_glass. Neither parameter appears anywhere in docs/mcp-restart-coordinator.md or docs/mcp-restart-classes.md (0 occurrences in each).
  • The prose states that passing dry_run=False "performs no restart and reports that apply is gated by a drain proof (a separate child)". At this head dry_run=False evaluates the hard gate inside this tool and returns apply_gate, apply_authorized, break_glass_requested, break_glass_authorized and, on denial, incident.

A safety document asserting there is no gate where a gate now exists will mislead the next author. Both statements need updating to the post-union behaviour.

Non-blocking observations

  • N1 — configuration_reload scopes sessions and leases out of its impact set but the terminal lock is deliberately retained (terminal_lock_in_scope excludes only client_reconnect and session_reconnect). The result is blast_radius=none together with verdict=unsafe whenever a terminal lock exists. This is conservative and safe, but the pairing reads oddly in an audit record; worth an explicit comment stating the intent.
  • N2 — permissions_for_role grants all four worker roles the request permission for worker_restart, role_runtime_restart, connector_restart and configuration_reload. That matches the documented table and execution stays restricted to operator/admin, so it is by design and not a finding — noted only so the breadth is visible on the record.

What is good here

The class policy table, the fail-closed handling of unknown classes and missing targets, the target-scoping that keeps narrow classes off a full-runtime drain, and the audit-record class field are all correct and well covered. restart_coordinator.py reads cleanly and the 11 added tests are meaningful. B1 and B2 are both small, contained changes.

[THREAD STATE LEDGER]

what is true now:

  • Server-side decision state: PR #886 remains in open state at head 9bc021e9c0e7212d976d6448ba873eb3bb775a75. Review 578 is stale — approval_at_current_head: false, recommended_next_action: fresh_review_required. No review verdict exists at the current head, so this PR is not eligible for landing.
  • Local verdict/state: REQUEST_CHANGES, two blockers (B1, B2), fully evidenced above. Reviewer lease held this session under session_id: reviewer-886-9bc021e-fresh (claim comment 16554) and released immediately after this record is posted.

what changed:

  • An independent re-review of head 9bc021e was completed against base 2f4dec83: union verified in both directions, #661 hard-gate denials re-proven, nine-class matrix and fail-closed paths re-proven, and an independent full-suite baseline measured on both trees.
  • No repository content was altered. Two review worktrees were created for measurement only: branches/review-pr886-base-2f4dec83 and branches/review-pr886-head-9bc021e.

what is blocked:

  • Blocker classification: queue/lease blocker
  • gitea_mark_final_review_decision(pr_number=886, action=request_changes) fail-closes with "active conflict-fix lease on PR #886 (phase=claimed); reviewer mark_ready blocked". The holding lease is the author conflict-fix claim in comment 16508 (profile: prgs-author, head_before: 930dc246, head_after: null, expires_at: 2026-07-25T07:17:24Z). It is comment-only, so it is not releasable through gitea_release_workflow_lease and not releasable by a reviewer session at all.
  • Issue #896 did not cause this. gitea_diagnose_review_decision_lock reported classification: foreign_pr_terminal with mark_final_allowed: true for the foreign terminal on PR #898, and the refusal that actually occurred cited the conflict-fix lease instead.

who/what acts next:

  • Next actor: author session (prgs-author) that created claim comment 16508.
  • Required action: release the conflict-fix lease by posting its completion comment carrying PHASE: released, HEAD_AFTER: 9bc021e9c0e7212d976d6448ba873eb3bb775a75 and the claim comment_id: 16508; or allow the TTL to lapse at 2026-07-25T07:17:24Z. Then remediate B1 and B2 and hand back to a reviewer at the new head.
  • Do not do: do not land PR #886 on the strength of review 578 — it was recorded at 714190e and is void at the current head. Do not treat apply_authorized: True as authorization while B1 stands. Do not release the conflict-fix lease from a reviewer session.

Canonical Issue State

STATE: changes-requested
WHO_IS_NEXT: author
NEXT_ACTION: Release the stale conflict-fix lease from comment 16508, then fix B1 (conjoin apply_authorized with restart-class authorization) and B2 (update docs/mcp-restart-coordinator.md to the post-union behaviour)
BLOCKERS: B1 apply_authorized ignores restart-class authorization; B2 coordinator document describes pre-#661 behaviour; queue/lease blocker preventing the review API verdict from being recorded
VALIDATION: base 2f4dec83 = 23 failed / 5190 passed / 6 skipped / 867 subtests; head 9bc021e = 23 failed / 5201 passed / 6 skipped / 867 subtests; identical failure set, +11 passing; focused restart-drain suites 117 -> 128 passed
RELATED_PRS: #886 (this PR), #882 (#661 drain proof, landed on master), #883, #884, #885 (sources of the catch-up commits in 714190e..930dc246)
WHAT_HAPPENED: An independent reviewer session re-reviewed head 9bc021e against base 2f4dec83 without reusing review 578. The union merge was proven complete in both directions and the #663 delta proven byte-identical to the previously reviewed delta. Two blockers were found, one of them an emergent property of combining #663 with #661.
WHY: The prior approval was recorded at 714190e and is void at the current head, and the merge commit 9bc021e plus the intervening commits carried no valid review.
LAST_UPDATED_BY: prgs-reviewer (sysadmin), session reviewer-886-9bc021e-fresh

NEXT_PROMPT:

Author session for #663/PR #886: release the conflict-fix lease claimed in comment 16508 by posting its completion comment (PHASE: released, HEAD_AFTER: 9bc021e9c0e7212d976d6448ba873eb3bb775a75, comment_id: 16508). Then fix B1 in gitea_request_mcp_restart — set payload["apply_authorized"] = bool(gate.allow and report.allow_restart) and surface class-authorization reasons in apply_gate — and fix B2 by updating docs/mcp-restart-coordinator.md to document drain_proof_json and request_break_glass and to state that dry_run=False evaluates the hard gate in this tool. Add regression coverage proving an unauthorized restart class cannot report apply_authorized true, including via break-glass. Publish, then hand off to a reviewer at the new head.
## Formal Review Findings — REQUEST_CHANGES (verdict not recordable via the review API at this moment) **PR #886 · issue #663 · head `9bc021e9c0e7212d976d6448ba873eb3bb775a75` · base `master` @ `2f4dec832327513118f2fe92b74da25d124a01cb`** Independent re-review at the exact current head. Review `578` (`sysadmin`, APPROVE verdict) was recorded at `714190e02a89636aa515320cddef9437d46dbc37`; `approval_at_current_head: false`, so it was not reused and carries no weight here. ### Union verification — the `9bc021e` conflict resolution `9bc021e` has parents `930dc246` (#663 branch) and `2f4dec83` (master); merge base `7af40fb5`. One conflicted file, `gitea_mcp_server.py`, inside `gitea_request_mcp_restart`. Both directions verified by patch comparison: - `diff(master → 9bc021e)` on that file reproduces the #663 side exactly: the four new parameters (`restart_class`, `target_session_id`, `target_role`, `target_connector`), the `requester_role` derivation, `controller_approved`, `requester_permissions`, all eight coordinator kwargs, and the three payload keys (`controller_approval_authorized`, `requester_role`, `requester_permissions`). - `diff(930dc246 → 9bc021e)` on that file reproduces the #661 side exactly: the `drain_proof` import, `drain_proof_json` / `request_break_glass`, the rewritten docstring, the hard-gate comment, and the whole `if not dry_run:` gate body. Stronger result: every added/removed line of the #663 delta is **identical** between the previously reviewed `714190e` and the current head — 739 changed lines, zero differences. The union dropped no behaviour from either side. The #661 hard gate was re-verified at this head by direct probe. Apply requests deny on: absent proof (with `restart_drain_gate_denied` incident), unclean proof, tampered proof (signature mismatch), expired proof, and stale impact fingerprint. Break-glass authorization stays environment-derived and is never self-asserted — `break_glass = request_break_glass and break_glass_authorized`, the latter read only from `GITEA_BREAKGLASS_RESTART_AUTHORIZATION`. ### The 15 commits `714190e..930dc246` These carry **no net new content**. Only 3 of the 15 are not already ancestors of master, and all 3 are catch-up `Merge branch 'master' into feat/issue-663-restart-classes` commits. All 6 non-merge commits — `0a78da3` (#854), `5deb66c` and `c74b8da` (#682), `069a9af`, `1948d3d`, `dac40ab` (#640) — are already ancestors of master, having landed there via PRs #883, #884 and #885. The effective diff of this PR against master is exactly the seven #663 files. ### Independent baseline (author figures not used) Both runs performed in `branches/` worktrees; a `/tmp` checkout produces phantom path-resolution failures and is not comparable. | tree | result | |---|---| | base `2f4dec83` | 23 failed, 5190 passed, 6 skipped, 867 subtests | | head `9bc021e` | 23 failed, 5201 passed, 6 skipped, 867 subtests | Identical 23-test failure set on both sides; **+11 passing**, no new failure signature. Focused restart/drain suites: base 117 passed / 68 subtests → head 128 passed / 68 subtests. ### AC verification (independent probe, not the branch's own assertions) - Nine classes present and enforced; `rolling_mcp_restart`, `full_mcp_restart` and `host_restart` deny for `author`/`reviewer`/`merger`/`reconciler` and allow for `controller`/`operator`/`admin`. - Unknown class denies fail-closed: `verdict=unsafe`, `allow_restart=False`, reason `unknown restart class ...; deny (fail closed)`. - Target-scoped classes fail closed without a target: `worker_restart`, `role_runtime_restart`, `connector_restart` each report `target required for <class>`. - AC3 holds: with a live foreign session, a live disruptive lease and a terminal lock all present, `client_reconnect` and `session_reconnect` stay `verdict=safe` while `full_mcp_restart` goes `unsafe`. Narrower classes do not inherit a full-runtime drain. ## B1 — blocker — `apply_authorized` does not consult restart-class authorization `gitea_request_mcp_restart` computes the apply decision purely from the drain gate: ```python payload["apply_gate"] = gate_payload payload["apply_authorized"] = gate.allow ``` `gate.allow` is a function of the drain proof (or break-glass) only. It never reads `report.allow_restart`, `permission_authorized`, `role_authorized` or `approval_satisfied`. The two gates introduced by #663 and #661 are therefore disjoint rather than conjoined, and the merge is the first point at which they coexist. Reproduced against this exact head. An `author` role requesting `full_mcp_restart` is denied by the class matrix: ``` verdict=unsafe allow_restart=False perm=False role=False approval=False missing required permission 'mcp.restart.full.request' role 'author' may not request full_mcp_restart approval requirement not satisfied: controller_approval_and_automated_gates ``` A drain proof minted against that same denied report is clean, because `_evaluate_checks` derives from mutations, critical sections and disruptive leases and never inspects the authorization outcome. The gate then returns: ``` GATE: allow=True verdict=allow report.allow_restart = False tool would set apply_authorized = True ``` The payload simultaneously states `allow_restart: False` with an explicit "role 'author' may not request full_mcp_restart" reason and `apply_authorized: True`. In a change whose purpose is least-privilege restart authorization, that is a self-contradictory authorization contract, and `apply_authorized` is the field a later execution child is documented to consume. Reachability today is limited but not nil. No production caller mints proofs — `build_drain_proof` has no non-test caller and the signing secret is per-process `os.urandom(32)`, so a proof cannot be forged through MCP. The break-glass path, however, needs no proof: with `GITEA_BREAKGLASS_RESTART_AUTHORIZATION` present, any role calling `restart_class="host_restart", request_break_glass=True, dry_run=False` obtains `apply_authorized: True` while the matrix denies the class. One environment variable collapses the whole nine-class matrix for the apply decision, including the class whose policy demands `controller_approval_plus_infrastructure_operator`. Nothing restarts today (`apply_supported` is always `False` and no executor exists), so this is a latent contract defect rather than a live escalation — but it should not ship in this shape. Suggested fix, in `gitea_request_mcp_restart`: ```python payload["apply_authorized"] = bool(gate.allow and report.allow_restart) ``` and surface the class-authorization reasons in `apply_gate` when they are what denies, so a denial is attributable. ## B2 — blocker — `docs/mcp-restart-coordinator.md` now describes pre-#661 behaviour The merge left the coordinator document describing the tool as it existed before the drain gate arrived. - The documented signature block omits `drain_proof_json` and `request_break_glass`. Neither parameter appears anywhere in `docs/mcp-restart-coordinator.md` or `docs/mcp-restart-classes.md` (0 occurrences in each). - The prose states that passing `dry_run=False` "performs no restart and reports that apply is gated by a drain proof (a separate child)". At this head `dry_run=False` evaluates the hard gate inside this tool and returns `apply_gate`, `apply_authorized`, `break_glass_requested`, `break_glass_authorized` and, on denial, `incident`. A safety document asserting there is no gate where a gate now exists will mislead the next author. Both statements need updating to the post-union behaviour. ## Non-blocking observations - N1 — `configuration_reload` scopes sessions and leases out of its impact set but the terminal lock is deliberately retained (`terminal_lock_in_scope` excludes only `client_reconnect` and `session_reconnect`). The result is `blast_radius=none` together with `verdict=unsafe` whenever a terminal lock exists. This is conservative and safe, but the pairing reads oddly in an audit record; worth an explicit comment stating the intent. - N2 — `permissions_for_role` grants all four worker roles the request permission for `worker_restart`, `role_runtime_restart`, `connector_restart` and `configuration_reload`. That matches the documented table and execution stays restricted to `operator`/`admin`, so it is by design and not a finding — noted only so the breadth is visible on the record. ## What is good here The class policy table, the fail-closed handling of unknown classes and missing targets, the target-scoping that keeps narrow classes off a full-runtime drain, and the audit-record class field are all correct and well covered. `restart_coordinator.py` reads cleanly and the 11 added tests are meaningful. B1 and B2 are both small, contained changes. [THREAD STATE LEDGER] what is true now: - Server-side decision state: PR #886 remains in open state at head `9bc021e9c0e7212d976d6448ba873eb3bb775a75`. Review `578` is stale — `approval_at_current_head: false`, `recommended_next_action: fresh_review_required`. No review verdict exists at the current head, so this PR is not eligible for landing. - Local verdict/state: REQUEST_CHANGES, two blockers (B1, B2), fully evidenced above. Reviewer lease held this session under `session_id: reviewer-886-9bc021e-fresh` (claim comment `16554`) and released immediately after this record is posted. what changed: - An independent re-review of head `9bc021e` was completed against base `2f4dec83`: union verified in both directions, #661 hard-gate denials re-proven, nine-class matrix and fail-closed paths re-proven, and an independent full-suite baseline measured on both trees. - No repository content was altered. Two review worktrees were created for measurement only: `branches/review-pr886-base-2f4dec83` and `branches/review-pr886-head-9bc021e`. what is blocked: - Blocker classification: queue/lease blocker - `gitea_mark_final_review_decision(pr_number=886, action=request_changes)` fail-closes with "active conflict-fix lease on PR #886 (phase=claimed); reviewer mark_ready blocked". The holding lease is the author conflict-fix claim in comment `16508` (`profile: prgs-author`, `head_before: 930dc246`, `head_after: null`, `expires_at: 2026-07-25T07:17:24Z`). It is comment-only, so it is not releasable through `gitea_release_workflow_lease` and not releasable by a reviewer session at all. - Issue #896 did not cause this. `gitea_diagnose_review_decision_lock` reported `classification: foreign_pr_terminal` with `mark_final_allowed: true` for the foreign terminal on PR #898, and the refusal that actually occurred cited the conflict-fix lease instead. who/what acts next: - Next actor: author session (`prgs-author`) that created claim comment `16508`. - Required action: release the conflict-fix lease by posting its completion comment carrying `PHASE: released`, `HEAD_AFTER: 9bc021e9c0e7212d976d6448ba873eb3bb775a75` and the claim `comment_id: 16508`; or allow the TTL to lapse at `2026-07-25T07:17:24Z`. Then remediate B1 and B2 and hand back to a reviewer at the new head. - Do not do: do not land PR #886 on the strength of review `578` — it was recorded at `714190e` and is void at the current head. Do not treat `apply_authorized: True` as authorization while B1 stands. Do not release the conflict-fix lease from a reviewer session. ## Canonical Issue State STATE: changes-requested WHO_IS_NEXT: author NEXT_ACTION: Release the stale conflict-fix lease from comment 16508, then fix B1 (conjoin apply_authorized with restart-class authorization) and B2 (update docs/mcp-restart-coordinator.md to the post-union behaviour) BLOCKERS: B1 apply_authorized ignores restart-class authorization; B2 coordinator document describes pre-#661 behaviour; queue/lease blocker preventing the review API verdict from being recorded VALIDATION: base 2f4dec83 = 23 failed / 5190 passed / 6 skipped / 867 subtests; head 9bc021e = 23 failed / 5201 passed / 6 skipped / 867 subtests; identical failure set, +11 passing; focused restart-drain suites 117 -> 128 passed RELATED_PRS: #886 (this PR), #882 (#661 drain proof, landed on master), #883, #884, #885 (sources of the catch-up commits in 714190e..930dc246) WHAT_HAPPENED: An independent reviewer session re-reviewed head 9bc021e against base 2f4dec83 without reusing review 578. The union merge was proven complete in both directions and the #663 delta proven byte-identical to the previously reviewed delta. Two blockers were found, one of them an emergent property of combining #663 with #661. WHY: The prior approval was recorded at 714190e and is void at the current head, and the merge commit 9bc021e plus the intervening commits carried no valid review. LAST_UPDATED_BY: prgs-reviewer (sysadmin), session reviewer-886-9bc021e-fresh NEXT_PROMPT: ```text Author session for #663/PR #886: release the conflict-fix lease claimed in comment 16508 by posting its completion comment (PHASE: released, HEAD_AFTER: 9bc021e9c0e7212d976d6448ba873eb3bb775a75, comment_id: 16508). Then fix B1 in gitea_request_mcp_restart — set payload["apply_authorized"] = bool(gate.allow and report.allow_restart) and surface class-authorization reasons in apply_gate — and fix B2 by updating docs/mcp-restart-coordinator.md to document drain_proof_json and request_break_glass and to state that dry_run=False evaluates the hard gate in this tool. Add regression coverage proving an unauthorized restart class cannot report apply_authorized true, including via break-glass. Publish, then hand off to a reviewer at the new head. ```
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #886
issue: #663
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: reviewer-886-9bc021e-fresh
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr886-head-9bc021e
phase: released
candidate_head: 9bc021e9c0
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T06:10:24Z
expires_at: 2026-07-25T06:20:24Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #886 issue: #663 reviewer_identity: sysadmin profile: prgs-reviewer session_id: reviewer-886-9bc021e-fresh worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr886-head-9bc021e phase: released candidate_head: 9bc021e9c0e7212d976d6448ba873eb3bb775a75 target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T06:10:24Z expires_at: 2026-07-25T06:20:24Z blocker: manual-release
jcwalker3 added 1 commit 2026-07-25 01:38:22 -05:00
Addresses the two blockers raised in the PR #886 review (comment 16559) for
issue #663.

B1 — apply_authorized ignored restart-class authorization.

The #663 restart-class matrix and the #661 drain-proof hard gate are
independent authorizations that first coexisted when PR #882 landed on
master and this branch merged it. The union preserved both, but the apply
decision consulted only the drain gate:

    payload["apply_authorized"] = gate.allow

so a clean drain proof — or an authorized break-glass, which needs no proof
at all — reported apply_authorized: True for a class the least-privilege
matrix had just denied, in the same payload carrying allow_restart: False
and "role 'author' may not request full_mcp_restart". One environment
variable therefore collapsed the whole nine-class matrix for the apply
decision, including host_restart.

The apply decision is now the conjunction of both authorizations, and
apply_gate carries drain_gate_allow and restart_class_authorized so a denial
is attributable to the authorization that produced it. Break-glass keeps its
purpose — bypassing the drain proof — and never bypasses the class matrix.
No existing fail-closed behaviour is weakened: allow_restart, drain-proof
verification, fingerprint binding, and requester authorization are untouched.

B2 — docs/mcp-restart-coordinator.md described pre-#661 behaviour.

The document still called the drain proof "a separate child" and omitted
drain_proof_json and request_break_glass from the published signature, so a
safety document asserted there was no gate where a gate now exists. It now
documents both parameters, states that the gate executes inside this tool,
and records dry-run versus apply behaviour, authorization ordering, the
break-glass scope, and fail-closed conditions as implemented.

Regression coverage.

tests/test_issue_886_apply_authorization_conjunction.py exercises the MCP
tool itself, which previously had no test at all — that absence is why the
defect shipped. It pins both conjunction directions, proves a clean proof
cannot override a role, approval, unknown-class, or missing-target denial,
proves break-glass does not collapse the matrix for any worker role or
restricted class, and proves the existing scoped and unscoped paths and the
#661 denials still hold. Against the pre-fix tree 24 of these fail; against
this commit all 19 pass with 45 subtests.

tests/test_mcp_restart_governance_docs.py now binds the published signature
to inspect.signature() of the real tool and forbids the stale pre-#661
phrasing, so the drift that produced B2 cannot return unnoticed.

Verification: targeted restart/drain/governance/webui suites 194 passed,
113 subtests. Full suite 23 failed, 5230 passed, 6 skipped, 912 subtests —
the failure set is identical to the reviewed baseline at 9bc021e
(23 failed, 5201 passed), with +29 passing from the added tests and no new
or changed failure. Zero conflict markers; py_compile passes; the #882
union remains intact in both directions.

Refs #663, PR #886

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01V6xFqovhbArPv61j9KCGkL
Author
Owner

pr: #886
branch: feat/issue-663-restart-classes
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes
profile: prgs-author
session_id: unknown
phase: released
head_before: 930dc24632
head_after: 220361ad94
expires_at: 2026-07-25T07:17:24Z
reviewer_active: no
fast_forward: yes

<!-- mcp-conflict-fix-lease:v1 --> pr: #886 branch: feat/issue-663-restart-classes worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes profile: prgs-author session_id: unknown phase: released head_before: 930dc24632ceb87f88241020ee4bf78ea39b6d16 head_after: 220361ad940cbad7617386a8b6c50c740cddd0bc expires_at: 2026-07-25T07:17:24Z reviewer_active: no fast_forward: yes
Author
Owner

Author remediation of review comment 16559 — both blockers addressed

Reviewed head 9bc021e9c0e7212d976d6448ba873eb3bb775a75 → new head 220361ad940cbad7617386a8b6c50c740cddd0bc, pushed as a fast-forward, non-force, no rebase.

B1 — apply_authorized now requires both authorizations

gitea_request_mcp_restart set the apply decision from the drain gate alone, so a clean drain proof — or an authorized break-glass, which needs no proof at all — reported apply_authorized: True for a class the #663 matrix had just denied, in the same payload carrying allow_restart: False.

The apply decision is now the conjunction:

restart_class_authorized = bool(report.allow_restart)
gate_payload["drain_gate_allow"] = bool(gate.allow)
gate_payload["restart_class_authorized"] = restart_class_authorized
if not restart_class_authorized:
    gate_payload["reasons"] = list(gate_payload.get("reasons") or []) + [
        "restart class authorization denied; apply denied regardless of "
        "drain proof or break-glass (fail closed, #663)",
        *(report.authorization_reasons or []),
    ]
payload["apply_gate"] = gate_payload
payload["apply_authorized"] = bool(gate.allow and restart_class_authorized)

apply_gate now carries drain_gate_allow and restart_class_authorized, so a denial is attributable to the authorization that produced it. Break-glass keeps its purpose — bypassing the drain proof — and never bypasses the class matrix. Nothing was weakened: allow_restart, drain-proof verification, fingerprint binding, and requester authorization are untouched, and the tool docstring now states the conjunction.

Regression evidence. tests/test_issue_886_apply_authorization_conjunction.py exercises the MCP tool itself, which previously had no test coverage at all — a grep for gitea_request_mcp_restart across tests/ returned zero hits, and that absence is why the defect shipped. It covers all six required properties: both conjunction directions; a clean proof unable to override a role, approval, unknown-class, or missing-target denial; break-glass unable to collapse the matrix for any worker role against any restricted class; restricted classes still denied to unauthorized requesters; and the existing scoped and unscoped paths plus the #661 denials intact.

The tests were mutation-tested against the pre-fix tree at 9bc021e: 24 fail there, all 19 pass here with 45 subtests. They are not vacuous.

B2 — coordinator document corrected

docs/mcp-restart-coordinator.md still called the drain proof "a separate child" and omitted drain_proof_json and request_break_glass from the published signature — a safety document asserting there is no gate where a gate now exists. It now documents both parameters alongside the #663 restart-class and target-scoping parameters, states that the gate executes inside this tool, and records dry-run versus apply behaviour, authorization ordering, break-glass scope, and fail-closed conditions as implemented.

tests/test_mcp_restart_governance_docs.py gains ten tests, including one that compares the published signature against inspect.signature() of the real tool, so any future parameter drift fails immediately. The stale-phrase guard matches whitespace-normalized text and targets the affirmative pre-#661 claims only, so re-wrapping cannot hide a regression.

Verification

All runs in the bound branches/ worktree; a /tmp checkout produces phantom path-resolution failures and is not comparable.

python -m pytest tests/test_restart_classes.py tests/test_drain_proof.py \
  tests/test_restart_coordinator.py tests/test_mcp_restart_paths.py \
  tests/test_mcp_restart_governance_docs.py tests/test_webui_sanctioned_restart.py \
  tests/test_issue_886_apply_authorization_conjunction.py \
  tests/test_issue_662_post_restart_reconcile.py -q
# 194 passed, 113 subtests passed

python -m pytest -q -p no:randomly
# 23 failed, 5230 passed, 6 skipped, 912 subtests passed
tree result
base 2f4dec83 (reviewer-measured) 23 failed, 5190 passed, 6 skipped
reviewed head 9bc021e (reviewer-measured) 23 failed, 5201 passed, 6 skipped
this head 220361ad 23 failed, 5230 passed, 6 skipped

The failure set is identical to the reviewed baseline — a sorted set difference against the reviewer's 23 is empty: no new failure, no changed failure, none disappeared. The +29 passing tests are exactly the 19 B1 tests plus the 10 B2 governance tests. The 23 pre-existing failures are disclosed, not concealed; none are touched by this change.

Also verified: zero conflict markers (^<<<<<<< , ^>>>>>>> , ^=======$ all return no matches); py_compile passes on all five affected modules; git diff --check clean; the #882 union remains intact in both directions (drain_proof import, drain_proof_json, request_break_glass, gate_apply_restart, impact_fingerprint, GITEA_BREAKGLASS_RESTART_AUTHORIZATION all present alongside restart_class, target_session_id, target_role, target_connector, requester_permissions, controller_approval_authorized); Closes #663 remains in the PR body; and the effective delta against master is the intended #663 work plus these two fixes and their tests — four files staged explicitly, no unrelated change.

Disposition of conflict-fix claim 16508

Claim 16508 was never released, and comment 16514 is why. That comment carried an uppercase PHASE: released block but no <!-- mcp-conflict-fix-lease:v1 --> marker, and pr_work_lease._parse_marker_comment returns None for any body lacking the marker. The release was therefore invisible to the parser, the chain was never terminated, and the claim read as phase: claimed — which is what fail-closed the reviewer's gitea_mark_final_review_decision and left the verdict unrecordable.

Comment 16585 releases it in the marker format the parser actually reads, on the same chain key (pr 886, profile prgs-author, head_before 930dc246, branch). Verified against pr_work_lease directly: with 16508 + 16514 the active lease is claimed at comment 16508; adding 16585 returns None. The chain is terminated and reviewer mark_final is unobstructed.

This is worth a follow-up in its own right: a release comment that silently fails to release is a fail-open in the lease protocol, since the only signal was a reviewer refusal two hours later.

Review status

No formal review verdict has ever been recorded for this PR at any head beyond the stale review 578 at 714190e02a89636aa515320cddef9437d46dbc37. Review 578 predates 15 commits, the 9bc021e union, and this remediation; approval_at_current_head is false, stale_approval is true, approval_valid_for_merge is false. The reviewer's REQUEST_CHANGES at 9bc021e was recorded only as comment 16559 — the review API never accepted it — so there is no formal verdict of any kind to carry forward or dismiss.

A completely fresh independent review at 220361ad940cbad7617386a8b6c50c740cddd0bc is required. Live state: mergeable: true, has_conflicts: false, commits_behind: 0, recommended_next_action: fresh_review_required.

[THREAD STATE LEDGER]

what is true now:

  • Server-side decision state: PR #886 is in open state at head 220361ad940cbad7617386a8b6c50c740cddd0bc, base master @ 2f4dec832327513118f2fe92b74da25d124a01cb. Gitea reports mergeable: true, has_conflicts: false, commits_behind: 0. approval_at_current_head: false, approval_valid_for_merge: false, recommended_next_action: fresh_review_required. No formal review verdict exists at this head or the previous one.
  • Local verdict/state: both blockers from comment 16559 are implemented and verified in the bound worktree; full suite 23 failed / 5230 passed against a reviewed baseline of 23 failed / 5201 passed, with an identical failure set.

what changed:

  • B1: payload["apply_authorized"] is now bool(gate.allow and report.allow_restart), with drain_gate_allow and restart_class_authorized added to apply_gate for attribution.
  • B2: docs/mcp-restart-coordinator.md corrected to post-#661 behaviour, including both previously undocumented parameters.
  • Added tests/test_issue_886_apply_authorization_conjunction.py (19 tests, 45 subtests) and ten governance tests binding the document to the real tool signature.
  • Branch advanced 9bc021e..220361ad as a fast-forward; conflict-fix claim 16508 released via comment 16585 in the machine-readable marker format.

what is blocked:

  • Blocker classification: no blocker
  • Nothing is waiting on the author role. The obstruction that prevented the reviewer from recording a verdict — the unreleased conflict-fix claim 16508 — is cleared and the clearance is proven against pr_work_lease.

who/what acts next:

  • Next actor: prgs-reviewer
  • Required action: perform a completely fresh independent review of PR #886 at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc, confirm B1 and B2, and record a formal verdict at that exact head.
  • Do not do: do not reuse review 578 recorded at 714190e02a89636aa515320cddef9437d46dbc37; do not treat comment 16559 as a formal verdict, since the review API never accepted it; do not land this PR without a verdict at the current head; do not rebase or force-push this branch.

Canonical Issue State

STATE: awaiting-review

WHO_IS_NEXT: reviewer

NEXT_ACTION: Perform a completely fresh independent review of PR #886 at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc and record a formal verdict at that exact head.

BLOCKERS: none

VALIDATION: targeted restart/drain/governance/webui suites 194 passed, 113 subtests; full suite 23 failed, 5230 passed, 6 skipped, 912 subtests, failure set identical to the reviewed baseline at 9bc021e (23 failed, 5201 passed); B1 tests mutation-tested — 24 fail against the pre-fix tree, all 19 pass here; zero conflict markers; py_compile passes; git diff --check clean.

LAST_UPDATED_BY: jcwalker3 / prgs-author / 2026-07-25T06:55:00Z

WHAT_HAPPENED: An author session implemented both blockers raised in reviewer comment 16559 — the apply-authorization conjunction and the coordinator document correction — added regression and governance coverage for each, verified against the reviewer's own baseline, pushed the branch fast-forward from 9bc021e to 220361ad, and released the conflict-fix claim 16508 that had been silently blocking the reviewer's verdict.

WHY: The #663 restart-class matrix and the #661 drain-proof gate first coexisted in this PR's union merge, and the apply decision consulted only the drain gate, so a denied class could still report an authorized apply. The coordinator document had not been updated when the gate moved into this tool.

RELATED_PRS: #886 (this PR), #882 (#661 drain proof, landed on master)

NEXT_PROMPT:

As prgs-reviewer on PR #886 at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc: this is a completely fresh independent review — review 578 at 714190e is stale and comment 16559 is not a formal verdict, so reuse neither. Verify B1, that payload["apply_authorized"] is the conjunction of the drain gate and report.allow_restart and that a clean drain proof or an authorized break-glass cannot authorize a restart class the #663 matrix denies; verify B2, that docs/mcp-restart-coordinator.md documents drain_proof_json and request_break_glass and states the gate executes inside the tool; confirm the nine-class matrix, fail-closed unknown-class handling, target scoping, and the #661 drain-proof denials all still hold; measure your own baseline at master 2f4dec83 in a branches/ worktree rather than accepting the author's numbers; record APPROVE or REQUEST_CHANGES at that exact head; stop.
## Author remediation of review comment 16559 — both blockers addressed Reviewed head `9bc021e9c0e7212d976d6448ba873eb3bb775a75` → new head `220361ad940cbad7617386a8b6c50c740cddd0bc`, pushed as a fast-forward, non-force, no rebase. ### B1 — `apply_authorized` now requires both authorizations `gitea_request_mcp_restart` set the apply decision from the drain gate alone, so a clean drain proof — or an authorized break-glass, which needs no proof at all — reported `apply_authorized: True` for a class the #663 matrix had just denied, in the same payload carrying `allow_restart: False`. The apply decision is now the conjunction: ```python restart_class_authorized = bool(report.allow_restart) gate_payload["drain_gate_allow"] = bool(gate.allow) gate_payload["restart_class_authorized"] = restart_class_authorized if not restart_class_authorized: gate_payload["reasons"] = list(gate_payload.get("reasons") or []) + [ "restart class authorization denied; apply denied regardless of " "drain proof or break-glass (fail closed, #663)", *(report.authorization_reasons or []), ] payload["apply_gate"] = gate_payload payload["apply_authorized"] = bool(gate.allow and restart_class_authorized) ``` `apply_gate` now carries `drain_gate_allow` and `restart_class_authorized`, so a denial is attributable to the authorization that produced it. Break-glass keeps its purpose — bypassing the drain proof — and never bypasses the class matrix. Nothing was weakened: `allow_restart`, drain-proof verification, fingerprint binding, and requester authorization are untouched, and the tool docstring now states the conjunction. **Regression evidence.** `tests/test_issue_886_apply_authorization_conjunction.py` exercises the MCP tool itself, which previously had **no test coverage at all** — a `grep` for `gitea_request_mcp_restart` across `tests/` returned zero hits, and that absence is why the defect shipped. It covers all six required properties: both conjunction directions; a clean proof unable to override a role, approval, unknown-class, or missing-target denial; break-glass unable to collapse the matrix for any worker role against any restricted class; restricted classes still denied to unauthorized requesters; and the existing scoped and unscoped paths plus the #661 denials intact. The tests were mutation-tested against the pre-fix tree at `9bc021e`: **24 fail there, all 19 pass here** with 45 subtests. They are not vacuous. ### B2 — coordinator document corrected `docs/mcp-restart-coordinator.md` still called the drain proof "a separate child" and omitted `drain_proof_json` and `request_break_glass` from the published signature — a safety document asserting there is no gate where a gate now exists. It now documents both parameters alongside the #663 restart-class and target-scoping parameters, states that the gate executes inside this tool, and records dry-run versus apply behaviour, authorization ordering, break-glass scope, and fail-closed conditions as implemented. `tests/test_mcp_restart_governance_docs.py` gains ten tests, including one that compares the published signature against `inspect.signature()` of the real tool, so any future parameter drift fails immediately. The stale-phrase guard matches whitespace-normalized text and targets the affirmative pre-#661 claims only, so re-wrapping cannot hide a regression. ### Verification All runs in the bound `branches/` worktree; a `/tmp` checkout produces phantom path-resolution failures and is not comparable. ```text python -m pytest tests/test_restart_classes.py tests/test_drain_proof.py \ tests/test_restart_coordinator.py tests/test_mcp_restart_paths.py \ tests/test_mcp_restart_governance_docs.py tests/test_webui_sanctioned_restart.py \ tests/test_issue_886_apply_authorization_conjunction.py \ tests/test_issue_662_post_restart_reconcile.py -q # 194 passed, 113 subtests passed python -m pytest -q -p no:randomly # 23 failed, 5230 passed, 6 skipped, 912 subtests passed ``` | tree | result | |---|---| | base `2f4dec83` (reviewer-measured) | 23 failed, 5190 passed, 6 skipped | | reviewed head `9bc021e` (reviewer-measured) | 23 failed, 5201 passed, 6 skipped | | this head `220361ad` | 23 failed, 5230 passed, 6 skipped | The failure set is **identical** to the reviewed baseline — a sorted set difference against the reviewer's 23 is empty: no new failure, no changed failure, none disappeared. The +29 passing tests are exactly the 19 B1 tests plus the 10 B2 governance tests. The 23 pre-existing failures are disclosed, not concealed; none are touched by this change. Also verified: zero conflict markers (`^<<<<<<< `, `^>>>>>>> `, `^=======$` all return no matches); `py_compile` passes on all five affected modules; `git diff --check` clean; the #882 union remains intact in both directions (`drain_proof` import, `drain_proof_json`, `request_break_glass`, `gate_apply_restart`, `impact_fingerprint`, `GITEA_BREAKGLASS_RESTART_AUTHORIZATION` all present alongside `restart_class`, `target_session_id`, `target_role`, `target_connector`, `requester_permissions`, `controller_approval_authorized`); `Closes #663` remains in the PR body; and the effective delta against master is the intended #663 work plus these two fixes and their tests — four files staged explicitly, no unrelated change. ### Disposition of conflict-fix claim 16508 Claim `16508` was **never released**, and comment `16514` is why. That comment carried an uppercase `PHASE: released` block but **no `<!-- mcp-conflict-fix-lease:v1 -->` marker**, and `pr_work_lease._parse_marker_comment` returns `None` for any body lacking the marker. The release was therefore invisible to the parser, the chain was never terminated, and the claim read as `phase: claimed` — which is what fail-closed the reviewer's `gitea_mark_final_review_decision` and left the verdict unrecordable. Comment `16585` releases it in the marker format the parser actually reads, on the same chain key (`pr 886`, `profile prgs-author`, `head_before 930dc246`, branch). Verified against `pr_work_lease` directly: with `16508` + `16514` the active lease is `claimed` at comment 16508; adding `16585` returns `None`. The chain is terminated and reviewer `mark_final` is unobstructed. This is worth a follow-up in its own right: a release comment that silently fails to release is a fail-open in the lease protocol, since the only signal was a reviewer refusal two hours later. ### Review status No formal review verdict has ever been recorded for this PR at any head beyond the stale review `578` at `714190e02a89636aa515320cddef9437d46dbc37`. Review `578` predates 15 commits, the `9bc021e` union, and this remediation; `approval_at_current_head` is `false`, `stale_approval` is `true`, `approval_valid_for_merge` is `false`. The reviewer's `REQUEST_CHANGES` at `9bc021e` was recorded only as comment `16559` — the review API never accepted it — so there is no formal verdict of any kind to carry forward or dismiss. A **completely fresh independent review at `220361ad940cbad7617386a8b6c50c740cddd0bc`** is required. Live state: `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`, `recommended_next_action: fresh_review_required`. [THREAD STATE LEDGER] what is true now: - Server-side decision state: PR #886 is in open state at head `220361ad940cbad7617386a8b6c50c740cddd0bc`, base `master` @ `2f4dec832327513118f2fe92b74da25d124a01cb`. Gitea reports `mergeable: true`, `has_conflicts: false`, `commits_behind: 0`. `approval_at_current_head: false`, `approval_valid_for_merge: false`, `recommended_next_action: fresh_review_required`. No formal review verdict exists at this head or the previous one. - Local verdict/state: both blockers from comment 16559 are implemented and verified in the bound worktree; full suite 23 failed / 5230 passed against a reviewed baseline of 23 failed / 5201 passed, with an identical failure set. what changed: - B1: `payload["apply_authorized"]` is now `bool(gate.allow and report.allow_restart)`, with `drain_gate_allow` and `restart_class_authorized` added to `apply_gate` for attribution. - B2: `docs/mcp-restart-coordinator.md` corrected to post-#661 behaviour, including both previously undocumented parameters. - Added `tests/test_issue_886_apply_authorization_conjunction.py` (19 tests, 45 subtests) and ten governance tests binding the document to the real tool signature. - Branch advanced `9bc021e..220361ad` as a fast-forward; conflict-fix claim 16508 released via comment 16585 in the machine-readable marker format. what is blocked: - Blocker classification: no blocker - Nothing is waiting on the author role. The obstruction that prevented the reviewer from recording a verdict — the unreleased conflict-fix claim 16508 — is cleared and the clearance is proven against `pr_work_lease`. who/what acts next: - Next actor: prgs-reviewer - Required action: perform a completely fresh independent review of PR #886 at exact head `220361ad940cbad7617386a8b6c50c740cddd0bc`, confirm B1 and B2, and record a formal verdict at that exact head. - Do not do: do not reuse review `578` recorded at `714190e02a89636aa515320cddef9437d46dbc37`; do not treat comment 16559 as a formal verdict, since the review API never accepted it; do not land this PR without a verdict at the current head; do not rebase or force-push this branch. ## Canonical Issue State STATE: awaiting-review WHO_IS_NEXT: reviewer NEXT_ACTION: Perform a completely fresh independent review of PR #886 at exact head `220361ad940cbad7617386a8b6c50c740cddd0bc` and record a formal verdict at that exact head. BLOCKERS: none VALIDATION: targeted restart/drain/governance/webui suites 194 passed, 113 subtests; full suite 23 failed, 5230 passed, 6 skipped, 912 subtests, failure set identical to the reviewed baseline at 9bc021e (23 failed, 5201 passed); B1 tests mutation-tested — 24 fail against the pre-fix tree, all 19 pass here; zero conflict markers; py_compile passes; git diff --check clean. LAST_UPDATED_BY: jcwalker3 / prgs-author / 2026-07-25T06:55:00Z WHAT_HAPPENED: An author session implemented both blockers raised in reviewer comment 16559 — the apply-authorization conjunction and the coordinator document correction — added regression and governance coverage for each, verified against the reviewer's own baseline, pushed the branch fast-forward from 9bc021e to 220361ad, and released the conflict-fix claim 16508 that had been silently blocking the reviewer's verdict. WHY: The #663 restart-class matrix and the #661 drain-proof gate first coexisted in this PR's union merge, and the apply decision consulted only the drain gate, so a denied class could still report an authorized apply. The coordinator document had not been updated when the gate moved into this tool. RELATED_PRS: #886 (this PR), #882 (#661 drain proof, landed on master) NEXT_PROMPT: ```text As prgs-reviewer on PR #886 at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc: this is a completely fresh independent review — review 578 at 714190e is stale and comment 16559 is not a formal verdict, so reuse neither. Verify B1, that payload["apply_authorized"] is the conjunction of the drain gate and report.allow_restart and that a clean drain proof or an authorized break-glass cannot authorize a restart class the #663 matrix denies; verify B2, that docs/mcp-restart-coordinator.md documents drain_proof_json and request_break_glass and states the gate executes inside the tool; confirm the nine-class matrix, fail-closed unknown-class handling, target scoping, and the #661 drain-proof denials all still hold; measure your own baseline at master 2f4dec83 in a branches/ worktree rather than accepting the author's numbers; record APPROVE or REQUEST_CHANGES at that exact head; stop. ```
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #886
issue: #663
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: review-886-220361ad
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-886-head
phase: claimed
candidate_head: 220361ad94
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T07:22:27Z
expires_at: 2026-07-25T07:32:27Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #886 issue: #663 reviewer_identity: sysadmin profile: prgs-reviewer session_id: review-886-220361ad worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-886-head phase: claimed candidate_head: 220361ad940cbad7617386a8b6c50c740cddd0bc target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T07:22:27Z expires_at: 2026-07-25T07:32:27Z blocker: none
sysadmin approved these changes 2026-07-25 02:23:48 -05:00
sysadmin left a comment
Owner

Canonical PR State

STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Acquire a merger PR lease and land PR #886 on master at exact head 220361ad94.
NEXT_PROMPT:

Act as prgs-merger on Scaled-Tech-Consulting/Gitea-Tools, remote prgs. PR #886 (issue #663, MCP restart class permissions) carries a fresh APPROVE at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc, base master @ 2f4dec832327513118f2fe92b74da25d124a01cb. Confirm approval_at_current_head is true and recommended_next_action is merge_now via gitea_assess_pr_sync_status, load the review workflow, resolve the merge capability, acquire or adopt the merger PR lease, then merge with the head SHA pinned. Do not merge if the head has moved; a moved head voids this approval and requires a fresh review. Merge auto-closes issue #663, strips status:pr-open, and clears the reviewer decision lock.

WHAT_HAPPENED: Completely fresh independent review at head 220361ad. Review 578 at 714190e was treated as void and comment 16559 was not treated as a formal verdict. Both blockers from 16559 were verified fixed by independent probe and mutation testing, and both full-suite runs were measured by the reviewer in branches/ worktrees rather than accepting the author's figures.
WHY: B1 is fixed — apply_authorized is now the conjunction of the drain gate and the restart-class matrix, proven by a reviewer-authored probe in which break-glass fails to authorize a denied class across four worker roles and three restricted classes. B2 is fixed — the coordinator document now describes the post-union behaviour including both previously undocumented parameters. No new failure signature and no regression.
ISSUE: 663
HEAD_SHA: 220361ad94
REVIEW_STATUS: APPROVE
MERGE_READY: true
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-886-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; the same command from branches/review-886-head @220361ad gave 23 failed / 5230 passed / 6 skipped / 912 subtests; sorted FAILED lines diff empty, 23 identical signatures, +40 passing. Mutation tests: reverting the B1 conjunction to gate.allow fails 21 tests; restoring the pre-fix coordinator document fails 8 governance tests. A reviewer-authored independent probe of the real MCP tool passed 4 tests / 12 subtests.
LAST_UPDATED_BY: sysadmin (prgs-reviewer)

NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 220361ad94, action approve. No offline, import, or helper path was used.

[THREAD STATE LEDGER] PR #886 — approved review posted to Gitea

What is true now:

  • PR state: open
  • Server-side decision state: approved review posted to Gitea at head 220361ad94
  • Local verdict/state: approve verdict prepared locally
  • Latest known validation: head 23 failed / 5230 passed / 6 skipped / 912 subtests against baseline 23 failed / 5190 passed / 6 skipped / 867 subtests, identical 23 failure signatures

What changed:

  • approved review posted to Gitea

What is blocked:

  • Blocker classification: no blocker

Who/what acts next:

  • Next actor: merger
  • Required action: acquire the merger PR lease and land PR #886 with the head SHA pinned
  • Do not do: do not merge a moved head
  • Resume from: this review

Server-side mutation ledger:

  • gitea_acquire_reviewer_pr_lease → reviewer lease claimed, comment 16590
  • gitea_mark_final_review_decision → approve marked at head 220361ad
  • gitea_submit_pr_review → approved review posted to Gitea

Fresh independent review — PR #886 (#663) at head 220361ad

Review 578 (714190e) is void at this head and was not reused. Comment 16559
was never accepted by the review API, so there was no formal verdict of any kind
to carry forward. This is a complete re-review.

B1 — fixed and independently proven

gitea_request_mcp_restart now computes:

restart_class_authorized = bool(report.allow_restart)
gate_payload["drain_gate_allow"] = bool(gate.allow)
gate_payload["restart_class_authorized"] = restart_class_authorized
payload["apply_authorized"] = bool(gate.allow and restart_class_authorized)

I did not take the author's tests as proof. I wrote an independent probe driving
the real MCP tool and asserting the property from the outside:

  • The exact escalation is closed. With GITEA_BREAKGLASS_RESTART_AUTHORIZATION
    set and request_break_glass=True, dry_run=False, every combination of
    {author, reviewer, merger, reconciler} × {rolling_mcp_restart, full_mcp_restart, host_restart} — 12 subtests — reports allow_restart: False
    and apply_authorized: False, while break_glass_authorized stays True.
    One environment variable no longer collapses the nine-class matrix.
  • It is a conjunction, not a blanket deny. operator requesting
    full_mcp_restart with both approval and break-glass authorized still reaches
    apply_authorized: True. I confirmed the same for controller and admin by
    direct probe, so the fix did not simply disable the apply path.
  • Denials are attributable. For the denied case apply_gate carries
    drain_gate_allow: True, restart_class_authorized: False, and the reason
    "restart class authorization denied; apply denied regardless of drain proof or
    break-glass (fail closed, #663)" followed by the matrix's own reasons.
  • Unknown class denies even with break-glass under an authorized controller.

Mutation test: reverting the line to payload["apply_authorized"] = gate.allow
fails 21 tests in tests/test_issue_886_apply_authorization_conjunction.py,
including every break-glass subtest. The new coverage is not vacuous, and it
covers a tool that previously had none.

B2 — fixed

docs/mcp-restart-coordinator.md no longer claims the gate is "a separate
child". It now publishes drain_proof_json and request_break_glass in the
signature block, states that the hard gate executes inside this tool, and adds
sections for dry-run versus apply, authorization ordering (naming the
conjunction explicitly), break-glass scope, and fail-closed conditions. The
component table gains a drain_proof.gate_apply_restart row.

Mutation test: restoring the pre-remediation document fails 8 tests in
tests/test_mcp_restart_governance_docs.py, including
test_drain_proof_and_break_glass_parameters_are_documented,
test_gate_is_documented_as_executing_inside_this_tool, and
test_stale_pre_661_wording_cannot_return. One of the added tests compares the
published signature against inspect.signature() of the real tool, so future
parameter drift fails immediately rather than rotting silently.

Nothing was weakened

The remediation delta 9bc021e..220361ad touches four files: the coordinator
document, gitea_mcp_server.py, and the two test files. The
gitea_mcp_server.py change is contained inside the existing if not dry_run:
branch and is purely additive apart from the one changed assignment. The dry-run
path still never reports apply authorization, apply_supported and
restart_performed remain False on every path, and the #661 drain-proof
denials, the #663 nine-class matrix, fail-closed unknown-class handling, and
target scoping all still hold. The effective delta against master remains the
intended #663 work plus these two fixes and their tests — no unrelated file.

Validation, measured by the reviewer

Both runs from branches/ worktrees, since several suites resolve the repo root
from the checkout path and a /tmp checkout is not comparable.

# baseline, branches/review-886-baseline @ 2f4dec83
23 failed, 5190 passed, 6 skipped, 867 subtests passed in 158.94s

# head, branches/review-886-head @ 220361ad
23 failed, 5230 passed, 6 skipped, 912 subtests passed in 158.34s

diff of the sorted FAILED lines is empty — the 23 signatures are identical.
+40 passing: the 11 original #663 tests plus the 29 remediation tests. No new
failure signature, none disappeared, none changed.

Non-blocking, carried forward from comment 16559

  • N1 — configuration_reload pairs blast_radius=none with verdict=unsafe
    when a terminal lock exists. Conservative and safe; the pairing still reads
    oddly in an audit record and deserves an explicit comment stating the intent.
  • N2 — permissions_for_role grants all four worker roles the request
    permission for the four self-service classes. Matches the documented table and
    execution stays restricted to operator/admin; noted only for visibility.

Neither blocks this approval.

A follow-up worth filing separately

The author's own analysis of claim 16508 is correct and is a real protocol
defect: comment 16514 carried an uppercase PHASE: released block but no
<!-- mcp-conflict-fix-lease:v1 --> marker, and pr_work_lease._parse_marker_comment
returns None for any body without the marker. The release was invisible, the
chain stayed claimed, and the only signal was a reviewer refusal two hours
later. A release comment that silently fails to release is a fail-open in the
lease protocol and should carry its own issue rather than riding along with
#663.

Canonical Handoff

REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
ISSUE: 663
PR: 886
WORKFLOW_STATE: approved-awaiting-merge
HEAD_SHA: 220361ad940cbad7617386a8b6c50c740cddd0bc
BASE_BRANCH: master
BASE_OR_MERGE_SHA: 2f4dec832327513118f2fe92b74da25d124a01cb
ACTING_ROLE: reviewer
ACTING_IDENTITY: sysadmin (prgs-reviewer)
COMPLETED_ACTIONS: Fresh independent review of PR #886 at head 220361ad without reusing review 578 or comment 16559; verified B1 by a reviewer-authored probe of the real MCP tool and by mutation testing; verified B2 by mutation testing the governance suite; measured both full-suite runs in branches/ worktrees; posted an approve verdict at the exact head.
VALIDATION_EVIDENCE: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-886-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; from branches/review-886-head @220361ad gave 23 failed / 5230 passed / 6 skipped / 912 subtests; sorted FAILED diff empty, 23 identical signatures, +40 passing. Reverting the B1 conjunction fails 21 tests; restoring the pre-fix coordinator document fails 8 governance tests; the reviewer probe passed 4 tests and 12 subtests.
MUTATION_LEDGER: gitea_acquire_reviewer_pr_lease claimed the reviewer lease as comment 16590; gitea_mark_final_review_decision marked approve at head 220361ad; gitea_submit_pr_review posted the approve verdict to Gitea. Local: three review worktrees created under branches/ for measurement and removed afterwards; no tracked file was edited.
BLOCKERS: none
NEXT_ACTOR: merger
NEXT_ACTION: Acquire a merger PR lease and land PR #886 on master with expected_head_sha pinned to 220361ad940cbad7617386a8b6c50c740cddd0bc.
PROHIBITED_ACTIONS: Do not merge a moved head — a changed head voids this approval and requires a fresh review. Do not merge from the author or reviewer role. Do not rebase or force-push this branch.
NEXT_PROMPT: Act as prgs-merger on Scaled-Tech-Consulting/Gitea-Tools, remote prgs. PR #886 (issue #663, MCP restart class permissions) carries a fresh approve verdict at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc, base master @ 2f4dec832327513118f2fe92b74da25d124a01cb. Confirm approval_at_current_head is true and recommended_next_action is merge_now via gitea_assess_pr_sync_status, load the review workflow, resolve the merge capability, acquire or adopt the merger PR lease, then merge with expected_head_sha pinned. A moved head voids the approval and requires a fresh review. The merge auto-closes issue #663, strips status:pr-open, and clears the reviewer decision lock; expect the merge itself to re-stale the daemon, so let a failing post-merge lease release lapse by TTL rather than switching profiles.
WORKFLOW_FAILURE_ISSUES: none
LAST_UPDATED: 2026-07-25T07:24:00Z
## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: Acquire a merger PR lease and land PR #886 on master at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc. NEXT_PROMPT: ```text Act as prgs-merger on Scaled-Tech-Consulting/Gitea-Tools, remote prgs. PR #886 (issue #663, MCP restart class permissions) carries a fresh APPROVE at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc, base master @ 2f4dec832327513118f2fe92b74da25d124a01cb. Confirm approval_at_current_head is true and recommended_next_action is merge_now via gitea_assess_pr_sync_status, load the review workflow, resolve the merge capability, acquire or adopt the merger PR lease, then merge with the head SHA pinned. Do not merge if the head has moved; a moved head voids this approval and requires a fresh review. Merge auto-closes issue #663, strips status:pr-open, and clears the reviewer decision lock. ``` WHAT_HAPPENED: Completely fresh independent review at head 220361ad. Review 578 at 714190e was treated as void and comment 16559 was not treated as a formal verdict. Both blockers from 16559 were verified fixed by independent probe and mutation testing, and both full-suite runs were measured by the reviewer in branches/ worktrees rather than accepting the author's figures. WHY: B1 is fixed — apply_authorized is now the conjunction of the drain gate and the restart-class matrix, proven by a reviewer-authored probe in which break-glass fails to authorize a denied class across four worker roles and three restricted classes. B2 is fixed — the coordinator document now describes the post-union behaviour including both previously undocumented parameters. No new failure signature and no regression. ISSUE: 663 HEAD_SHA: 220361ad940cbad7617386a8b6c50c740cddd0bc REVIEW_STATUS: APPROVE MERGE_READY: true BLOCKERS: none VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-886-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; the same command from branches/review-886-head @220361ad gave 23 failed / 5230 passed / 6 skipped / 912 subtests; sorted FAILED lines diff empty, 23 identical signatures, +40 passing. Mutation tests: reverting the B1 conjunction to gate.allow fails 21 tests; restoring the pre-fix coordinator document fails 8 governance tests. A reviewer-authored independent probe of the real MCP tool passed 4 tests / 12 subtests. LAST_UPDATED_BY: sysadmin (prgs-reviewer) NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 220361ad940cbad7617386a8b6c50c740cddd0bc, action approve. No offline, import, or helper path was used. [THREAD STATE LEDGER] PR #886 — approved review posted to Gitea What is true now: - PR state: open - Server-side decision state: approved review posted to Gitea at head 220361ad940cbad7617386a8b6c50c740cddd0bc - Local verdict/state: approve verdict prepared locally - Latest known validation: head 23 failed / 5230 passed / 6 skipped / 912 subtests against baseline 23 failed / 5190 passed / 6 skipped / 867 subtests, identical 23 failure signatures What changed: - approved review posted to Gitea What is blocked: - Blocker classification: no blocker Who/what acts next: - Next actor: merger - Required action: acquire the merger PR lease and land PR #886 with the head SHA pinned - Do not do: do not merge a moved head - Resume from: this review Server-side mutation ledger: - gitea_acquire_reviewer_pr_lease → reviewer lease claimed, comment 16590 - gitea_mark_final_review_decision → approve marked at head 220361ad - gitea_submit_pr_review → approved review posted to Gitea --- # Fresh independent review — PR #886 (#663) at head `220361ad` Review `578` (`714190e`) is void at this head and was not reused. Comment `16559` was never accepted by the review API, so there was no formal verdict of any kind to carry forward. This is a complete re-review. ## B1 — fixed and independently proven `gitea_request_mcp_restart` now computes: ```python restart_class_authorized = bool(report.allow_restart) gate_payload["drain_gate_allow"] = bool(gate.allow) gate_payload["restart_class_authorized"] = restart_class_authorized payload["apply_authorized"] = bool(gate.allow and restart_class_authorized) ``` I did not take the author's tests as proof. I wrote an independent probe driving the real MCP tool and asserting the property from the outside: - **The exact escalation is closed.** With `GITEA_BREAKGLASS_RESTART_AUTHORIZATION` set and `request_break_glass=True, dry_run=False`, every combination of `{author, reviewer, merger, reconciler}` × `{rolling_mcp_restart, full_mcp_restart, host_restart}` — 12 subtests — reports `allow_restart: False` **and** `apply_authorized: False`, while `break_glass_authorized` stays `True`. One environment variable no longer collapses the nine-class matrix. - **It is a conjunction, not a blanket deny.** `operator` requesting `full_mcp_restart` with both approval and break-glass authorized still reaches `apply_authorized: True`. I confirmed the same for `controller` and `admin` by direct probe, so the fix did not simply disable the apply path. - **Denials are attributable.** For the denied case `apply_gate` carries `drain_gate_allow: True`, `restart_class_authorized: False`, and the reason "restart class authorization denied; apply denied regardless of drain proof or break-glass (fail closed, #663)" followed by the matrix's own reasons. - **Unknown class denies even with break-glass** under an authorized controller. Mutation test: reverting the line to `payload["apply_authorized"] = gate.allow` fails **21 tests** in `tests/test_issue_886_apply_authorization_conjunction.py`, including every break-glass subtest. The new coverage is not vacuous, and it covers a tool that previously had none. ## B2 — fixed `docs/mcp-restart-coordinator.md` no longer claims the gate is "a separate child". It now publishes `drain_proof_json` and `request_break_glass` in the signature block, states that the hard gate executes inside this tool, and adds sections for dry-run versus apply, authorization ordering (naming the conjunction explicitly), break-glass scope, and fail-closed conditions. The component table gains a `drain_proof.gate_apply_restart` row. Mutation test: restoring the pre-remediation document fails **8** tests in `tests/test_mcp_restart_governance_docs.py`, including `test_drain_proof_and_break_glass_parameters_are_documented`, `test_gate_is_documented_as_executing_inside_this_tool`, and `test_stale_pre_661_wording_cannot_return`. One of the added tests compares the published signature against `inspect.signature()` of the real tool, so future parameter drift fails immediately rather than rotting silently. ## Nothing was weakened The remediation delta `9bc021e..220361ad` touches four files: the coordinator document, `gitea_mcp_server.py`, and the two test files. The `gitea_mcp_server.py` change is contained inside the existing `if not dry_run:` branch and is purely additive apart from the one changed assignment. The dry-run path still never reports apply authorization, `apply_supported` and `restart_performed` remain `False` on every path, and the #661 drain-proof denials, the #663 nine-class matrix, fail-closed unknown-class handling, and target scoping all still hold. The effective delta against master remains the intended #663 work plus these two fixes and their tests — no unrelated file. ## Validation, measured by the reviewer Both runs from `branches/` worktrees, since several suites resolve the repo root from the checkout path and a `/tmp` checkout is not comparable. ```text # baseline, branches/review-886-baseline @ 2f4dec83 23 failed, 5190 passed, 6 skipped, 867 subtests passed in 158.94s # head, branches/review-886-head @ 220361ad 23 failed, 5230 passed, 6 skipped, 912 subtests passed in 158.34s ``` `diff` of the sorted `FAILED` lines is empty — the 23 signatures are identical. `+40` passing: the 11 original #663 tests plus the 29 remediation tests. No new failure signature, none disappeared, none changed. ## Non-blocking, carried forward from comment 16559 - N1 — `configuration_reload` pairs `blast_radius=none` with `verdict=unsafe` when a terminal lock exists. Conservative and safe; the pairing still reads oddly in an audit record and deserves an explicit comment stating the intent. - N2 — `permissions_for_role` grants all four worker roles the request permission for the four self-service classes. Matches the documented table and execution stays restricted to `operator`/`admin`; noted only for visibility. Neither blocks this approval. ## A follow-up worth filing separately The author's own analysis of claim `16508` is correct and is a real protocol defect: comment `16514` carried an uppercase `PHASE: released` block but no `<!-- mcp-conflict-fix-lease:v1 -->` marker, and `pr_work_lease._parse_marker_comment` returns `None` for any body without the marker. The release was invisible, the chain stayed `claimed`, and the only signal was a reviewer refusal two hours later. A release comment that silently fails to release is a fail-open in the lease protocol and should carry its own issue rather than riding along with #663. <!-- sph:v1 --> ## Canonical Handoff ```text REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools ISSUE: 663 PR: 886 WORKFLOW_STATE: approved-awaiting-merge HEAD_SHA: 220361ad940cbad7617386a8b6c50c740cddd0bc BASE_BRANCH: master BASE_OR_MERGE_SHA: 2f4dec832327513118f2fe92b74da25d124a01cb ACTING_ROLE: reviewer ACTING_IDENTITY: sysadmin (prgs-reviewer) COMPLETED_ACTIONS: Fresh independent review of PR #886 at head 220361ad without reusing review 578 or comment 16559; verified B1 by a reviewer-authored probe of the real MCP tool and by mutation testing; verified B2 by mutation testing the governance suite; measured both full-suite runs in branches/ worktrees; posted an approve verdict at the exact head. VALIDATION_EVIDENCE: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-886-baseline @2f4dec83 gave 23 failed / 5190 passed / 6 skipped / 867 subtests; from branches/review-886-head @220361ad gave 23 failed / 5230 passed / 6 skipped / 912 subtests; sorted FAILED diff empty, 23 identical signatures, +40 passing. Reverting the B1 conjunction fails 21 tests; restoring the pre-fix coordinator document fails 8 governance tests; the reviewer probe passed 4 tests and 12 subtests. MUTATION_LEDGER: gitea_acquire_reviewer_pr_lease claimed the reviewer lease as comment 16590; gitea_mark_final_review_decision marked approve at head 220361ad; gitea_submit_pr_review posted the approve verdict to Gitea. Local: three review worktrees created under branches/ for measurement and removed afterwards; no tracked file was edited. BLOCKERS: none NEXT_ACTOR: merger NEXT_ACTION: Acquire a merger PR lease and land PR #886 on master with expected_head_sha pinned to 220361ad940cbad7617386a8b6c50c740cddd0bc. PROHIBITED_ACTIONS: Do not merge a moved head — a changed head voids this approval and requires a fresh review. Do not merge from the author or reviewer role. Do not rebase or force-push this branch. NEXT_PROMPT: Act as prgs-merger on Scaled-Tech-Consulting/Gitea-Tools, remote prgs. PR #886 (issue #663, MCP restart class permissions) carries a fresh approve verdict at exact head 220361ad940cbad7617386a8b6c50c740cddd0bc, base master @ 2f4dec832327513118f2fe92b74da25d124a01cb. Confirm approval_at_current_head is true and recommended_next_action is merge_now via gitea_assess_pr_sync_status, load the review workflow, resolve the merge capability, acquire or adopt the merger PR lease, then merge with expected_head_sha pinned. A moved head voids the approval and requires a fresh review. The merge auto-closes issue #663, strips status:pr-open, and clears the reviewer decision lock; expect the merge itself to re-stale the daemon, so let a failing post-merge lease release lapse by TTL rather than switching profiles. WORKFLOW_FAILURE_ISSUES: none LAST_UPDATED: 2026-07-25T07:24:00Z ```
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #886
issue: #663
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: review-886-220361ad
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-886-head
phase: released
candidate_head: 220361ad94
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T07:24:44Z
expires_at: 2026-07-25T07:34:44Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #886 issue: #663 reviewer_identity: sysadmin profile: prgs-reviewer session_id: review-886-220361ad worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-886-head phase: released candidate_head: 220361ad940cbad7617386a8b6c50c740cddd0bc target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T07:24:44Z expires_at: 2026-07-25T07:34:44Z blocker: manual-release
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #886
issue: #663
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 65552-179c25ed6369
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes
phase: claimed
candidate_head: 220361ad94
target_branch: master
target_branch_sha: 2f4dec8323
last_activity: 2026-07-25T07:33:46Z
expires_at: 2026-07-25T07:43:46Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #886 issue: #663 reviewer_identity: sysadmin profile: prgs-merger session_id: 65552-179c25ed6369 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-663-restart-classes phase: claimed candidate_head: 220361ad940cbad7617386a8b6c50c740cddd0bc target_branch: master target_branch_sha: 2f4dec832327513118f2fe92b74da25d124a01cb last_activity: 2026-07-25T07:33:46Z expires_at: 2026-07-25T07:43:46Z blocker: none
sysadmin merged commit 8598537a35 into master 2026-07-25 02:34:24 -05:00
Owner

Stale #332 review-decision lock cleanup (#594)

Status: APPLIED

  • actor: sysadmin
  • profile: prgs-merger
  • timestamp: 2026-07-25T07:34:26.243641+00:00
  • last terminal: approve on PR feat: enforce MCP restart class permissions (#886)
  • PR state: closed (merged=True)
  • merge_commit_sha: 8598537a3547b646bfeedf6f96482959489746dd
  • prior live_mutations_count: 6
  • prior profile_identity: prgs-reviewer

Manual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-25T07:34:26.243641+00:00` - last terminal: `approve` on PR #886 - PR state: `closed` (merged=True) - merge_commit_sha: `8598537a3547b646bfeedf6f96482959489746dd` - prior live_mutations_count: `6` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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