feat: enforce MCP restart class permissions #886

2026-07-24T17:21:10-05:00

jcwalker3 commented

2026-07-24 17:21:10 -05:00

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

feat: enforce MCP restart class permissions (#663 ) 714190e02a

jcwalker3 commented

2026-07-24 17:22:51 -05:00

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:

feat: enforce MCP restart class permissions (#886)

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  ## 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

sysadmin left a comment

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

Merge branch 'master' into feat/issue-663-restart-classes 301c78de20

jcwalker3 added 1 commit 2026-07-24 21:27:18 -05:00

Merge branch 'master' into feat/issue-663-restart-classes 41622c5985

jcwalker3 added 1 commit 2026-07-24 22:34:35 -05:00

Merge branch 'master' into feat/issue-663-restart-classes 930dc24632

jcwalker3 commented

2026-07-25 00:17:24 -05:00

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

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 referenced this issue from a commit

2026-07-25 00:17:45 -05:00

Merge master into feat/issue-663-restart-classes (resolve #886 conflict)

jcwalker3 added 1 commit 2026-07-25 00:17:45 -05:00

Merge master into feat/issue-663-restart-classes (resolve #886 conflict) 9bc021e9c0

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]>

jcwalker3 commented

2026-07-25 00:22:10 -05:00

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.

sysadmin commented

2026-07-25 01:05:49 -05:00

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

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

sysadmin commented

2026-07-25 01:10:07 -05:00

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. ```

sysadmin commented

2026-07-25 01:10:25 -05:00

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

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 referenced this issue from a commit

2026-07-25 01:38:22 -05:00

fix(restart): require both authorizations for apply, correct coordinator doc

jcwalker3 added 1 commit 2026-07-25 01:38:22 -05:00

fix(restart): require both authorizations for apply, correct coordinator doc 220361ad94

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

You are not authorized to merge this pull request.

This pull request can be merged automatically.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/issue-663-restart-classes:feat/issue-663-restart-classes

git checkout feat/issue-663-restart-classes

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

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

feat: enforce MCP restart class permissions #886

Summary

Validation

Scope and risk

Canonical Issue State

Canonical Handoff

Formal Review Report: APPROVED

Verification

Conflict remediation complete — PR #886 no longer carries conflicts

Conflict-fix lease release

What conflicted

How it was resolved

Verification evidence

Review status — a fresh independent review is required

Canonical Issue State

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

Union verification — the 9bc021e conflict resolution

The 15 commits 714190e..930dc246

Independent baseline (author figures not used)

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

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

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

Non-blocking observations

What is good here

Canonical Issue State

Checkout

Union verification — the `9bc021e` conflict resolution

The 15 commits `714190e..930dc246`

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

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