From 3428fb419065fd47518c1673272d9dc472b942bd Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Fri, 24 Jul 2026 00:57:33 -0400 Subject: [PATCH] feat(mcp-health): inventory and guard MCP restart/reload/kill paths (#657) Enumerate every code/script/host path that can restart, reload, reconnect, kill, or force-recreate an MCP process, classify each, and link it to the guard that constrains it. - mcp_restart_paths.py: machine-readable registry (single source of truth) with classifications (sanctioned_narrow / guarded_fail_closed / forbidden / removed / host_residual) plus fail-closed guards: * assert_restart_attempt_registered() -- unknown restart attempts fail closed * assert_no_daemon_self_replacement() -- daemon never os.execv/os.kill/os._exit itself (source-tree scan; comment/docstring mentions ignored) * assert_auto_restart_helper_absent() -- keeps the #685-removed _trigger_mcp_auto_restart from returning * assert_registry_wellformed() -- every path classified, guarded, referenced - docs/mcp-restart-path-inventory.md: complete inventory table linked from #655; documents residual host behaviors (/mcp reconnect) and rollout. - tests/test_mcp_restart_paths.py: 17 tests -- registry well-formedness, unknown-attempt fail-closed, daemon-self-replacement scan (with injected violation + comment/docstring negative case), legacy-helper-removed regression, pkill-stays-contamination (#630), and doc/module lock-step. No behavior change to existing modules; regression assertions codify invariants that already hold (per #657 flag-free-before-hard-block rollout). Links #652 #653 #655 #656. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/mcp-restart-path-inventory.md | 90 ++++++ mcp_restart_paths.py | 475 +++++++++++++++++++++++++++++ tests/test_mcp_restart_paths.py | 146 +++++++++ 3 files changed, 711 insertions(+) create mode 100644 docs/mcp-restart-path-inventory.md create mode 100644 mcp_restart_paths.py create mode 100644 tests/test_mcp_restart_paths.py diff --git a/docs/mcp-restart-path-inventory.md b/docs/mcp-restart-path-inventory.md new file mode 100644 index 0000000..c880269 --- /dev/null +++ b/docs/mcp-restart-path-inventory.md @@ -0,0 +1,90 @@ +# MCP restart / reload / kill path inventory (#657) + +Complete inventory of every code, script, and host path that can **restart, +reload, reconnect, kill, or force-recreate** an MCP process in this project, +with each path classified and linked to the guard that constrains it. + +This document is the human-readable companion to the machine-readable registry +in [`mcp_restart_paths.py`](../mcp_restart_paths.py). The two are kept in +lock-step by [`tests/test_mcp_restart_paths.py`](../tests/test_mcp_restart_paths.py): +every `path_id` below must appear in this file, and the source guards are run +against the live tree. + +Roadmap linkage: this inventory is the enumeration step of the restart +governance work — parent **#655**, restart-governance ADR **#656**, vision +**#652**, roadmap **#653**. Related detection/guard work: master-advance +staleness **#591**/**#420**, side-effect-free resolver **#685**, transport flap +**#584**, manual-kill contamination **#630**. + +## Classifications + +| Classification | Meaning | +|---|---| +| `sanctioned_narrow_recovery` | One-shot, safe-by-construction recovery that never targets the running daemon. | +| `guarded_fail_closed` | Detects a restart-requiring condition, then fails mutations closed and emits reconnect guidance. Never self-restarts. | +| `forbidden` | A workflow-safety violation; where an LLM tool could invoke it, it is marked contamination. | +| `removed` | A previously-existing unguarded restart primitive that has been deleted; a regression guard keeps it absent. | +| `host_residual` | Behavior owned by the host/IDE, outside this process's control. Documented, not code-guarded here. | + +## The rule + +**No component may perform an unguarded full restart of the MCP daemon.** The +in-process daemon (`gitea_mcp_server.py`, `mcp_server.py`, +`role_session_router.py`) must never replace or terminate its own process: +replacing the process after the host has wired up the stdio pipes desyncs the +JSON-RPC transport (observed with Antigravity/Cascade hosts). Recovery is owned +by the host/operator via a client reconnect — the daemon only ever *detects* +and *fails closed*. + +## Inventory + +| path_id | Classification | Mechanism | Guard | Refs | +|---|---|---|---|---| +| `cli_venv_bootstrap_execv` | sanctioned_narrow_recovery | CLI wrapper scripts re-exec into `venv/bin/python3` via `os.execv`, guarded by `sys.executable != venv_python`. | One-shot pre-import bootstrap; runs before any MCP transport exists and only when not already on the venv interpreter; idempotent guard prevents a re-exec loop. | #657 | +| `daemon_self_replacement` | forbidden | The daemon replacing/terminating its own process (`os.execv`/`os.kill`/`os._exit`) to reload code. | Forbidden by design; enforced against the source tree by `assert_no_daemon_self_replacement()`. | #657, #584 | +| `legacy_auto_restart_helper` | removed | A helper (`_trigger_mcp_auto_restart`) that actively restarted the server from the read-only resolver path. | Removed in #685; kept absent by `assert_auto_restart_helper_absent()`. | #685, #657 | +| `config_touch_reload` | removed | Touching (utime) the MCP client config to make the host reload the server. | Removed from the resolver in #685: stale detection is report-only, never mutating config, spawning threads, or calling `os._exit`. | #685, #657 | +| `master_advance_auto_restart` | guarded_fail_closed | On-disk master advancing past the running code. | `master_parity_gate` captures startup parity and blocks mutations while stale, emitting restart guidance; the process never self-restarts. | #420, #591, #657 | +| `stale_runtime_resolver_reconnect` | guarded_fail_closed | The capability resolver detecting a stale serving process. | Report-only (#685): returns `restart_required`/`stop_required` and an exact reconnect action; no restart, thread, config touch, or `os._exit`. | #685, #657 | +| `manual_daemon_kill` | forbidden | Shell kills of the daemon: `pkill -f mcp_server.py`, `killall`, broad `pkill -f python` sweeps, or `kill ` of a daemon pid. | Forbidden (#630): `runtime_recovery_guard` classifies these as contamination and `gitea_record_daemon_process_kill_attempt` writes a durable marker that fails later mutations closed. Operator maintenance authorization is read only from the environment. | #630, #657 | +| `conflict_marker_infra_stop` | guarded_fail_closed | The daemon entrypoint scans for unresolved merge-conflict markers at startup and stops (`sys.exit(1)`). | Fail-closed startup stop, not a restart: the process exits and waits for the operator to resolve conflicts and relaunch; never loops. | #657 | +| `ide_client_reconnect` | host_residual | A manual `/mcp reconnect` (or equivalent host action) that recreates the MCP client connection. | Outside this process's control; the sanctioned recovery the gates point operators toward. No in-process code initiates it. | #584, #656, #657 | +| `profile_switch_runtime` | sanctioned_narrow_recovery | Switching the active execution profile at runtime (dynamic-profile mode). | In-process and restart-free: `runtime_switching_supported` is true, so a switch rebinds capability without recreating the process. | #656, #657 | + +## Guards enforced in CI + +`tests/test_mcp_restart_paths.py` asserts, against the live source tree: + +1. **Registry well-formedness** — every path has a valid classification, a + non-empty guard description, references, and locations; ids are unique; all + five classifications are represented. +2. **Unknown restart attempts fail closed** — + `assert_restart_attempt_registered()` raises `UnknownRestartPathError` for + any path id not in this inventory, so a novel/unnamed restart primitive + cannot slip through silently. +3. **Daemon never self-replaces** — `assert_no_daemon_self_replacement()` scans + the daemon modules for `os.execv`/`os.kill`/`os._exit`/`os.abort` calls + (comment/docstring mentions are ignored) and finds none. +4. **Legacy helper stays removed** — `assert_auto_restart_helper_absent()` + confirms `_trigger_mcp_auto_restart` has not returned. +5. **pkill stays forbidden** — a daemon `pkill` command still classifies as + contamination via `runtime_recovery_guard`. + +## Residual host behaviors (outside process control) + +* `/mcp reconnect` in the IDE/host — the sanctioned recovery for stale-runtime, + transport-flap (#584), and worktree-binding conditions. The daemon can only + emit guidance toward it. +* Host-level process management (the operator relaunching the daemon after a + fail-closed stop, or after resolving merge conflicts). + +These are documented rather than code-guarded because the process cannot +observe or gate them from inside itself. + +## Rollout + +Per #657, guards are introduced flag-free as **regression assertions** (they +codify invariants that already hold) before any hard runtime block is layered +on. When the restart coordinator (#655/#656) lands, registered paths gain a +coordinator token/capability check; unregistered attempts already fail closed +today via `assert_restart_attempt_registered()`. diff --git a/mcp_restart_paths.py b/mcp_restart_paths.py new file mode 100644 index 0000000..087fe72 --- /dev/null +++ b/mcp_restart_paths.py @@ -0,0 +1,475 @@ +"""Inventory and fail-closed guards for MCP restart/reload/kill paths (#657). + +Single source of truth enumerating every code/script/doc path that can +restart, reload, reconnect, kill, or force-recreate an MCP process. Each path +is classified and linked to the guard that constrains it. The companion +human-readable inventory lives in ``docs/mcp-restart-path-inventory.md`` and is +kept in lock-step with this module by ``tests/test_mcp_restart_paths.py``. + +Design intent (aligns with #655 restart-coordinator roadmap): + +* **No unguarded full restart.** The in-process MCP daemon + (``gitea_mcp_server.py`` / ``mcp_server.py`` / ``role_session_router.py``) + must never replace or kill its own process — replacing the process after the + host wired up the stdio pipes desyncs the JSON-RPC transport (observed with + Antigravity/Cascade hosts). ``assert_no_daemon_self_replacement`` enforces + this against the live source tree. +* **No legacy auto-restart helper.** ``_trigger_mcp_auto_restart`` was removed + when the stale-runtime resolver became side-effect free (#685); + ``assert_auto_restart_helper_absent`` keeps it removed. +* **Unknown restart attempts fail closed.** LLM tools must route any restart + intent through a *registered* path. ``assert_restart_attempt_registered`` + raises ``UnknownRestartPathError`` for anything not in this inventory. +* **pkill stays forbidden (#630).** Manual daemon kills are classified as + contamination by :mod:`runtime_recovery_guard`; this module records that path + and the test asserts the classification still holds. + +This module performs no restarts, spawns no threads, and touches no config or +process state. It is pure inventory + read-only source assertions. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from pathlib import Path +from typing import Iterable + +# --- Classifications ------------------------------------------------------- + +#: A narrow, one-shot recovery that is safe by construction (e.g. a CLI wrapper +#: re-execing into the venv interpreter before importing anything, or an +#: in-process profile switch). Never targets the running MCP daemon process. +CLASS_SANCTIONED_NARROW = "sanctioned_narrow_recovery" + +#: The path detects a condition that would require a restart, then *fails +#: closed* on mutations and emits restart/reconnect guidance. It never restarts +#: the process itself (recovery is owned by the host/operator). +CLASS_GUARDED_FAIL_CLOSED = "guarded_fail_closed" + +#: The path is forbidden. Attempting it is a workflow-safety violation and, +#: where an LLM tool could invoke it, is marked as contamination. +CLASS_FORBIDDEN = "forbidden" + +#: A previously-existing unguarded restart primitive that has been deleted. A +#: regression guard keeps it absent. +CLASS_REMOVED = "removed" + +#: Behavior that lives in the host/IDE and is outside this process's control +#: (e.g. a manual ``/mcp reconnect``). Documented, not code-guarded here. +CLASS_HOST_RESIDUAL = "host_residual" + +VALID_CLASSIFICATIONS = frozenset( + { + CLASS_SANCTIONED_NARROW, + CLASS_GUARDED_FAIL_CLOSED, + CLASS_FORBIDDEN, + CLASS_REMOVED, + CLASS_HOST_RESIDUAL, + } +) + +#: The in-process MCP daemon modules. These must never self-replace/self-kill. +DAEMON_MODULES = ( + "gitea_mcp_server.py", + "mcp_server.py", + "role_session_router.py", +) + +#: The legacy auto-restart helper removed in #685. Must stay removed. +LEGACY_AUTO_RESTART_HELPER = "_trigger_mcp_auto_restart" + +#: Call patterns that would let the daemon replace or terminate its own +#: process. Matched as calls (trailing ``(``) so prose/docstring mentions such +#: as "we do NOT os.execv() here" or "never calls ``os._exit``" do not trip the +#: scanner (comment lines are stripped first regardless). +DAEMON_SELF_REPLACEMENT_PRIMITIVES = ( + "os.execv(", + "os.execve(", + "os.execvp(", + "os.execvpe(", + "os.kill(", + "os.killpg(", + "os._exit(", + "os.abort(", +) + + +@dataclass(frozen=True) +class RestartPath: + """One classified restart/reload/kill path in the inventory.""" + + path_id: str + title: str + mechanism: str + classification: str + guard: str + locations: tuple[str, ...] + references: tuple[str, ...] + residual_host: bool = False + notes: str = "" + + +class UnknownRestartPathError(RuntimeError): + """Raised when a restart attempt is not a registered, classified path.""" + + +# --- The inventory --------------------------------------------------------- + +_RESTART_PATHS: tuple[RestartPath, ...] = ( + RestartPath( + path_id="cli_venv_bootstrap_execv", + title="CLI wrapper venv re-exec", + mechanism=( + "Standalone CLI scripts re-exec into venv/bin/python3 via os.execv " + "at import top, guarded by `sys.executable != venv_python`." + ), + classification=CLASS_SANCTIONED_NARROW, + guard=( + "One-shot, pre-import bootstrap; runs before any MCP transport " + "exists and only when not already on the venv interpreter, so it " + "cannot desync a live daemon. Idempotent guard condition prevents " + "a re-exec loop." + ), + locations=( + "create_pr.py", + "create_issue.py", + "close_issue.py", + "merge_pr.py", + "review_pr.py", + "edit_pr.py", + "delete_branch.py", + "mark_issue.py", + "manage_labels.py", + "list_issues.py", + "list_prs.py", + ), + references=("#657",), + ), + RestartPath( + path_id="daemon_self_replacement", + title="MCP daemon self-replacement", + mechanism=( + "The in-process MCP daemon replacing/terminating its own process " + "(os.execv/os.kill/os._exit) to reload code." + ), + classification=CLASS_FORBIDDEN, + guard=( + "Forbidden by design: replacing the process after the host wired " + "up stdio desyncs JSON-RPC (Antigravity/Cascade). Enforced against " + "the source tree by assert_no_daemon_self_replacement()." + ), + locations=("gitea_mcp_server.py:~155 (decision comment)",) + DAEMON_MODULES, + references=("#657", "#584"), + ), + RestartPath( + path_id="legacy_auto_restart_helper", + title="Legacy _trigger_mcp_auto_restart helper", + mechanism=( + "A helper that actively restarted the MCP server from the " + "read-only resolver path." + ), + classification=CLASS_REMOVED, + guard=( + "Removed in #685 when the resolver became side-effect free. Kept " + "absent by assert_auto_restart_helper_absent()." + ), + locations=("gitea_mcp_server.py", "mcp_server.py"), + references=("#685", "#657"), + ), + RestartPath( + path_id="config_touch_reload", + title="MCP client config-touch reload", + mechanism=( + "Touching (utime) the MCP client config file to make the host " + "reload/recreate the server process." + ), + classification=CLASS_REMOVED, + guard=( + "Removed from the resolver in #685: stale-runtime detection is " + "report-only and never mutates client config, spawns threads, or " + "calls os._exit." + ), + locations=("gitea_mcp_server.py (resolve_task_capability)",), + references=("#685", "#657"), + ), + RestartPath( + path_id="master_advance_auto_restart", + title="Master-advance staleness gate", + mechanism=( + "On-disk master advancing past the running code. The master-parity " + "gate detects it and fails mutations closed with restart guidance." + ), + classification=CLASS_GUARDED_FAIL_CLOSED, + guard=( + "Detect + fail closed only; the process never self-restarts. " + "master_parity_gate captures startup parity and blocks mutations " + "while stale, emitting restart/reconnect guidance." + ), + locations=( + "master_parity_gate.py", + "gitea_mcp_server.py (gitea_assess_master_parity)", + ), + references=("#420", "#591", "#657"), + ), + RestartPath( + path_id="stale_runtime_resolver_reconnect", + title="Stale-runtime resolver reconnect guidance", + mechanism=( + "The capability resolver detecting a stale serving process and " + "reporting restart_required/stop_required for a client reconnect." + ), + classification=CLASS_GUARDED_FAIL_CLOSED, + guard=( + "Report-only (#685): returns restart_required/stop_required and an " + "exact_safe_next_action pointing at IDE/client reconnect; performs " + "no restart, thread spawn, config touch, or os._exit." + ), + locations=("gitea_mcp_server.py (gitea_resolve_task_capability)",), + references=("#685", "#657"), + ), + RestartPath( + path_id="manual_daemon_kill", + title="Manual daemon kill (pkill/killall/kill)", + mechanism=( + "Shell kills of the MCP daemon: `pkill -f mcp_server.py`, " + "`killall`, broad `pkill -f python` sweeps, or `kill ` of a " + "daemon pid." + ), + classification=CLASS_FORBIDDEN, + guard=( + "Forbidden (#630): runtime_recovery_guard classifies these as " + "contamination and gitea_record_daemon_process_kill_attempt writes " + "a durable marker that fails subsequent mutations closed. Operator " + "maintenance authorization is read only from the environment, not " + "from a tool argument." + ), + locations=( + "runtime_recovery_guard.py", + "gitea_mcp_server.py (gitea_record_daemon_process_kill_attempt)", + ), + references=("#630", "#657"), + ), + RestartPath( + path_id="conflict_marker_infra_stop", + title="Startup conflict-marker infra stop", + mechanism=( + "The daemon entrypoint scans for unresolved merge-conflict markers " + "at startup and stops (sys.exit(1)) if found." + ), + classification=CLASS_GUARDED_FAIL_CLOSED, + guard=( + "Fail-closed startup stop, not a restart: the process exits and " + "waits for the operator to resolve conflicts and relaunch. Never " + "self-restarts or loops." + ), + locations=("mcp_server.py (check_conflict_markers)",), + references=("#657",), + ), + RestartPath( + path_id="ide_client_reconnect", + title="Host/IDE MCP reconnect", + mechanism=( + "A manual `/mcp reconnect` (or equivalent host action) that the " + "IDE performs to recreate the MCP client connection." + ), + classification=CLASS_HOST_RESIDUAL, + guard=( + "Outside this process's control. It is the sanctioned recovery the " + "gates point operators toward; documented as residual host " + "behavior. No in-process code initiates it." + ), + locations=("host/IDE",), + references=("#584", "#656", "#657"), + residual_host=True, + ), + RestartPath( + path_id="profile_switch_runtime", + title="Runtime profile switch", + mechanism=( + "Switching the active execution profile at runtime " + "(dynamic-profile mode)." + ), + classification=CLASS_SANCTIONED_NARROW, + guard=( + "In-process and restart-free: runtime_switching_supported is true, " + "so a profile switch rebinds capability without recreating the " + "process. No restart primitive is invoked." + ), + locations=("gitea_mcp_server.py (gitea_activate_profile)",), + references=("#656", "#657"), + ), +) + +_BY_ID: dict[str, RestartPath] = {p.path_id: p for p in _RESTART_PATHS} + + +# --- Read-only accessors --------------------------------------------------- + + +def iter_restart_paths() -> tuple[RestartPath, ...]: + """Return the full inventory as an immutable tuple.""" + + return _RESTART_PATHS + + +def restart_path_ids() -> frozenset[str]: + """Return the set of registered path ids.""" + + return frozenset(_BY_ID) + + +def get_restart_path(path_id: str) -> RestartPath: + """Return the registered path, or raise :class:`UnknownRestartPathError`.""" + + try: + return _BY_ID[path_id] + except KeyError as exc: + raise UnknownRestartPathError( + f"unknown restart path id {path_id!r}; not in the #657 inventory" + ) from exc + + +def paths_by_classification(classification: str) -> tuple[RestartPath, ...]: + """Return all registered paths with the given classification.""" + + if classification not in VALID_CLASSIFICATIONS: + raise ValueError(f"unknown classification {classification!r}") + return tuple(p for p in _RESTART_PATHS if p.classification == classification) + + +def assert_restart_attempt_registered(path_id: str) -> RestartPath: + """Fail closed unless ``path_id`` is a registered, classified restart path. + + LLM tools that intend to trigger any restart/reload/reconnect must name a + registered path so an unknown/novel restart primitive cannot slip through + silently. Forbidden and removed paths are registered too — this only + asserts the attempt is *known*, not that it is *permitted*; callers must + still honor the classification. + """ + + return get_restart_path(path_id) + + +def assert_registry_wellformed() -> None: + """Validate the inventory's own invariants (fail closed on drift).""" + + seen: set[str] = set() + for path in _RESTART_PATHS: + if path.path_id in seen: + raise ValueError(f"duplicate restart path id {path.path_id!r}") + seen.add(path.path_id) + if path.classification not in VALID_CLASSIFICATIONS: + raise ValueError( + f"{path.path_id!r} has invalid classification " + f"{path.classification!r}" + ) + if not path.guard.strip(): + raise ValueError(f"{path.path_id!r} is missing a guard description") + if not path.references: + raise ValueError(f"{path.path_id!r} is missing references") + if not path.locations: + raise ValueError(f"{path.path_id!r} is missing locations") + if path.classification == CLASS_HOST_RESIDUAL and not path.residual_host: + raise ValueError( + f"{path.path_id!r} is host_residual but residual_host is False" + ) + + +# --- Source-tree guards ---------------------------------------------------- + + +def _repo_root(root: str | os.PathLike[str] | None = None) -> Path: + if root is not None: + return Path(root) + return Path(__file__).resolve().parent + + +def _iter_code_lines(text: str) -> Iterable[tuple[int, str]]: + """Yield (1-based lineno, line) for lines that are not full-line comments.""" + + for lineno, line in enumerate(text.splitlines(), start=1): + if line.lstrip().startswith("#"): + continue + yield lineno, line + + +def scan_daemon_self_replacement( + root: str | os.PathLike[str] | None = None, +) -> list[dict[str, object]]: + """Return violations where a daemon module could self-replace/self-kill. + + Scans :data:`DAEMON_MODULES` for calls in + :data:`DAEMON_SELF_REPLACEMENT_PRIMITIVES`. Full-line comments are ignored, + and only call forms (with a trailing ``(``) match, so decision comments and + docstrings that merely mention the primitives do not produce false hits. + """ + + repo = _repo_root(root) + violations: list[dict[str, object]] = [] + for module in DAEMON_MODULES: + path = repo / module + if not path.exists(): + continue + text = path.read_text(encoding="utf-8", errors="replace") + for lineno, line in _iter_code_lines(text): + for primitive in DAEMON_SELF_REPLACEMENT_PRIMITIVES: + if primitive in line: + violations.append( + { + "module": module, + "line": lineno, + "primitive": primitive, + "text": line.strip(), + } + ) + return violations + + +def assert_no_daemon_self_replacement( + root: str | os.PathLike[str] | None = None, +) -> None: + """Fail closed if any daemon module can restart/kill its own process.""" + + violations = scan_daemon_self_replacement(root) + if violations: + rendered = "; ".join( + f"{v['module']}:{v['line']} {v['primitive']}" for v in violations + ) + raise AssertionError( + "MCP daemon must never self-replace/self-kill (#657); found: " + f"{rendered}" + ) + + +def scan_auto_restart_helper( + root: str | os.PathLike[str] | None = None, +) -> list[dict[str, object]]: + """Return occurrences of a *definition* of the legacy auto-restart helper.""" + + repo = _repo_root(root) + needle = f"def {LEGACY_AUTO_RESTART_HELPER}" + hits: list[dict[str, object]] = [] + for module in DAEMON_MODULES: + path = repo / module + if not path.exists(): + continue + text = path.read_text(encoding="utf-8", errors="replace") + for lineno, line in _iter_code_lines(text): + if needle in line: + hits.append({"module": module, "line": lineno}) + return hits + + +def assert_auto_restart_helper_absent( + root: str | os.PathLike[str] | None = None, +) -> None: + """Fail closed if the removed ``_trigger_mcp_auto_restart`` reappears.""" + + hits = scan_auto_restart_helper(root) + if hits: + rendered = "; ".join(f"{h['module']}:{h['line']}" for h in hits) + raise AssertionError( + f"{LEGACY_AUTO_RESTART_HELPER} was removed in #685 and must not " + f"return (#657); found definition at: {rendered}" + ) diff --git a/tests/test_mcp_restart_paths.py b/tests/test_mcp_restart_paths.py new file mode 100644 index 0000000..cf0c5eb --- /dev/null +++ b/tests/test_mcp_restart_paths.py @@ -0,0 +1,146 @@ +"""Tests for the MCP restart-path inventory and guards (#657). + +Covers: +* the registry is well-formed and every path is classified; +* unknown restart attempts fail closed (AC "fail closed on unknown restart"); +* the previously-unguarded full-restart primitives stay guarded/absent + against the real source tree (AC "tests for at least one previously + unguarded path"); +* pkill of the daemon is still classified as contamination (#630, AC3); +* the inventory doc and module stay in lock-step. +""" + +import os +import tempfile +import unittest +from pathlib import Path + +import mcp_restart_paths as rp +import runtime_recovery_guard + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +DOC_PATH = os.path.join(REPO_ROOT, "docs", "mcp-restart-path-inventory.md") + + +class TestRegistryWellformed(unittest.TestCase): + def test_registry_is_wellformed(self): + # Must not raise. + rp.assert_registry_wellformed() + + def test_every_path_has_valid_classification(self): + for path in rp.iter_restart_paths(): + self.assertIn(path.classification, rp.VALID_CLASSIFICATIONS) + self.assertTrue(path.guard.strip(), path.path_id) + self.assertTrue(path.references, path.path_id) + self.assertTrue(path.locations, path.path_id) + + def test_ids_are_unique(self): + ids = [p.path_id for p in rp.iter_restart_paths()] + self.assertEqual(len(ids), len(set(ids))) + + def test_covers_every_classification(self): + present = {p.classification for p in rp.iter_restart_paths()} + self.assertEqual(present, set(rp.VALID_CLASSIFICATIONS)) + + +class TestUnknownAttemptFailsClosed(unittest.TestCase): + def test_unknown_path_raises(self): + with self.assertRaises(rp.UnknownRestartPathError): + rp.assert_restart_attempt_registered("totally_novel_restart_hack") + + def test_get_unknown_raises(self): + with self.assertRaises(rp.UnknownRestartPathError): + rp.get_restart_path("nope") + + def test_registered_attempt_returns_path(self): + path = rp.assert_restart_attempt_registered("manual_daemon_kill") + self.assertEqual(path.classification, rp.CLASS_FORBIDDEN) + + +class TestDaemonNeverSelfReplaces(unittest.TestCase): + """Previously-unguarded full-restart primitive: daemon self-replacement.""" + + def test_no_self_replacement_in_source(self): + # The live daemon modules must contain no os.execv/os.kill/os._exit + # self-restart call. Must not raise. + rp.assert_no_daemon_self_replacement(REPO_ROOT) + + def test_scanner_flags_injected_violation(self): + # Guard the guard: prove the scanner catches a real self-replace call. + with tempfile.TemporaryDirectory() as tmp: + bad = Path(tmp) / "gitea_mcp_server.py" + bad.write_text( + "import os\n" + "def restart():\n" + " os.execv('/usr/bin/python', ['python'])\n", + encoding="utf-8", + ) + found = rp.scan_daemon_self_replacement(tmp) + self.assertTrue(found) + with self.assertRaises(AssertionError): + rp.assert_no_daemon_self_replacement(tmp) + + def test_scanner_ignores_comment_and_docstring_mentions(self): + with tempfile.TemporaryDirectory() as tmp: + ok = Path(tmp) / "gitea_mcp_server.py" + ok.write_text( + "import os\n" + "# NOT os.execv() to re-point the interpreter here.\n" + '"""Never calls os._exit to restart."""\n' + "value = 1\n", + encoding="utf-8", + ) + self.assertEqual(rp.scan_daemon_self_replacement(tmp), []) + + +class TestLegacyAutoRestartHelperRemoved(unittest.TestCase): + """Previously-unguarded full-restart path: _trigger_mcp_auto_restart.""" + + def test_helper_absent_in_source(self): + # Must not raise: helper was removed in #685. + rp.assert_auto_restart_helper_absent(REPO_ROOT) + + def test_scanner_flags_reintroduced_helper(self): + with tempfile.TemporaryDirectory() as tmp: + bad = Path(tmp) / "mcp_server.py" + bad.write_text( + "def _trigger_mcp_auto_restart():\n return True\n", + encoding="utf-8", + ) + with self.assertRaises(AssertionError): + rp.assert_auto_restart_helper_absent(tmp) + + +class TestPkillStaysForbidden(unittest.TestCase): + """AC3: pkill of the daemon remains forbidden/contaminating (#630).""" + + def test_manual_daemon_kill_registered_as_forbidden(self): + path = rp.get_restart_path("manual_daemon_kill") + self.assertEqual(path.classification, rp.CLASS_FORBIDDEN) + + def test_pkill_classified_as_contamination(self): + assessment = runtime_recovery_guard.assess_recovery_command( + "pkill -f mcp_server.py" + ) + self.assertTrue(assessment["contaminated"]) + + def test_read_only_probe_not_contamination(self): + assessment = runtime_recovery_guard.assess_recovery_command( + "ps aux | grep mcp_server" + ) + self.assertFalse(assessment["contaminated"]) + + +class TestInventoryDocInSync(unittest.TestCase): + def test_doc_exists(self): + self.assertTrue(os.path.exists(DOC_PATH), DOC_PATH) + + def test_doc_mentions_every_path_id(self): + with open(DOC_PATH, encoding="utf-8") as handle: + doc = handle.read() + for path in rp.iter_restart_paths(): + self.assertIn(path.path_id, doc, f"doc missing {path.path_id}") + + +if __name__ == "__main__": + unittest.main()