From 1301a57de4626118cc2a0d81ce6eb09da22ac3ac Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 23 Jul 2026 19:16:28 -0400 Subject: [PATCH] docs(governance): MCP restart governance and authorization policy (#656) Adds docs/architecture/mcp-restart-governance.md, the restart-governance/v1 ADR defining who may restart the MCP control plane and under what conditions. - Recovery ladder (reconnect -> rebind -> scoped restart -> full restart -> host) with restart stated as the last resort. - Authorization matrix across author/reviewer/merger/reconciler/controller/ operator/admin; no LLM worker role may perform or authorize a full or host restart. - v1 authority decision recorded: controller approval + automated safety gates; quorum deferred to a superseding ADR. - Break-glass path with pre-declared incident and mandatory post-hoc audit. - Ambiguous policy state denies restart. - Stable policy IDs RG-01..RG-08 for later enforcement code to bind to. Cross-links the ADR from docs/safety-model.md and docs/webui-deployment.md, and adds tests/test_mcp_restart_governance_docs.py asserting acceptance criteria 1-5. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/architecture/mcp-restart-governance.md | 222 ++++++++++++++++++++ docs/safety-model.md | 14 ++ docs/webui-deployment.md | 9 + tests/test_mcp_restart_governance_docs.py | 107 ++++++++++ 4 files changed, 352 insertions(+) create mode 100644 docs/architecture/mcp-restart-governance.md create mode 100644 tests/test_mcp_restart_governance_docs.py diff --git a/docs/architecture/mcp-restart-governance.md b/docs/architecture/mcp-restart-governance.md new file mode 100644 index 0000000..da8fbd4 --- /dev/null +++ b/docs/architecture/mcp-restart-governance.md @@ -0,0 +1,222 @@ +# ADR: MCP restart governance and authorization policy + +- **Status:** Accepted (policy effective immediately for LLM and operator sessions; enforcement tooling may lag) +- **Date:** 2026-07-23 +- **Tracking issue:** [#656](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/656) +- **Policy version:** `restart-governance/v1` +- **Related:** + - Umbrella: [#655](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/655) — MCP restart / health hardening umbrella + - Vision: [#652](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/652) — control-plane restart vision + - Roadmap: [#653](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/653) — restart hardening roadmap + - Coordinator: [#630](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/630) — forbids process-kill recovery + - Break-glass / console approval: [#642](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/642) + - Transport recovery: [#591](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/591), [#584](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/584) — EOF / transport-flap self-recovery + - Stable-control runtime split: `docs/architecture/mcp-stable-control-runtime-policy-adr.md` (#615) + - Client-namespace health: `docs/mcp-namespace-health.md` (#543) + - Reconnect-only EOF recovery: `docs/mcp-namespace-eof-recovery.md` + +## 1. Context + +The Gitea MCP server is the **control plane** for real issue and PR mutations +(create, comment, lock, review, merge, reconcile). The same process serves every +role namespace (`gitea-author`, `gitea-reviewer`, `gitea-merger`, +`gitea-reconciler`, `gitea-controller`) and holds the in-memory capability-gate +code loaded at startup. + +Restarting that process is destructive to concurrent work: + +- It resets every session's identity, preflight, and capability-lease binding. +- It can interrupt a mutation mid-critical-section (a lock acquire, a review + submit, a merge), leaving durable state half-written. +- Relaunching from the wrong checkout or worktree silently changes which code + the control plane runs, defeating master-parity gates (#420 / #615). + +Today there is **no durable written policy** stating who may restart MCP, under +what conditions, that restart is a last resort, and how controller approval, +automated safety gates, and break-glass interact. Operators and LLM sessions +therefore invent restart behavior ad hoc, which makes concurrent multi-role work +unsafe. #630 and #642 need this policy as their backbone. + +This ADR defines that policy. It does **not** implement coordinator code or HA +multi-instance restart (those are later children of #655). + +## 2. Decision + +### 2.1 v1 decision (recorded) + +**Restart authority in v1 is `controller approval + automated safety gates`.** + +A restart of the stable control runtime is authorized only when **both** hold: + +1. A **controller** role explicitly approves the restart, recording an audit + entry (who, why, scope, affected sessions), **and** +2. The **automated safety gates** pass: a completed drain acknowledgement (no + affected session is mid-critical-section) or a declared break-glass incident + (§2.5). + +Quorum among multiple controllers is **not** required day-one. It is deferred +unless a later investigation (tracked under #653) proves single-controller +approval is insufficient. This ADR records the v1 decision so enforcement code +(#630) has a fixed target; changing it requires a superseding ADR. + +### 2.2 Restart is a last resort — the recovery ladder + +Restart is the **last** rung. Before any restart, exhaust the narrower +recoveries, in order: + +1. **Reconnect** the IDE/client MCP namespace (transport EOF, `client is + closing: EOF`, transient `#584` flap). No process change. See + `docs/mcp-namespace-eof-recovery.md`. +2. **Refresh / rebind** the session workspace: re-run `gitea_whoami`, + `gitea_resolve_task_capability`, and pass an explicit validated + `worktree_path`. Fixes stale session context without touching the process. +3. **Scoped restart** of a single misbehaving namespace/service (where the + deployment supports per-service restart) rather than the whole control plane. +4. **Full restart** of the stable control runtime process — operator-owned, + controller-approved, drained. +5. **Host / infrastructure restart** — the broadest action; same authorization + as a full restart plus infrastructure ownership. + +A session **must** try rungs 1–2 and record why they were insufficient before +requesting a restart at rung 3 or above. Skipping straight to restart is a +policy violation. + +### 2.3 Authorization matrix + +| Role | Reconnect (1) | Refresh/rebind (2) | Scoped restart (3) | Full restart (4) | Host restart (5) | +|---|---|---|---|---|---| +| **author** | self | self | request only | **forbidden** | forbidden | +| **reviewer** | self | self | request only | **forbidden** | forbidden | +| **merger** | self | self | request only | **forbidden** | forbidden | +| **reconciler** | self | self | request only | **forbidden** | forbidden | +| **controller** | self | self | **approve** (+gates) | **approve** (+gates) | request to operator | +| **operator** | self | self | execute (controller-approved) | execute (controller-approved) | execute (controller-approved) | +| **admin** | self | self | execute | execute | execute (break-glass) | + +Legend: *self* = may perform for its own client session; *request only* = may +raise a restart request but not authorize or execute it; *approve* = may +authorize under §2.1 gates; *execute* = may perform the process action after the +authorization is recorded. + +Key invariants: + +- **No LLM worker role (author/reviewer/merger/reconciler) may perform or + authorize a full or host restart.** They may only reconnect/rebind their own + client and file a restart request. +- **Controller approval authorizes; operator/admin executes.** The approving + controller and the executing operator may be the same human, but both the + approval and the execution are audited. +- Privileged process actions (full restart, host restart) are reserved to + **operator/admin**, never to an automated worker. + +### 2.4 Approved conditions + +A restart at rung 3+ is approved only under one of these recorded conditions: + +- **No affected sessions:** the control plane has no live session that would be + interrupted (verified, not assumed). +- **Full drain acknowledged:** every affected session has drained + (no open critical section — no held mutation lease mid-write) and the drain is + acknowledged in the audit record. +- **Controller + gates:** controller approval plus passing automated safety + gates (§2.1), the standard v1 path. +- **Quorum:** not required in v1; reserved for a future superseding ADR. +- **Break-glass:** an incident-backed emergency exception (§2.5). + +Restart **never** bypasses mutation gates mid-critical-section. Drain before +restart is mandatory except under break-glass with a declared incident. + +### 2.5 Break-glass + +Break-glass is a **separate, narrower** authorization path for emergencies where +the normal drain-and-approve path cannot complete (e.g. the control plane is +wedged and cannot drain). + +Break-glass conditions: + +- A declared incident record exists (id, timestamp, declarer) **before** the + action. +- The action is taken by **operator or admin** authority only — never by an LLM + worker role, and never unilaterally by an operator with active peers when a + controller is reachable. +- The scope is the minimum necessary rung of the ladder. +- A **mandatory post-hoc audit** entry is filed: what was restarted, why the + normal path was impossible, which sessions were affected, and the incident id. + +Break-glass suspends the drain requirement, not the audit requirement. + +### 2.6 Explicit prohibitions + +- **A unilateral LLM or operator full restart while active peer sessions + exist is forbidden.** An LLM worker role must not kill, restart, or relaunch + the MCP process; a lone operator must not full-restart over live peer work + without controller approval or a break-glass incident. +- Process-kill recovery is forbidden as a routine tool (#630). This ADR does not + introduce a kill path. +- Ambiguous policy state **denies** restart (§4). + +## 3. Security requirements + +- Full restart and host restart are **privileged**; only operator/admin execute + them, only after a controller approval or break-glass incident is recorded. +- Break-glass is a distinct authorization path with its own audit mandate; it is + never the default and never silent. +- **Every approval and every restart action is audited** (who approved, who + executed, scope, affected sessions, condition, policy version). No restart is + authorized without a durable audit entry. + +## 4. Failure behavior + +**Ambiguous policy → deny restart.** If it cannot be established that a +restart is authorized under §2 — unknown affected-session state, missing +controller approval, absent break-glass incident, or an unclassifiable request — +the safe action is to **refuse** the restart and stop with a recovery report, +never to restart on assumption. + +## 5. Policy IDs (for enforcement code) + +Enforcement code (#630 coordinator, #642 console approval) binds to these stable +policy identifiers rather than to prose: + +| Policy ID | Statement | +|---|---| +| `RG-01` | Restart is last resort; rungs 1–2 must be tried and recorded first (§2.2). | +| `RG-02` | v1 authority = controller approval + automated safety gates (§2.1). | +| `RG-03` | No LLM worker role performs or authorizes full/host restart (§2.3). | +| `RG-04` | Full/host restart executed by operator/admin only, post approval (§2.3). | +| `RG-05` | Drain before restart is mandatory except break-glass with incident (§2.4). | +| `RG-06` | Break-glass requires a pre-declared incident and post-hoc audit (§2.5). | +| `RG-07` | Unilateral LLM/operator full restart with active peers is forbidden (§2.6). | +| `RG-08` | Ambiguous policy state denies restart (§4). | + +The `restart-governance/v1` **policy version** field is emitted on future +restart audit events so approvals can be reconciled against the policy revision +in force. + +## 6. Dogfooding + +Gitea-Tools governs its own MCP control plane by this policy. Author, reviewer, +merger, and reconciler sessions operating on this repository use the recovery +ladder (§2.2) — reconnect and rebind, never self-restart — and any real restart +of the Gitea-Tools stable control runtime follows the controller-approval + +drain path defined here. + +## 7. Acceptance and cross-links + +This ADR is the authoritative restart-governance policy. It **must** stay +cross-linked from the safety model and the web-console deployment boundary: + +- `docs/safety-model.md` § Process restart governance references this ADR. +- `docs/webui-deployment.md` references this ADR for restart/reload disposition. + +It is linked to its issue lineage — umbrella **#655**, vision **#652**, roadmap +**#653**, coordinator **#630**, and break-glass / console approval **#642** — in +§ Related above. + +## 8. Non-goals + +- Implementing the restart coordinator or approval state machine (#630, later + children of #655). +- Implementing HA multi-instance restart or quorum machinery. +- Introducing any process-kill or auto-restart tool; existing auto-restart + behavior must be inventoried before any new restart tool is enabled. diff --git a/docs/safety-model.md b/docs/safety-model.md index 31c740a..bbab241 100644 --- a/docs/safety-model.md +++ b/docs/safety-model.md @@ -46,3 +46,17 @@ If shell helpers are unavailable and MCP commit cannot run, stop with a recovery report (restart session, clear hung terminals, use MCP-native commit). See [`llm-workflow-runbooks.md`](llm-workflow-runbooks.md) § MCP-native commit path (#260) and agent temp artifact cleanup (#261). + +## 7. Process restart governance + +Restarting the MCP control-plane process is destructive to concurrent multi-role +work and is governed by a dedicated policy. Restart is a **last resort** behind +narrower recoveries (reconnect, rebind), full/host restart is reserved to +operator/admin under **controller approval + automated safety gates**, a +unilateral LLM or operator full restart with active peers is **forbidden**, and +ambiguous policy state **denies** restart. Break-glass is a separate, +incident-backed path with a mandatory audit. + +See [`architecture/mcp-restart-governance.md`](architecture/mcp-restart-governance.md) +(#656) for the authorization matrix, the recovery ladder, break-glass +conditions, and the `RG-01`–`RG-08` policy IDs. diff --git a/docs/webui-deployment.md b/docs/webui-deployment.md index fa754ad..2ad46ab 100644 --- a/docs/webui-deployment.md +++ b/docs/webui-deployment.md @@ -55,6 +55,15 @@ shipped to the browser. assumption paths, and the client-secret policy. Use it to verify an instance is configured for internal-only operation. +## Process restart / reload disposition + +The console never exposes a restart or reload control; process restart of the +MCP control-plane runtime is governed separately. Restart is a last resort behind +reconnect/rebind, full restart is operator/admin-only under controller approval +plus safety gates, and break-glass is an incident-backed path. See +[`architecture/mcp-restart-governance.md`](architecture/mcp-restart-governance.md) +(#656). + ## Non-goals (MVP) - Full SSO or session login in the UI diff --git a/tests/test_mcp_restart_governance_docs.py b/tests/test_mcp_restart_governance_docs.py new file mode 100644 index 0000000..5b12e50 --- /dev/null +++ b/tests/test_mcp_restart_governance_docs.py @@ -0,0 +1,107 @@ +"""Documentation acceptance for the MCP restart governance ADR (#656). + +Enforces issue #656 acceptance criteria: + +* AC1 — policy document exists with an authorization matrix and the recorded + v1 decision (controller approval + automated safety gates). +* AC2 — restart is stated as a last resort with enumerated narrower recoveries. +* AC3 — a unilateral LLM full restart with affected sessions is forbidden. +* AC4 — break-glass conditions are listed. +* AC5 — the ADR is linked to #655, #652, #653, #630, #642, and is cross-linked + from the safety model and the web-console deployment boundary docs. +""" +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +ADR = REPO_ROOT / "docs" / "architecture" / "mcp-restart-governance.md" +ADR_BASENAME = "mcp-restart-governance.md" + +CROSS_LINK_DOCS = ( + REPO_ROOT / "docs" / "safety-model.md", + REPO_ROOT / "docs" / "webui-deployment.md", +) + +LINKED_ISSUES = ("#655", "#652", "#653", "#630", "#642") +POLICY_IDS = ("RG-01", "RG-02", "RG-03", "RG-04", "RG-05", "RG-06", "RG-07", "RG-08") + + +def _read(path: Path) -> str: + assert path.is_file(), f"missing {path.relative_to(REPO_ROOT)}" + return path.read_text(encoding="utf-8") + + +def test_ac1_adr_exists_with_matrix_and_v1_decision(): + text = _read(ADR) + lower = text.lower() + assert text.lstrip().startswith("#"), "ADR lacks a title" + assert "#656" in text + assert "authorization matrix" in lower + # The matrix is a real table with the worker and privileged roles. + for role in ("author", "reviewer", "merger", "reconciler", "controller", + "operator", "admin"): + assert role in lower, f"authorization matrix missing role {role!r}" + # Recorded v1 decision. + assert "restart-governance/v1" in text + assert "controller approval" in lower and "automated safety gates" in lower + + +def test_ac2_restart_is_last_resort_with_narrower_recoveries(): + text = _read(ADR) + lower = text.lower() + assert "last resort" in lower + # Enumerated narrower recoveries precede full restart on the ladder. + for rung in ("reconnect", "rebind", "scoped restart", "full restart", + "host"): + assert rung in lower, f"recovery ladder missing rung {rung!r}" + + +def test_ac3_forbids_unilateral_llm_full_restart_with_affected_sessions(): + text = _read(ADR) + lower = text.lower() + assert "forbidden" in lower + assert "llm" in lower and "restart" in lower + assert "unilateral" in lower + # A worker role must not perform or authorize full/host restart. + assert "must not" in lower + + +def test_ac4_break_glass_conditions_listed(): + text = _read(ADR) + lower = text.lower() + assert "break-glass" in lower + assert "incident" in lower + assert "audit" in lower + + +def test_ac5_adr_links_issue_lineage(): + text = _read(ADR) + for issue in LINKED_ISSUES: + assert issue in text, f"ADR must link issue {issue}" + + +def test_ac5_safety_model_and_deployment_cross_link_adr(): + for path in CROSS_LINK_DOCS: + text = _read(path) + assert ADR_BASENAME in text, ( + f"{path.relative_to(REPO_ROOT)} must cross-link {ADR_BASENAME} " + f"(issue #656 acceptance criterion 5)" + ) + + +def test_policy_ids_present_for_enforcement_code(): + text = _read(ADR) + for pid in POLICY_IDS: + assert pid in text, f"policy id {pid} missing from ADR" + + +def test_failure_behavior_denies_on_ambiguity(): + text = _read(ADR) + lower = text.lower() + assert "ambiguous" in lower and "deny" in lower + + +def test_cross_links_do_not_embed_secrets(): + for path in (ADR,) + CROSS_LINK_DOCS: + text = _read(path) + for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"): + assert marker not in text, f"{path} contains {marker!r}"