Compare commits
6
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 jcwalker3
|
467e35504c | ||
|
jcwalker3
|
3a0d9e24ea | ||
|
jcwalker3
|
dc0a05e5e9 | ||
|
jcwalker3
|
8b34f9da0a | ||
|
jcwalker3
|
04d9df559e | ||
 sysadmin
|
8a63476787 |
@@ -1,90 +0,0 @@
|
||||
# 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 <pid>` 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()`.
|
||||
@@ -1,475 +0,0 @@
|
||||
"""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 <pid>` 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}"
|
||||
)
|
||||
+51
-2
@@ -228,25 +228,74 @@ def find_active_reviewer_lease(
|
||||
return None
|
||||
|
||||
|
||||
def _conflict_fix_chain_key(lease: dict) -> tuple | None:
|
||||
"""Identity of the lease chain a conflict-fix marker belongs to (#842).
|
||||
|
||||
Keyed by PR number, profile, head_before, and branch. Returns None when any
|
||||
required component (pr_number, profile, head_before) is missing or malformed.
|
||||
"""
|
||||
raw = lease.get("raw_fields") or {}
|
||||
pr_number = lease.get("pr_number")
|
||||
profile = (lease.get("profile") or "").strip().lower()
|
||||
head_before = lease.get("head_before")
|
||||
branch = (lease.get("branch") or raw.get("branch") or "").strip()
|
||||
if not (pr_number and profile and head_before):
|
||||
return None
|
||||
return (pr_number, profile, head_before, branch)
|
||||
|
||||
|
||||
def _conflict_fix_chain_matches(key1: tuple, key2: tuple) -> bool:
|
||||
"""True when two conflict-fix chain keys refer to the same lease chain."""
|
||||
pr1, profile1, head1, branch1 = key1
|
||||
pr2, profile2, head2, branch2 = key2
|
||||
if pr1 != pr2 or profile1 != profile2 or head1 != head2:
|
||||
return False
|
||||
if branch1 and branch2 and branch1 != branch2:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _conflict_fix_chain_terminated_after(entries: list[dict], index: int) -> bool:
|
||||
"""True when a later marker terminates the conflict-fix chain of ``entries[index]``.
|
||||
|
||||
Append-only newest-wins: a terminal marker (phase=released/blocked/done)
|
||||
ends only its matching claim chain (#842).
|
||||
"""
|
||||
key = _conflict_fix_chain_key(entries[index])
|
||||
if key is None:
|
||||
return False
|
||||
for later in entries[index + 1:]:
|
||||
phase = (later.get("phase") or "").strip().lower()
|
||||
if phase not in _TERMINAL_CONFLICT_FIX_PHASES:
|
||||
continue
|
||||
later_key = _conflict_fix_chain_key(later)
|
||||
if later_key and _conflict_fix_chain_matches(key, later_key):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def find_active_conflict_fix_lease(
|
||||
comments: list[dict],
|
||||
*,
|
||||
pr_number: int,
|
||||
now: datetime | None = None,
|
||||
) -> dict[str, Any] | None:
|
||||
"""Return the newest unexpired conflict-fix lease for *pr_number*, if any."""
|
||||
"""Return the newest unexpired, non-terminated conflict-fix lease for *pr_number*, if any."""
|
||||
now = now or datetime.now(timezone.utc)
|
||||
candidates = [
|
||||
entry for entry in _comment_entries(comments, pr_number=pr_number)
|
||||
if entry.get("lease_kind") == "conflict_fix"
|
||||
]
|
||||
for lease in reversed(candidates):
|
||||
for index in range(len(candidates) - 1, -1, -1):
|
||||
lease = candidates[index]
|
||||
if _lease_expired(lease, now=now):
|
||||
continue
|
||||
phase = (lease.get("phase") or "").strip().lower()
|
||||
if phase in _TERMINAL_CONFLICT_FIX_PHASES:
|
||||
continue
|
||||
if phase in _ACTIVE_CONFLICT_FIX_PHASES or phase:
|
||||
if _conflict_fix_chain_terminated_after(candidates, index):
|
||||
continue
|
||||
return lease
|
||||
return None
|
||||
|
||||
|
||||
@@ -1,146 +0,0 @@
|
||||
"""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()
|
||||
@@ -19,6 +19,7 @@ from pr_work_lease import ( # noqa: E402
|
||||
assess_reviewer_mutation_blocked,
|
||||
assess_reviewer_stale_head_final_report,
|
||||
format_conflict_fix_lease_body,
|
||||
find_active_conflict_fix_lease,
|
||||
parse_conflict_fix_lease_comment,
|
||||
parse_reviewer_lease_comment,
|
||||
)
|
||||
@@ -203,5 +204,157 @@ class TestFormatLease(unittest.TestCase):
|
||||
self.assertEqual(parsed["pr_number"], 376)
|
||||
|
||||
|
||||
class TestConflictFixLeaseLifecycle(unittest.TestCase):
|
||||
def test_claim_followed_by_matching_release(self):
|
||||
claim_body = _conflict_fix_body(phase="claimed", worktree="branches/fix-376")
|
||||
expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z")
|
||||
release_body = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"branch: feat/fix-376",
|
||||
"worktree: branches/fix-376",
|
||||
"profile: prgs-author",
|
||||
"phase: released",
|
||||
f"head_before: {HEAD_A}",
|
||||
f"head_after: {HEAD_B}",
|
||||
f"expires_at: {expires}",
|
||||
])
|
||||
comments = [{"body": claim_body}, {"body": release_body}]
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW)
|
||||
self.assertIsNone(lease)
|
||||
|
||||
def test_expired_claim_without_release(self):
|
||||
past_expires = (NOW - timedelta(minutes=10)).isoformat().replace("+00:00", "Z")
|
||||
claim_body = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"phase: claimed",
|
||||
f"head_before: {HEAD_A}",
|
||||
f"expires_at: {past_expires}",
|
||||
"profile: prgs-author",
|
||||
])
|
||||
comments = [{"body": claim_body}]
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW)
|
||||
self.assertIsNone(lease)
|
||||
|
||||
def test_mismatched_release_different_head(self):
|
||||
claim_body = _conflict_fix_body(phase="claimed", worktree="branches/fix-376")
|
||||
expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z")
|
||||
release_body = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"profile: prgs-author",
|
||||
"phase: released",
|
||||
f"head_before: {HEAD_B}",
|
||||
f"expires_at: {expires}",
|
||||
])
|
||||
comments = [{"body": claim_body}, {"body": release_body}]
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW)
|
||||
self.assertIsNotNone(lease)
|
||||
self.assertEqual(lease["phase"], "claimed")
|
||||
|
||||
def test_mismatched_release_different_branch(self):
|
||||
claim_body = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"branch: feat/branch-A",
|
||||
"phase: claimed",
|
||||
f"head_before: {HEAD_A}",
|
||||
f"expires_at: {(NOW + timedelta(minutes=60)).isoformat().replace('+00:00', 'Z')}",
|
||||
"profile: prgs-author",
|
||||
])
|
||||
release_body = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"branch: feat/branch-B",
|
||||
"phase: released",
|
||||
f"head_before: {HEAD_A}",
|
||||
f"expires_at: {(NOW + timedelta(minutes=60)).isoformat().replace('+00:00', 'Z')}",
|
||||
"profile: prgs-author",
|
||||
])
|
||||
comments = [{"body": claim_body}, {"body": release_body}]
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW)
|
||||
self.assertIsNotNone(lease)
|
||||
self.assertEqual(lease["phase"], "claimed")
|
||||
|
||||
def test_release_followed_by_newer_claim(self):
|
||||
claim_1 = _conflict_fix_body(phase="claimed", worktree="branches/fix-376")
|
||||
expires = (NOW + timedelta(minutes=60)).isoformat().replace("+00:00", "Z")
|
||||
release_1 = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"profile: prgs-author",
|
||||
"phase: released",
|
||||
f"head_before: {HEAD_A}",
|
||||
f"head_after: {HEAD_B}",
|
||||
f"expires_at: {expires}",
|
||||
])
|
||||
claim_2 = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"profile: prgs-author",
|
||||
"phase: claimed",
|
||||
f"head_before: {HEAD_B}",
|
||||
f"expires_at: {expires}",
|
||||
])
|
||||
comments = [{"body": claim_1}, {"body": release_1}, {"body": claim_2}]
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW)
|
||||
self.assertIsNotNone(lease)
|
||||
self.assertEqual(lease["head_before"], HEAD_B)
|
||||
|
||||
def test_malformed_or_ambiguous_markers(self):
|
||||
malformed_release = "\n".join([
|
||||
CONFLICT_FIX_LEASE_MARKER,
|
||||
"pr: #376",
|
||||
"phase: released",
|
||||
# missing head_before and profile
|
||||
])
|
||||
claim_body = _conflict_fix_body(phase="claimed")
|
||||
comments = [{"body": claim_body}, {"body": malformed_release}]
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=376, now=NOW)
|
||||
self.assertIsNotNone(lease)
|
||||
|
||||
def test_pr818_historical_sequence(self):
|
||||
comment_14696 = "\n".join([
|
||||
"<!-- mcp-conflict-fix-lease:v1 -->",
|
||||
"pr: #818",
|
||||
"branch: feat/issue-638-webui-app-shell-phase1",
|
||||
"worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-638-webui-app-shell-phase1",
|
||||
"profile: prgs-author",
|
||||
"session_id: unknown",
|
||||
"phase: claimed",
|
||||
"head_before: 08061b7b8aebdd099a37d1abf5dafcf38e4fd3fb",
|
||||
"expires_at: 2026-07-23T07:12:13Z",
|
||||
"reviewer_active: no",
|
||||
])
|
||||
comment_14730 = "\n".join([
|
||||
"<!-- mcp-conflict-fix-lease:v1 -->",
|
||||
"pr: #818",
|
||||
"branch: feat/issue-638-webui-app-shell-phase1",
|
||||
"worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/issue-638-webui-app-shell-phase1",
|
||||
"profile: prgs-author",
|
||||
"session_id: prgs-author-61241-e5129c60",
|
||||
"phase: released",
|
||||
"head_before: 08061b7b8aebdd099a37d1abf5dafcf38e4fd3fb",
|
||||
"head_after: 64b6eb5d5402663098de5ded3b0617cc3b3df98f",
|
||||
"expires_at: 2026-07-23T06:05:00Z",
|
||||
"reviewer_active: no",
|
||||
])
|
||||
comments = [{"body": comment_14696}, {"body": comment_14730}]
|
||||
check_now = datetime(2026, 7, 23, 6, 30, tzinfo=timezone.utc)
|
||||
lease = find_active_conflict_fix_lease(comments, pr_number=818, now=check_now)
|
||||
self.assertIsNone(lease)
|
||||
|
||||
reviewer_gate = assess_reviewer_mutation_blocked(
|
||||
pr_number=818,
|
||||
comments=comments,
|
||||
reviewed_head_sha="64b6eb5d5402663098de5ded3b0617cc3b3df98f",
|
||||
live_head_sha="64b6eb5d5402663098de5ded3b0617cc3b3df98f",
|
||||
mutation="approve",
|
||||
now=check_now,
|
||||
)
|
||||
self.assertTrue(reviewer_gate["mutation_allowed"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user