From 461e1dac78a54c7d30851d05ae8e5983fdcc659b Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 17:35:11 -0400 Subject: [PATCH] feat(mcp): enforce scoped recovery playbook before full restart (Closes #669) Add recovery_playbook.py with the narrow-to-broad recovery ladder, symptom routing, attempt-log helpers, and escalation metrics. Wire the attempt-log gate into restart_coordinator so rolling/full/host restarts require prior insufficient narrower attempts (or break-glass). Document the ladder and update gitea_request_mcp_restart for prior_recovery_attempts_json. Co-Authored-By: Grok 4.5 --- docs/mcp-recovery-playbook.md | 94 +++ docs/mcp-restart-coordinator.md | 20 +- gitea_mcp_server.py | 44 +- recovery_playbook.py | 583 ++++++++++++++++++ restart_coordinator.py | 48 +- ...sue_886_apply_authorization_conjunction.py | 14 + tests/test_recovery_playbook.py | 217 +++++++ 7 files changed, 1004 insertions(+), 16 deletions(-) create mode 100644 docs/mcp-recovery-playbook.md create mode 100644 recovery_playbook.py create mode 100644 tests/test_recovery_playbook.py diff --git a/docs/mcp-recovery-playbook.md b/docs/mcp-recovery-playbook.md new file mode 100644 index 0000000..b589b56 --- /dev/null +++ b/docs/mcp-recovery-playbook.md @@ -0,0 +1,94 @@ +# MCP scoped recovery playbook (#669) + +**Parent:** [#655](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/655) +**Vision / roadmap:** [#652](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/652) · [#653](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/653) +**Class matrix:** [#663](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/663) · `docs/mcp-restart-classes.md` +**Coordinator:** [#658](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/658) · `restart_coordinator.py` +**Audit lineage:** [#665](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/665) + +## Decision + +Full-server MCP reset is a **last resort**. Prefer the narrowest recovery that +can clear the symptom. The coordinator **refuses** `rolling_mcp_restart`, +`full_mcp_restart`, and `host_restart` unless: + +1. The inventory carries a prior **attempt log** of at least one *insufficient* + narrower recovery, **or** +2. **Break-glass** is authorized + (`request_break_glass` + `GITEA_BREAKGLASS_RESTART_AUTHORIZATION`). + +Break-glass still never bypasses the #663 class matrix (role/permission). + +## Ladder (narrow → broad) + +| Rank | Action | Self-service | Implementation / delegation | +|---:|---|---|---| +| 0 | `client_reconnect` | yes | Host auto-reconnect / client reconnect · [#584](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/584) · `docs/mcp-namespace-eof-recovery.md` | +| 1 | `capability_refresh` | yes | `gitea_resolve_task_capability` + `gitea_whoami` · [#610](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/610) · [#685](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/685) | +| 2 | `session_reconnect` | yes | Runtime rebind + explicit `worktree_path` · [#543](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/543) · [#618](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/618) | +| 3 | `configuration_reload` | no | Class `configuration_reload` · console reload · [#642](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/642) | +| 4 | `lease_recovery` | no | Lock/lease recovery paths · [#702](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/702) · [#753](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/753) · [#790](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/790) | +| 5 | `worker_restart` | no | Class `worker_restart` · #663 | +| 6 | `role_runtime_restart` | no | Class `role_runtime_restart` · console restart · #642/#663 | +| 7 | `connector_restart` | no | Class `connector_restart` · #663 | +| 8 | `rolling_mcp_restart` | no | Class `rolling_mcp_restart` · design [#668](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/668) · **attempt log required** | +| 9 | `full_mcp_restart` | no | Class `full_mcp_restart` · **attempt log required** | +| 10 | `host_restart` | no | Class `host_restart` · **attempt log required** | + +Machine-readable source of truth: `recovery_playbook.RECOVERY_LADDER` and +`recovery_playbook.ladder_document()`. + +## Attempt log shape + +Each prior attempt is a mapping: + +```json +{ + "action": "client_reconnect", + "outcome": "insufficient", + "reason": "transport still closed after IDE reconnect", + "actor": "prgs-controller-12345", + "recorded_at": "2026-07-25T21:00:00+00:00" +} +``` + +Outcomes that count toward escalation: `failed`, `insufficient`, `denied`, +`unresolved`, `timeout`, `error`. + +Pass attempts into the coordinator via inventory +`prior_recovery_attempts` or the MCP tool argument +`prior_recovery_attempts_json` on `gitea_request_mcp_restart`. + +Helper: `recovery_playbook.build_attempt_record(...)`. + +## Symptom → first rung + +`recovery_playbook.recommend_actions(symptoms=[...])` maps symptoms such as +`transport_eof`, `stale_capability`, `stale_lease`, `daemon_corrupt` to the +narrowest recommended action, then walks the ladder. Soft recommendations +never replace the hard gate on broad restarts. + +## Enforcement points + +1. **`recovery_playbook.assess_escalation`** — pure gate. +2. **`restart_coordinator.evaluate_restart_impact`** — when `restart_class` is + set (policy-enforced path), broad classes require the gate; report fields + `attempt_log_satisfied`, `playbook_escalation`, `break_glass`. +3. **`gitea_request_mcp_restart`** — accepts attempt JSON and env-authorized + break-glass; never restarts a process. + +## Metrics + +`recovery_playbook.recovery_metrics(attempts)` reports the fraction of +successful recoveries that avoided full/host restart +(`fraction_avoided_full_restart`). + +## Non-goals + +* HA multi-instance execution (#668 design only here). +* Normalizing `pkill` (#630 contamination stays forbidden). +* Silent mutation of leases or processes from the playbook itself. + +## Manual process kills + +Remain forbidden and contaminating (#630). The playbook never recommends them. diff --git a/docs/mcp-restart-coordinator.md b/docs/mcp-restart-coordinator.md index 628895f..684663d 100644 --- a/docs/mcp-restart-coordinator.md +++ b/docs/mcp-restart-coordinator.md @@ -95,12 +95,18 @@ gitea_request_mcp_restart(remote, host, org, repo, target_session_id=None, target_role=None, target_connector=None, drain_proof_json=None, - request_break_glass=False) + request_break_glass=False, + prior_recovery_attempts_json=None) ``` It **never restarts anything**: `apply_supported` is always `false` and `restart_performed` is always `false`. +`prior_recovery_attempts_json` (#669) is an optional JSON array of prior +narrow recovery attempts. Rolling / full / host classes require at least one +*insufficient* narrower attempt (or authorized break-glass). See +`docs/mcp-recovery-playbook.md`. + ### Dry-run versus apply | Call | Behavior | @@ -112,9 +118,10 @@ It **never restarts anything**: `apply_supported` is always `false` and An apply requires **both** authorizations, and they are independent: -1. **Restart-class authorization** (#663) — the requester's role and permissions - must allow the requested class, the class's approval requirement must be - satisfied, and any target-scoped class must name its target. Failing any of +1. **Restart-class authorization** (#663 / #669) — the requester's role and + permissions must allow the requested class, the class's approval requirement + must be satisfied, any target-scoped class must name its target, and broad + classes must satisfy the recovery-playbook attempt-log gate. Failing any of these makes `allow_restart` `false`. 2. **Drain-proof gate** (#661) — a valid, unexpired, clean proof bound to the current impact fingerprint, or an authorized break-glass. @@ -127,7 +134,10 @@ the authorization that produced it. ### Break-glass -Break-glass bypasses the **drain proof only** — never the restart-class matrix. +Break-glass bypasses the **drain proof only** — never the restart-class matrix +(role/permission). Separately, authorized break-glass also satisfies the #669 +attempt-log requirement for broad restarts (rolling/full/host), because that +gate is not a class-matrix permission check. It is honoured solely when `request_break_glass` is set *and* the environment carries `GITEA_BREAKGLASS_RESTART_AUTHORIZATION`; like operator override, the tool argument expresses caller intent and cannot be self-asserted by a worker diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index aab8249..915eaad 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -22575,8 +22575,9 @@ def gitea_request_mcp_restart( target_connector: str | None = None, drain_proof_json: str | None = None, request_break_glass: bool = False, + prior_recovery_attempts_json: str | None = None, ) -> dict: - """Evaluate a proposed MCP restart and return an impact preview (#658). + """Evaluate a proposed MCP restart and return an impact preview (#658/#669). Central restart coordinator: resolves the requested restart class, gathers live control-plane state (sessions, @@ -22600,10 +22601,16 @@ def gitea_request_mcp_restart( independent — the drain gate proves the blast radius was drained and knows nothing about whether this requester may request this class — so a class the matrix denied never reports an authorized apply. Break-glass bypasses the - drain proof only; it never bypasses the class matrix. ``apply_gate`` carries + drain proof and, when env-authorized, the #669 attempt-log requirement for + broad restarts; it never bypasses the class matrix. ``apply_gate`` carries ``drain_gate_allow`` and ``restart_class_authorized`` so a denial is attributable to the authorization that produced it. + ``prior_recovery_attempts_json`` (#669) is an optional JSON array of prior + narrow recovery attempts ``{action, outcome, reason, ...}``. Rolling / full + / host restart classes require at least one *insufficient* narrower attempt + unless break-glass is authorized. + Operator override authority is read from the process environment (``GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION``), never self-asserted by the requesting session: ``request_override`` only expresses caller intent @@ -22709,12 +22716,37 @@ def gitea_request_mcp_restart( requester_role ) + prior_recovery_attempts: list[dict] = [] + if prior_recovery_attempts_json: + try: + parsed_attempts = json.loads(prior_recovery_attempts_json) + if isinstance(parsed_attempts, list): + prior_recovery_attempts = [ + dict(a) for a in parsed_attempts if isinstance(a, dict) + ] + else: + incomplete_reasons.append( + "prior_recovery_attempts_json must be a JSON array (#669)" + ) + inventory_complete = False + except (ValueError, TypeError) as exc: + incomplete_reasons.append( + f"invalid prior_recovery_attempts_json: {_redact(str(exc))}" + ) + inventory_complete = False + + break_glass_authorized = bool( + (os.environ.get("GITEA_BREAKGLASS_RESTART_AUTHORIZATION") or "").strip() + ) + break_glass = bool(request_break_glass and break_glass_authorized) + inventory = { "sessions": sessions, "leases": leases, "terminal_lock": terminal_lock, "inventory_complete": inventory_complete, "incomplete_reasons": incomplete_reasons, + "prior_recovery_attempts": prior_recovery_attempts, } report = restart_coordinator.evaluate_restart_impact( @@ -22730,6 +22762,7 @@ def gitea_request_mcp_restart( target_session_id=target_session_id, target_role=target_role, target_connector=target_connector, + break_glass=break_glass, ) payload = report.as_dict() @@ -22762,13 +22795,6 @@ def gitea_request_mcp_restart( except (ValueError, TypeError) as exc: proof_parse_error = f"invalid drain_proof_json: {_redact(str(exc))}" - break_glass_authorized = bool( - ( - os.environ.get("GITEA_BREAKGLASS_RESTART_AUTHORIZATION") or "" - ).strip() - ) - break_glass = bool(request_break_glass and break_glass_authorized) - expected_fp = drain_proof.impact_fingerprint(report.as_dict()) gate = drain_proof.gate_apply_restart( proof=proof_obj, diff --git a/recovery_playbook.py b/recovery_playbook.py new file mode 100644 index 0000000..268a3be --- /dev/null +++ b/recovery_playbook.py @@ -0,0 +1,583 @@ +"""Scoped MCP recovery playbook (#669). + +Operational recovery must prefer the *narrowest* action that can fix the +symptom. Full MCP / host restarts are last-resort rungs on a documented +ladder; the coordinator refuses those rungs unless a prior attempt log +shows narrower recoveries already failed (or break-glass is authorized). + +This module is pure classification and recommendation: + +* No network, filesystem, or process I/O. +* Never restarts anything. +* Narrow recovery *execution* is delegated to existing tools/docs (linked + per rung) — the playbook records which rung to try next and whether + escalation to a broad restart is allowed. + +Design lineage: umbrella #655, class matrix #663, coordinator #658, +auto-reconnect #584, stale-runtime #610, contamination #630, audit #665. +Vision #652 / roadmap #653. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from datetime import datetime, timezone +from enum import Enum +from typing import Any, Mapping, Sequence + +PLAYBOOK_VERSION = "1.0.0-issue-669" + +# Attempt outcomes that count as "tried and insufficient" for escalation. +INSUFFICIENT_OUTCOMES = frozenset( + { + "failed", + "insufficient", + "denied", + "unresolved", + "timeout", + "error", + } +) + +# Break-glass / operator override still records that the ladder was skipped. +OUTCOME_BREAK_GLASS = "break_glass" +OUTCOME_SUCCESS = "success" +OUTCOME_SKIPPED = "skipped" + + +class RecoveryAction(str, Enum): + """Ordered recovery ladder (narrow → broad).""" + + CLIENT_RECONNECT = "client_reconnect" + CAPABILITY_REFRESH = "capability_refresh" + SESSION_RECONNECT = "session_reconnect" + CONFIGURATION_RELOAD = "configuration_reload" + LEASE_RECOVERY = "lease_recovery" + WORKER_RESTART = "worker_restart" + ROLE_RUNTIME_RESTART = "role_runtime_restart" + CONNECTOR_RESTART = "connector_restart" + ROLLING_MCP_RESTART = "rolling_mcp_restart" + FULL_MCP_RESTART = "full_mcp_restart" + HOST_RESTART = "host_restart" + + +# Classes that require a prior narrow-attempt log (unless break-glass). +BROAD_RESTART_ACTIONS: frozenset[RecoveryAction] = frozenset( + { + RecoveryAction.ROLLING_MCP_RESTART, + RecoveryAction.FULL_MCP_RESTART, + RecoveryAction.HOST_RESTART, + } +) + +# Map #663 restart_class strings onto playbook actions. +RESTART_CLASS_TO_ACTION: dict[str, RecoveryAction] = { + "client_reconnect": RecoveryAction.CLIENT_RECONNECT, + "session_reconnect": RecoveryAction.SESSION_RECONNECT, + "configuration_reload": RecoveryAction.CONFIGURATION_RELOAD, + "worker_restart": RecoveryAction.WORKER_RESTART, + "role_runtime_restart": RecoveryAction.ROLE_RUNTIME_RESTART, + "connector_restart": RecoveryAction.CONNECTOR_RESTART, + "rolling_mcp_restart": RecoveryAction.ROLLING_MCP_RESTART, + "full_mcp_restart": RecoveryAction.FULL_MCP_RESTART, + "host_restart": RecoveryAction.HOST_RESTART, +} + + +@dataclass(frozen=True) +class RecoveryRung: + """One rung on the recovery ladder.""" + + action: RecoveryAction + rank: int + summary: str + # Existing implementation or explicit delegation target. + implementation: str + issue_links: tuple[str, ...] + self_service: bool + # Restart-class permission when this rung is requested via coordinator. + restart_class: str | None = None + + def as_dict(self) -> dict[str, Any]: + return { + "action": self.action.value, + "rank": self.rank, + "summary": self.summary, + "implementation": self.implementation, + "issue_links": list(self.issue_links), + "self_service": self.self_service, + "restart_class": self.restart_class, + } + + +# Canonical ladder. Rank 0 is narrowest. +RECOVERY_LADDER: tuple[RecoveryRung, ...] = ( + RecoveryRung( + RecoveryAction.CLIENT_RECONNECT, + 0, + "Reconnect the IDE/client MCP transport (EOF / transport flap).", + "Host auto-reconnect or explicit client reconnect; " + "docs/mcp-namespace-eof-recovery.md", + ("#584", "#655"), + True, + "client_reconnect", + ), + RecoveryRung( + RecoveryAction.CAPABILITY_REFRESH, + 1, + "Re-resolve task capability and clear stale permission context.", + "Delegated: gitea_resolve_task_capability + gitea_whoami " + "(no process change).", + ("#610", "#685", "#655"), + True, + None, + ), + RecoveryRung( + RecoveryAction.SESSION_RECONNECT, + 2, + "Rebind identity, workspace, and namespace for one session.", + "Delegated: gitea_get_runtime_context + explicit worktree_path " + "rebind (#618); docs/mcp-namespace-health.md", + ("#543", "#618", "#655"), + True, + "session_reconnect", + ), + RecoveryRung( + RecoveryAction.CONFIGURATION_RELOAD, + 3, + "Gracefully reload configuration without replacing the daemon.", + "restart_coordinator class configuration_reload; console " + "system.reload_namespace (#642).", + ("#642", "#663", "#655"), + False, + "configuration_reload", + ), + RecoveryRung( + RecoveryAction.LEASE_RECOVERY, + 4, + "Recover or rebind stale leases/locks without a process restart.", + "Delegated: issue lock recovery / lease lifecycle paths " + "(#702, #753, #790).", + ("#702", "#753", "#790", "#655"), + False, + None, + ), + RecoveryRung( + RecoveryAction.WORKER_RESTART, + 5, + "Restart one worker after its own lease and mutation scope drains.", + "restart_coordinator class worker_restart (#663).", + ("#663", "#655"), + False, + "worker_restart", + ), + RecoveryRung( + RecoveryAction.ROLE_RUNTIME_RESTART, + 6, + "Restart one role runtime and re-probe that namespace only.", + "restart_coordinator class role_runtime_restart; console " + "system.restart_namespace (#642).", + ("#642", "#663", "#655"), + False, + "role_runtime_restart", + ), + RecoveryRung( + RecoveryAction.CONNECTOR_RESTART, + 7, + "Restart one connector while unrelated runtimes stay available.", + "restart_coordinator class connector_restart (#663).", + ("#663", "#655"), + False, + "connector_restart", + ), + RecoveryRung( + RecoveryAction.ROLLING_MCP_RESTART, + 8, + "Drain/restart/verify one instance at a time (HA path).", + "restart_coordinator class rolling_mcp_restart; design #668.", + ("#668", "#663", "#655"), + False, + "rolling_mcp_restart", + ), + RecoveryRung( + RecoveryAction.FULL_MCP_RESTART, + 9, + "Full stable-control MCP process restart after verified full drain.", + "restart_coordinator class full_mcp_restart; requires attempt log " + "unless break-glass (#669).", + ("#658", "#661", "#663", "#669", "#655"), + False, + "full_mcp_restart", + ), + RecoveryRung( + RecoveryAction.HOST_RESTART, + 10, + "Host/infrastructure restart — broadest last-resort action.", + "restart_coordinator class host_restart; operator-owned.", + ("#663", "#669", "#655"), + False, + "host_restart", + ), +) + +_LADDER_BY_ACTION: dict[RecoveryAction, RecoveryRung] = { + rung.action: rung for rung in RECOVERY_LADDER +} + +# Symptom tokens → preferred first rung (decision tree, #663 lineage). +SYMPTOM_TO_FIRST_ACTION: dict[str, RecoveryAction] = { + "transport_eof": RecoveryAction.CLIENT_RECONNECT, + "client_closing_eof": RecoveryAction.CLIENT_RECONNECT, + "transport_flap": RecoveryAction.CLIENT_RECONNECT, + "namespace_disconnected": RecoveryAction.CLIENT_RECONNECT, + "stale_capability": RecoveryAction.CAPABILITY_REFRESH, + "permission_stale": RecoveryAction.CAPABILITY_REFRESH, + "runtime_reconnect_required": RecoveryAction.CAPABILITY_REFRESH, + "stale_runtime": RecoveryAction.SESSION_RECONNECT, + "worktree_unbound": RecoveryAction.SESSION_RECONNECT, + "namespace_unhealthy": RecoveryAction.SESSION_RECONNECT, + "config_drift": RecoveryAction.CONFIGURATION_RELOAD, + "profile_misbound": RecoveryAction.CONFIGURATION_RELOAD, + "stale_lease": RecoveryAction.LEASE_RECOVERY, + "dead_pid_lock": RecoveryAction.LEASE_RECOVERY, + "orphan_worktree": RecoveryAction.LEASE_RECOVERY, + "single_worker_stuck": RecoveryAction.WORKER_RESTART, + "role_runtime_dead": RecoveryAction.ROLE_RUNTIME_RESTART, + "connector_dead": RecoveryAction.CONNECTOR_RESTART, + "ha_instance_unhealthy": RecoveryAction.ROLLING_MCP_RESTART, + "daemon_corrupt": RecoveryAction.FULL_MCP_RESTART, + "full_process_deadlock": RecoveryAction.FULL_MCP_RESTART, + "host_unresponsive": RecoveryAction.HOST_RESTART, +} + + +def _utc_now() -> datetime: + return datetime.now(timezone.utc) + + +def resolve_action(value: RecoveryAction | str) -> RecoveryAction: + """Resolve a recovery action or fail closed for unknown values.""" + if isinstance(value, RecoveryAction): + return value + text = str(value or "").strip() + # Accept #663 restart_class aliases. + if text in RESTART_CLASS_TO_ACTION: + return RESTART_CLASS_TO_ACTION[text] + try: + return RecoveryAction(text) + except ValueError as exc: + raise ValueError( + f"unknown recovery action {value!r}; deny (fail closed, #669)" + ) from exc + + +def ladder_rank(action: RecoveryAction | str) -> int: + resolved = resolve_action(action) + return _LADDER_BY_ACTION[resolved].rank + + +def rung_for(action: RecoveryAction | str) -> RecoveryRung: + return _LADDER_BY_ACTION[resolve_action(action)] + + +def normalize_attempt(raw: Mapping[str, Any]) -> dict[str, Any] | None: + """Normalize one prior-recovery attempt record; return None if unusable.""" + if not isinstance(raw, Mapping): + return None + action_raw = raw.get("action") or raw.get("recovery_action") or raw.get( + "restart_class" + ) + if not action_raw: + return None + try: + action = resolve_action(str(action_raw)) + except ValueError: + return None + outcome = str( + raw.get("outcome") or raw.get("status") or raw.get("result") or "" + ).strip().lower() + if not outcome: + return None + recorded_at = raw.get("recorded_at") or raw.get("at") or raw.get("timestamp") + reason = str(raw.get("reason") or raw.get("detail") or "").strip() + actor = str(raw.get("actor") or raw.get("session_id") or "").strip() + return { + "action": action.value, + "outcome": outcome, + "reason": reason, + "actor": actor, + "recorded_at": recorded_at, + "rank": ladder_rank(action), + "raw": dict(raw), + } + + +def normalize_attempt_log( + attempts: Sequence[Mapping[str, Any]] | None, +) -> list[dict[str, Any]]: + """Return usable attempt records in ladder order.""" + out: list[dict[str, Any]] = [] + for raw in attempts or (): + norm = normalize_attempt(raw) + if norm is not None: + out.append(norm) + out.sort(key=lambda a: (a["rank"], str(a.get("recorded_at") or ""))) + return out + + +def narrower_insufficient_attempts( + attempts: Sequence[Mapping[str, Any]] | None, + *, + requested: RecoveryAction | str, +) -> list[dict[str, Any]]: + """Return prior attempts narrower than *requested* that were insufficient.""" + target_rank = ladder_rank(requested) + usable = [] + for attempt in normalize_attempt_log(attempts): + if attempt["rank"] >= target_rank: + continue + if attempt["outcome"] in INSUFFICIENT_OUTCOMES: + usable.append(attempt) + return usable + + +@dataclass(frozen=True) +class EscalationAssessment: + """Whether a requested broad recovery may proceed given the attempt log.""" + + requested_action: str + allowed: bool + require_attempt_log: bool + break_glass: bool + reasons: list[str] = field(default_factory=list) + qualifying_attempts: list[dict[str, Any]] = field(default_factory=list) + recommended_next: list[dict[str, Any]] = field(default_factory=list) + playbook_version: str = PLAYBOOK_VERSION + + def as_dict(self) -> dict[str, Any]: + return { + "playbook_version": self.playbook_version, + "requested_action": self.requested_action, + "allowed": self.allowed, + "require_attempt_log": self.require_attempt_log, + "break_glass": self.break_glass, + "reasons": list(self.reasons), + "qualifying_attempts": list(self.qualifying_attempts), + "recommended_next": list(self.recommended_next), + } + + +def assess_escalation( + requested: RecoveryAction | str, + *, + prior_recovery_attempts: Sequence[Mapping[str, Any]] | None = None, + break_glass: bool = False, +) -> EscalationAssessment: + """Gate broad restarts on a prior narrow-attempt log (#669 AC3). + + Narrow / mid-ladder actions do not require a prior attempt log. + ``full_mcp_restart``, ``host_restart``, and ``rolling_mcp_restart`` + require at least one *insufficient* narrower attempt unless + ``break_glass`` is true. + """ + action = resolve_action(requested) + require_log = action in BROAD_RESTART_ACTIONS + reasons: list[str] = [] + qualifying = narrower_insufficient_attempts( + prior_recovery_attempts, requested=action + ) + + if not require_log: + return EscalationAssessment( + requested_action=action.value, + allowed=True, + require_attempt_log=False, + break_glass=bool(break_glass), + reasons=["narrow recovery; attempt log not required"], + qualifying_attempts=qualifying, + recommended_next=[], + ) + + if break_glass: + return EscalationAssessment( + requested_action=action.value, + allowed=True, + require_attempt_log=True, + break_glass=True, + reasons=[ + "break-glass authorized; broad restart permitted without " + "narrow-attempt log (#669)" + ], + qualifying_attempts=qualifying, + recommended_next=[], + ) + + if qualifying: + return EscalationAssessment( + requested_action=action.value, + allowed=True, + require_attempt_log=True, + break_glass=False, + reasons=[ + f"{len(qualifying)} narrower recovery attempt(s) recorded as " + "insufficient; escalation permitted" + ], + qualifying_attempts=qualifying, + recommended_next=[], + ) + + # Deny: recommend the next untried narrow rung(s). + recommended = recommend_actions( + symptoms=(), + prior_recovery_attempts=prior_recovery_attempts, + max_actions=3, + ) + reasons.append( + f"{action.value} requires a prior attempt log of insufficient " + "narrower recoveries (or break-glass); none found — deny (fail " + "closed, #669)" + ) + return EscalationAssessment( + requested_action=action.value, + allowed=False, + require_attempt_log=True, + break_glass=False, + reasons=reasons, + qualifying_attempts=[], + recommended_next=recommended.get("recommended_actions") or [], + ) + + +def recommend_actions( + *, + symptoms: Sequence[str] = (), + prior_recovery_attempts: Sequence[Mapping[str, Any]] | None = None, + max_actions: int = 5, +) -> dict[str, Any]: + """Return ordered recommended recovery actions for the given symptoms. + + Soft mode (rollout): recommendations only — callers decide whether to + hard-gate. Hard mode for broad restarts is :func:`assess_escalation`. + """ + attempted_success = { + a["action"] + for a in normalize_attempt_log(prior_recovery_attempts) + if a["outcome"] == OUTCOME_SUCCESS + } + attempted_any = { + a["action"] for a in normalize_attempt_log(prior_recovery_attempts) + } + + first_actions: list[RecoveryAction] = [] + for symptom in symptoms: + key = str(symptom or "").strip().lower().replace(" ", "_").replace("-", "_") + mapped = SYMPTOM_TO_FIRST_ACTION.get(key) + if mapped is not None and mapped not in first_actions: + first_actions.append(mapped) + + # Default entry: client reconnect then walk the ladder. + if not first_actions: + first_actions = [RecoveryAction.CLIENT_RECONNECT] + + recommended: list[dict[str, Any]] = [] + seen: set[str] = set() + min_rank = min(ladder_rank(a) for a in first_actions) + + for rung in RECOVERY_LADDER: + if rung.rank < min_rank: + continue + if rung.action.value in attempted_success: + continue + if rung.action.value in seen: + continue + # Prefer rungs not yet attempted; still list previously-failed ones + # only if nothing else remains. + entry = rung.as_dict() + entry["already_attempted"] = rung.action.value in attempted_any + recommended.append(entry) + seen.add(rung.action.value) + if len(recommended) >= max(1, int(max_actions)): + break + + return { + "playbook_version": PLAYBOOK_VERSION, + "symptoms": [str(s) for s in symptoms], + "recommended_actions": recommended, + "ladder": [r.as_dict() for r in RECOVERY_LADDER], + "read_only": True, + "hard_gate_note": ( + "Broad restarts (rolling/full/host) still require " + "assess_escalation / coordinator attempt-log enforcement." + ), + } + + +def build_attempt_record( + action: RecoveryAction | str, + *, + outcome: str, + reason: str = "", + actor: str = "", + recorded_at: str | None = None, + extra: Mapping[str, Any] | None = None, +) -> dict[str, Any]: + """Build a durable-shaped attempt log entry for inventory/audit (#665).""" + resolved = resolve_action(action) + record = { + "action": resolved.value, + "outcome": str(outcome or "").strip().lower(), + "reason": str(reason or "").strip(), + "actor": str(actor or "").strip(), + "recorded_at": recorded_at or _utc_now().isoformat(), + "rank": ladder_rank(resolved), + "playbook_version": PLAYBOOK_VERSION, + } + if extra: + record["extra"] = dict(extra) + return record + + +def recovery_metrics( + attempts: Sequence[Mapping[str, Any]] | None, +) -> dict[str, Any]: + """Compute the fraction of recoveries that avoided full/host restart. + + A recovery *episode* is approximated as one attempt with + ``outcome=success``. Successes on non-broad rungs count as avoided full + restart; successes on full/host count as full-restart recoveries. + """ + norms = normalize_attempt_log(attempts) + successes = [a for a in norms if a["outcome"] == OUTCOME_SUCCESS] + broad_success = [ + a + for a in successes + if resolve_action(a["action"]) + in {RecoveryAction.FULL_MCP_RESTART, RecoveryAction.HOST_RESTART} + ] + avoided = [a for a in successes if a not in broad_success] + total = len(successes) + fraction_avoided = (len(avoided) / total) if total else None + return { + "playbook_version": PLAYBOOK_VERSION, + "attempts_total": len(norms), + "successes_total": total, + "successes_avoided_full_restart": len(avoided), + "successes_full_or_host_restart": len(broad_success), + "fraction_avoided_full_restart": fraction_avoided, + "insufficient_attempts": sum( + 1 for a in norms if a["outcome"] in INSUFFICIENT_OUTCOMES + ), + } + + +def ladder_document() -> dict[str, Any]: + """Machine-readable ladder for docs/tools inventory.""" + return { + "playbook_version": PLAYBOOK_VERSION, + "parent_issues": ["#655", "#652", "#653"], + "enforcement_issue": "#669", + "ladder": [r.as_dict() for r in RECOVERY_LADDER], + "broad_restart_actions": [a.value for a in sorted(BROAD_RESTART_ACTIONS, key=lambda x: x.value)], + "insufficient_outcomes": sorted(INSUFFICIENT_OUTCOMES), + "symptom_map": {k: v.value for k, v in sorted(SYMPTOM_TO_FIRST_ACTION.items())}, + } diff --git a/restart_coordinator.py b/restart_coordinator.py index 5452ad0..8134b44 100644 --- a/restart_coordinator.py +++ b/restart_coordinator.py @@ -1,4 +1,4 @@ -"""MCP restart coordinator and impact analysis (#658). +"""MCP restart coordinator and impact analysis (#658 / #669). Before any sanctioned MCP restart, a central coordinator must evaluate the live control-plane state — active sessions, leases/locks, in-flight issue/PR @@ -16,6 +16,9 @@ Design rules (mirrors the read-only posture of ``workflow_dashboard`` / a mutative apply path is a later child gated by a drain proof (non-goal here). * **Fail closed.** If the inventory is not explicitly complete, the verdict is ``unsafe`` / deny — an incomplete evaluation must never green-light a restart. +* **Narrow-first (#669).** Broad classes (rolling / full / host) require a + prior attempt log of insufficient narrower recoveries unless break-glass is + authorized. See :mod:`recovery_playbook`. * **No secrets.** Session ids, pids, and profiles are operational metadata, not credentials; nothing secret flows through this module. @@ -32,8 +35,9 @@ from enum import Enum from typing import Any, Mapping, Sequence import lease_lifecycle +import recovery_playbook -COORDINATOR_VERSION = "1.1.0-issue-663" +COORDINATOR_VERSION = "1.2.0-issue-669" # Restart verdicts. Exactly the three the acceptance criteria name. VERDICT_SAFE = "safe" @@ -349,6 +353,10 @@ class RestartImpactReport: counts: dict[str, int] audit_record: dict[str, Any] incomplete_reasons: list[str] = field(default_factory=list) + # #669 playbook escalation gate (attempt-log enforcement). + playbook_escalation: dict[str, Any] = field(default_factory=dict) + attempt_log_satisfied: bool = True + break_glass: bool = False def as_dict(self) -> dict[str, Any]: return { @@ -382,6 +390,9 @@ class RestartImpactReport: "prior_recovery_attempts": list(self.prior_recovery_attempts), "counts": dict(self.counts), "audit_record": dict(self.audit_record), + "playbook_escalation": dict(self.playbook_escalation), + "attempt_log_satisfied": self.attempt_log_satisfied, + "break_glass": self.break_glass, } @@ -496,6 +507,7 @@ def evaluate_restart_impact( target_session_id: str | None = None, target_role: str | None = None, target_connector: str | None = None, + break_glass: bool = False, ) -> RestartImpactReport: """Evaluate a proposed MCP restart and return an impact preview. @@ -584,6 +596,30 @@ def evaluate_restart_impact( dict(a) for a in (inventory.get("prior_recovery_attempts") or []) ] + # #669: broad restarts require a prior narrow-attempt log unless break-glass. + playbook_escalation: dict[str, Any] = {} + attempt_log_satisfied = True + if policy_enforced and resolved_class is not None: + try: + escalation = recovery_playbook.assess_escalation( + resolved_class.value, + prior_recovery_attempts=prior_recovery_attempts, + break_glass=bool(break_glass), + ) + playbook_escalation = escalation.as_dict() + attempt_log_satisfied = bool(escalation.allowed) + if not attempt_log_satisfied: + authorization_reasons.extend(list(escalation.reasons)) + except ValueError as exc: + # Unknown mapping should never happen for enum values; fail closed. + attempt_log_satisfied = False + playbook_escalation = { + "allowed": False, + "reasons": [str(exc)], + "playbook_version": recovery_playbook.PLAYBOOK_VERSION, + } + authorization_reasons.append(str(exc)) + session_impacts = [ _classify_session( s, @@ -682,6 +718,7 @@ def evaluate_restart_impact( and role_authorized and approval_satisfied and target_complete + and attempt_log_satisfied ) if policy_enforced and not authorization_ok: @@ -745,6 +782,7 @@ def evaluate_restart_impact( "affected_issues": len(affected_issues), "affected_prs": len(affected_prs), "prior_recovery_attempts": len(prior_recovery_attempts), + "attempt_log_satisfied": attempt_log_satisfied, } audit_record = { @@ -765,6 +803,9 @@ def evaluate_restart_impact( "allow_restart": allow_restart, "blast_radius": blast_radius, "counts": counts, + "attempt_log_satisfied": attempt_log_satisfied, + "break_glass": bool(break_glass), + "playbook_version": recovery_playbook.PLAYBOOK_VERSION, } return RestartImpactReport( @@ -804,4 +845,7 @@ def evaluate_restart_impact( counts=counts, audit_record=audit_record, incomplete_reasons=incomplete_reasons, + playbook_escalation=playbook_escalation, + attempt_log_satisfied=attempt_log_satisfied, + break_glass=bool(break_glass), ) diff --git a/tests/test_issue_886_apply_authorization_conjunction.py b/tests/test_issue_886_apply_authorization_conjunction.py index 6fa2888..5227f9c 100644 --- a/tests/test_issue_886_apply_authorization_conjunction.py +++ b/tests/test_issue_886_apply_authorization_conjunction.py @@ -35,6 +35,17 @@ BREAK_GLASS_ENV = "GITEA_BREAKGLASS_RESTART_AUTHORIZATION" QUIET_SESSIONS: list[dict] = [] QUIET_LEASES: list[dict] = [] +# #669: broad restarts need a prior narrow-attempt log (unless break-glass). +PRIOR_NARROW_ATTEMPTS_JSON = json.dumps( + [ + { + "action": "client_reconnect", + "outcome": "insufficient", + "reason": "still flapping after reconnect", + } + ] +) + class _FakeDB: """Minimal control-plane DB stand-in for the restart inventory.""" @@ -128,6 +139,7 @@ class TestConjunction(_RestartToolHarness): preview = self._call( role="operator", restart_class="full_mcp_restart", + prior_recovery_attempts_json=PRIOR_NARROW_ATTEMPTS_JSON, env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, ) self.assertTrue(preview["allow_restart"], @@ -136,6 +148,7 @@ class TestConjunction(_RestartToolHarness): result = self._call( role="operator", restart_class="full_mcp_restart", + prior_recovery_attempts_json=PRIOR_NARROW_ATTEMPTS_JSON, dry_run=False, drain_proof_json=self._clean_proof_for(preview), env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, @@ -342,6 +355,7 @@ class TestExistingPathsStillWork(_RestartToolHarness): result = self._call( role="operator", restart_class="full_mcp_restart", + prior_recovery_attempts_json=PRIOR_NARROW_ATTEMPTS_JSON, dry_run=False, env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, ) diff --git a/tests/test_recovery_playbook.py b/tests/test_recovery_playbook.py new file mode 100644 index 0000000..57e5005 --- /dev/null +++ b/tests/test_recovery_playbook.py @@ -0,0 +1,217 @@ +"""Unit tests for the scoped recovery playbook (#669).""" + +from __future__ import annotations + +import recovery_playbook as rp +import restart_coordinator as rc + + +def test_ladder_covers_eleven_ordered_rungs(): + ranks = [r.rank for r in rp.RECOVERY_LADDER] + assert ranks == list(range(len(rp.RECOVERY_LADDER))) + assert len(rp.RECOVERY_LADDER) == 11 + assert rp.RECOVERY_LADDER[0].action is rp.RecoveryAction.CLIENT_RECONNECT + assert rp.RECOVERY_LADDER[-1].action is rp.RecoveryAction.HOST_RESTART + + +def test_ladder_document_links_parent_issues(): + doc = rp.ladder_document() + assert "#655" in doc["parent_issues"] + assert "#652" in doc["parent_issues"] + assert "#653" in doc["parent_issues"] + assert doc["enforcement_issue"] == "#669" + assert "full_mcp_restart" in doc["broad_restart_actions"] + + +def test_recommend_transport_eof_starts_at_client_reconnect(): + plan = rp.recommend_actions(symptoms=["transport_eof"]) + assert plan["recommended_actions"][0]["action"] == "client_reconnect" + assert plan["recommended_actions"][0]["issue_links"] + + +def test_recommend_skips_successful_prior_attempts(): + attempts = [ + rp.build_attempt_record( + "client_reconnect", outcome="success", reason="reconnected" + ) + ] + plan = rp.recommend_actions( + symptoms=["transport_eof"], prior_recovery_attempts=attempts + ) + actions = [a["action"] for a in plan["recommended_actions"]] + assert "client_reconnect" not in actions + assert actions[0] == "capability_refresh" + + +def test_escalation_denied_without_attempt_log(): + result = rp.assess_escalation("full_mcp_restart", prior_recovery_attempts=[]) + assert result.allowed is False + assert result.require_attempt_log is True + assert any("#669" in r for r in result.reasons) + assert result.recommended_next # soft recommendations still provided + + +def test_escalation_allowed_after_insufficient_narrower(): + attempts = [ + rp.build_attempt_record( + "client_reconnect", + outcome="insufficient", + reason="still flapping", + ), + rp.build_attempt_record( + "session_reconnect", + outcome="failed", + reason="namespace still dead", + ), + ] + result = rp.assess_escalation( + "full_mcp_restart", prior_recovery_attempts=attempts + ) + assert result.allowed is True + assert len(result.qualifying_attempts) == 2 + + +def test_escalation_break_glass_bypasses_attempt_log(): + result = rp.assess_escalation( + "host_restart", prior_recovery_attempts=[], break_glass=True + ) + assert result.allowed is True + assert result.break_glass is True + + +def test_narrow_action_does_not_require_attempt_log(): + result = rp.assess_escalation( + "client_reconnect", prior_recovery_attempts=[] + ) + assert result.allowed is True + assert result.require_attempt_log is False + + +def test_same_rank_attempt_does_not_qualify_for_escalation(): + attempts = [ + rp.build_attempt_record( + "full_mcp_restart", outcome="failed", reason="already failed full" + ) + ] + result = rp.assess_escalation( + "full_mcp_restart", prior_recovery_attempts=attempts + ) + assert result.allowed is False + + +def test_success_outcome_does_not_qualify_for_escalation(): + attempts = [ + rp.build_attempt_record( + "client_reconnect", outcome="success", reason="fixed" + ) + ] + result = rp.assess_escalation( + "full_mcp_restart", prior_recovery_attempts=attempts + ) + assert result.allowed is False + + +def test_recovery_metrics_fraction_avoided(): + attempts = [ + rp.build_attempt_record("client_reconnect", outcome="success"), + rp.build_attempt_record("session_reconnect", outcome="success"), + rp.build_attempt_record("full_mcp_restart", outcome="success"), + ] + metrics = rp.recovery_metrics(attempts) + assert metrics["successes_total"] == 3 + assert metrics["successes_avoided_full_restart"] == 2 + assert metrics["successes_full_or_host_restart"] == 1 + assert abs(metrics["fraction_avoided_full_restart"] - (2 / 3)) < 1e-9 + + +def test_coordinator_denies_full_restart_without_attempt_log(): + inv = { + "inventory_complete": True, + "sessions": [], + "leases": [], + "prior_recovery_attempts": [], + } + report = rc.evaluate_restart_impact( + inv, + restart_class=rc.RestartClass.FULL_MCP_RESTART, + requester_role="controller", + requester_permissions=rc.permissions_for_role("controller"), + controller_approved=True, + operator_authorized=True, + ) + assert report.allow_restart is False + assert report.attempt_log_satisfied is False + assert report.verdict == rc.VERDICT_UNSAFE + blob = " ".join(report.reasons + report.authorization_reasons) + assert "#669" in blob or "attempt log" in blob + + +def test_coordinator_allows_full_restart_with_attempt_log(): + inv = { + "inventory_complete": True, + "sessions": [], + "leases": [], + "prior_recovery_attempts": [ + { + "action": "client_reconnect", + "outcome": "insufficient", + "reason": "still broken", + } + ], + } + report = rc.evaluate_restart_impact( + inv, + restart_class=rc.RestartClass.FULL_MCP_RESTART, + requester_role="controller", + requester_permissions=rc.permissions_for_role("controller"), + controller_approved=True, + operator_authorized=True, + ) + assert report.attempt_log_satisfied is True + assert report.allow_restart is True + assert report.verdict == rc.VERDICT_SAFE + + +def test_coordinator_break_glass_allows_without_log(): + inv = { + "inventory_complete": True, + "sessions": [], + "leases": [], + "prior_recovery_attempts": [], + } + report = rc.evaluate_restart_impact( + inv, + restart_class=rc.RestartClass.FULL_MCP_RESTART, + requester_role="controller", + requester_permissions=rc.permissions_for_role("controller"), + controller_approved=True, + operator_authorized=True, + break_glass=True, + ) + assert report.break_glass is True + assert report.attempt_log_satisfied is True + assert report.allow_restart is True + + +def test_coordinator_client_reconnect_unaffected(): + inv = { + "inventory_complete": True, + "sessions": [], + "leases": [], + "prior_recovery_attempts": [], + } + report = rc.evaluate_restart_impact( + inv, + restart_class=rc.RestartClass.CLIENT_RECONNECT, + requester_role="author", + requester_permissions=rc.permissions_for_role("author"), + ) + assert report.attempt_log_satisfied is True + assert report.allow_restart is True + + +def test_restart_class_alias_accepted(): + assert ( + rp.resolve_action("full_mcp_restart") + is rp.RecoveryAction.FULL_MCP_RESTART + ) -- 2.43.7