From 29ad93d145372dbd8421a7f6f6c9acaaaedd3dca Mon Sep 17 00:00:00 2001 From: jcwalker3 Date: Sat, 25 Jul 2026 17:52:08 -0500 Subject: [PATCH] feat(mcp): expose sanctioned Codex MCP reconnect request (Closes #678) Add gitea_request_mcp_reconnect as a report-only callable surface so Codex and other agent hosts can request host/IDE reconnect with typed blockers and exact UI steps. Never kills processes or edits config. Co-Authored-By: Grok 4.5 --- docs/mcp-namespace-eof-recovery.md | 27 +- docs/mcp-restart-path-inventory.md | 5 +- docs/mcp-tool-inventory.md | 1 + gitea_mcp_server.py | 150 ++++++++- mcp_client_reconnect.py | 328 +++++++++++++++++++ mcp_restart_paths.py | 40 ++- tests/test_issue_678_mcp_client_reconnect.py | 323 ++++++++++++++++++ 7 files changed, 853 insertions(+), 21 deletions(-) create mode 100644 mcp_client_reconnect.py create mode 100644 tests/test_issue_678_mcp_client_reconnect.py diff --git a/docs/mcp-namespace-eof-recovery.md b/docs/mcp-namespace-eof-recovery.md index 7fd8ef2..edb184a 100644 --- a/docs/mcp-namespace-eof-recovery.md +++ b/docs/mcp-namespace-eof-recovery.md @@ -47,18 +47,33 @@ Do the steps in order. Stop as soon as a live **client-namespace** call succeeds - Only the Gitea namespace fails → single-namespace transport close. Continue. - Every server fails → restart the whole MCP client, not just one namespace. -2. **Reconnect the namespace through the client, not the shell.** Use the IDE / - client MCP-reconnect action for that server entry (in Claude Code: - `/mcp` → reconnect the affected `gitea-*` server). Reconnecting forces the - client to spawn a fresh subprocess and re-open the pipe. This clears the - closed-client state that a bare `kill`/respawn from a terminal does **not**. +2. **Request the sanctioned reconnect surface (#678), then reconnect through + the client — not the shell.** From a still-reachable Gitea MCP namespace + (or after host auto-reconnect), call: + + ```text + gitea_request_mcp_reconnect( + namespace="gitea-author", # or gitea-reviewer / gitea-merger / … + reason="transport_eof", + client="codex", # or claude_code / generic + ) + ``` + + The tool is **report-only**: it never restarts a process. It returns + namespace, profile, pid/session, startup SHA, current master SHA, boundary + status, and a **typed blocker** with exact operator UI steps for Codex + (Reload Developer Tools / per-server reconnect) or Claude Code (`/mcp`). + Then perform the host reconnect those steps describe so the client spawns a + fresh subprocess and re-opens the pipe. That clears the closed-client state + that a bare `kill`/respawn from a terminal does **not**. 3. **Do not "fix" it by importing the server or poking the process.** Reaching for `python -c 'import gitea_mcp_server ...'`, raw JSON-RPC from a shell, killing PIDs to force a respawn, or touching MCP config mtimes does **not** restore the *client's* view of the namespace and violates the daemon-import guard (#558, `docs/mcp-daemon-import-guard.md`). The only sanctioned repair - is a **client reconnect / relaunch**. + is a **client reconnect / relaunch** (or the typed operator path returned by + `gitea_request_mcp_reconnect`). 4. **Verify through the same path the workflow will use.** After reconnect, call the specific tool the blocked workflow needs — not just any tool — through diff --git a/docs/mcp-restart-path-inventory.md b/docs/mcp-restart-path-inventory.md index c880269..f88b81c 100644 --- a/docs/mcp-restart-path-inventory.md +++ b/docs/mcp-restart-path-inventory.md @@ -45,10 +45,11 @@ and *fails closed*. | `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 | +| `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, #678 | +| `codex_client_reconnect_request` | guarded_fail_closed | `gitea_request_mcp_reconnect` report-only tool for Codex/LLM sessions. | Report-only (#678): returns namespace/profile/pid/startup SHA/master SHA/boundary status and a typed operator blocker with exact client UI steps; never restarts or kills. | #678, #630, #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 | +| `ide_client_reconnect` | host_residual | A manual `/mcp reconnect` (or equivalent host action) that recreates the MCP client connection. Agents obtain exact UI steps via `gitea_request_mcp_reconnect` (#678). | Outside this process's control; the sanctioned recovery the gates point operators toward. No in-process code initiates it. | #584, #656, #657, #678 | | `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 diff --git a/docs/mcp-tool-inventory.md b/docs/mcp-tool-inventory.md index 66094ec..799d0d0 100644 --- a/docs/mcp-tool-inventory.md +++ b/docs/mcp-tool-inventory.md @@ -137,6 +137,7 @@ that gates each call, not which tools exist. - `gitea_release_merger_pr_lease` - `gitea_release_reviewer_pr_lease` - `gitea_release_workflow_lease` +- `gitea_request_mcp_reconnect` - `gitea_request_mcp_restart` - `gitea_resolve_task_capability` - `gitea_resume_review_draft` diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 915eaad..ea6e6e0 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -2092,6 +2092,7 @@ import root_checkout_guard # noqa: E402 import workflow_scope_guard # noqa: E402 # #683 production scope / force-on guards import stable_branch_push_guard # noqa: E402 import runtime_recovery_guard # noqa: E402 # #630 manual daemon-kill contamination +import mcp_client_reconnect # noqa: E402 # #678 sanctioned Codex reconnect request import remote_repo_guard # noqa: E402 import anti_stomp_preflight # noqa: E402 import issue_claim_heartbeat # noqa: E402 @@ -14024,11 +14025,16 @@ def _classify_operation_gate_reasons(reasons: list[str]) -> dict: def _stale_runtime_reconnect_action() -> str: - """Sanctioned recovery for a stale daemon — reconnect only (#685/#897).""" + """Sanctioned recovery for a stale daemon — reconnect only (#685/#897/#678).""" return ( - "Reconnect the IDE/client MCP session so the server reloads at the " - "current master head. Do not call gitea_activate_profile or switch " - "MCP role sessions — profile switching does not clear a stale daemon." + "blocker_kind=runtime_reconnect_required: call " + "gitea_request_mcp_reconnect(namespace=, " + "reason='stale-runtime', client='codex') for a typed operator " + "reconnect blocker with exact UI steps, then reconnect the IDE/client " + "MCP session so the server reloads at the current master head. Do not " + "call gitea_activate_profile, pkill, touch configs, or switch MCP role " + "sessions — profile switching does not clear a stale daemon. After " + "reconnect restart from gitea_whoami → gitea_resolve_task_capability." ) @@ -20998,10 +21004,15 @@ def gitea_resolve_task_capability( # serving process/profile inventory is stale — even if permission is OK. if runtime_stale_blocker: next_safe_action = ( - "blocker_kind=runtime_reconnect_required: reconnect/restart the " - "IDE-managed Gitea MCP server for this profile so it reloads current " - "master. Do not edit mcp_config.json by hand; the resolver does not " - "touch config, spawn recovery threads, or terminate the process." + "blocker_kind=runtime_reconnect_required: call " + "gitea_request_mcp_reconnect(namespace=, " + "reason='stale-runtime', client='codex') for a typed operator " + "blocker with exact UI steps, then reconnect/reload the IDE-managed " + "Gitea MCP server for this profile so it reloads current master. " + "Do not edit mcp_config.json by hand, pkill, or touch configs; the " + "resolver does not touch config, spawn recovery threads, or " + "terminate the process. After reconnect restart from gitea_whoami → " + "gitea_resolve_task_capability." ) # Task/role alignment guards (#167): the requested task, not the @@ -22559,6 +22570,129 @@ def gitea_workflow_dashboard( return payload +@mcp.tool() +def gitea_request_mcp_reconnect( + namespace: str | None = None, + reason: str | None = None, + client: str = "codex", + remote: str = "dadeschools", + host: str | None = None, + session_id: str | None = None, +) -> dict: + """Request a sanctioned host/IDE MCP reconnect for a named namespace (#678). + + Codex and other agent hosts can detect stale or closed Gitea MCP runtimes + (``stop_required`` / ``restart_required`` from capability resolution, transport + EOF, missing namespace attachment). The **host owns the transport** — this + process cannot reopen the client's stdio pipe. This tool is the callable + surface agents use to: + + 1. Report reconnect status fields (namespace, profile, pid/session, + startup SHA, current master SHA, boundary status). + 2. Return a **typed blocker** with exact operator UI steps for Codex (or + another client) when reconnect is required. + + This tool **never** restarts, kills, reloads, or reconfigures an MCP + process. Forbidden recovery paths (pkill, touch/mtime hacks, config/.env + edits, session-state edits, raw API) are never recommended. + + After the operator reconnects, workflows must restart from preflight: + ``gitea_whoami`` → ``gitea_resolve_task_capability`` → task. + + Args: + namespace: MCP namespace to reconnect (e.g. ``gitea-author``). Defaults + to the active profile's inferred namespace. + reason: Why reconnect is requested: ``stale-runtime``, ``transport_eof``, + ``missing_namespace``, ``not_required``, or free-form (normalized). + client: Operator UI surface — ``codex`` (default), ``claude_code``, or + ``generic``. + remote: Known instance — ``dadeschools`` or ``prgs`` (parity context). + host: Optional host override for parity context. + session_id: Optional session id to echo in the report. + + Returns: + dict with reconnect report fields, ``reconnect_performed=False``, + ``typed_blocker`` when reconnect is required, and + ``forbidden_recovery_paths``. + """ + # Read-only: gitea.read is sufficient. Never a mutation. + read_block = _profile_operation_gate("gitea.read") + if read_block: + return { + "success": False, + "read_only": True, + "reconnect_performed": False, + "mutation_performed": False, + "reasons": read_block, + "permission_report": _permission_block_report("gitea.read"), + "forbidden_recovery_paths": list( + mcp_client_reconnect.FORBIDDEN_RECOVERY_PATHS + ), + } + + profile = get_profile() + profile_name = (profile.get("profile_name") or "").strip() or None + inferred_ns = role_namespace_gate.infer_mcp_namespace(profile_name) + ns = (namespace or "").strip() or inferred_ns or "gitea-tools" + + parity = _current_master_parity() + startup_sha = ( + parity.get("daemon_start_head") + or parity.get("startup_head") + or _process_boot_head_sha + ) + current_sha = parity.get("local_head") or parity.get("current_head") + if not current_sha: + try: + current_sha = master_parity_gate.read_git_head(PROJECT_ROOT) + except Exception: # noqa: BLE001 + current_sha = None + + boundary = mcp_client_reconnect.classify_boundary_status( + startup_sha=startup_sha if isinstance(startup_sha, str) else None, + current_master_sha=current_sha if isinstance(current_sha, str) else None, + live_stale=bool(parity.get("live_stale")) if parity.get("live_known") else None, + in_parity=parity.get("in_parity") if parity.get("determinable") else None, + ) + + # Infer reason from parity when caller left it unspecified. + effective_reason = reason + if not (effective_reason or "").strip(): + if parity.get("restart_required") or parity.get("live_stale"): + effective_reason = mcp_client_reconnect.REASON_STALE_RUNTIME + elif boundary == mcp_client_reconnect.BOUNDARY_CLEAN: + effective_reason = mcp_client_reconnect.REASON_NOT_REQUIRED + else: + effective_reason = mcp_client_reconnect.REASON_UNSPECIFIED + + payload = mcp_client_reconnect.build_reconnect_request( + namespace=ns, + profile=profile_name, + pid=os.getpid(), + session_id=session_id + or f"{(profile_name or 'session')}-{os.getpid()}", + startup_sha=startup_sha if isinstance(startup_sha, str) else None, + current_master_sha=current_sha if isinstance(current_sha, str) else None, + boundary_status=boundary, + reason=effective_reason, + client=client, + live_stale=bool(parity.get("live_stale")) if parity.get("live_known") else None, + in_parity=parity.get("in_parity") if parity.get("determinable") else None, + restart_required=bool(parity.get("restart_required")), + stop_required=bool(parity.get("restart_required")), + extra={ + "remote": remote if remote in REMOTES else remote, + "host": host, + "session_context_audit": session_ctx.mutation_context_audit_fields(), + "parity_summary": master_parity_gate.format_parity(parity), + "live_stale": parity.get("live_stale"), + "live_known": parity.get("live_known"), + "in_parity": parity.get("in_parity"), + }, + ) + return payload + + @mcp.tool() def gitea_request_mcp_restart( remote: str = "dadeschools", diff --git a/mcp_client_reconnect.py b/mcp_client_reconnect.py new file mode 100644 index 0000000..2283d0a --- /dev/null +++ b/mcp_client_reconnect.py @@ -0,0 +1,328 @@ +"""Sanctioned MCP client reconnect request surface for Codex/LLM sessions (#678). + +Codex and other agent hosts can detect stale or closed Gitea MCP runtimes, but +the host owns the transport. This module never restarts, kills, or reloads a +daemon. It builds: + +1. A **callable reconnect request** result agents can invoke via + ``gitea_request_mcp_reconnect`` (report-only, side-effect free). +2. A **typed blocker** with exact operator UI steps when recovery must be + performed by the host/operator. + +Forbidden recovery paths (must never be recommended): + +* ``pkill`` / ``kill`` / ``killall`` of MCP daemons +* ``touch`` / mtime config reload hacks +* ``.env`` or MCP config edits as recovery +* session-state file edits +* raw Gitea API / direct server-import fallbacks + +After the operator reconnects, workflows restart from identity / runtime / +capability preflight (``gitea_whoami`` → ``gitea_resolve_task_capability`` → +task). +""" + +from __future__ import annotations + +from typing import Any, Mapping + +# --- Reason vocabulary ------------------------------------------------------- + +REASON_STALE_RUNTIME = "stale-runtime" +REASON_TRANSPORT_EOF = "transport_eof" +REASON_MISSING_NAMESPACE = "missing_namespace" +REASON_NOT_REQUIRED = "not_required" +REASON_UNSPECIFIED = "unspecified" + +VALID_REASONS = frozenset( + { + REASON_STALE_RUNTIME, + REASON_TRANSPORT_EOF, + REASON_MISSING_NAMESPACE, + REASON_NOT_REQUIRED, + REASON_UNSPECIFIED, + } +) + +# Boundary statuses reported to callers (match review_workflow_boundary style). +BOUNDARY_CLEAN = "clean" +BOUNDARY_MISMATCH = "mismatch" +BOUNDARY_STALE = "stale" +BOUNDARY_UNKNOWN = "unknown" + +# Typed blocker kinds +BLOCKER_OPERATOR_RECONNECT = "operator_mcp_reconnect_required" +BLOCKER_NONE = "none" + +FORBIDDEN_RECOVERY_PATHS: tuple[str, ...] = ( + "pkill / kill / killall of mcp_server.py, gitea_mcp_server, or broad python sweeps", + "touch / mtime-based MCP config reload hacks", + ".env edits as recovery", + "MCP config file edits as recovery", + "session-state file edits as recovery", + "raw Gitea API or direct MCP server-import fallbacks", +) + +# Client-specific operator UI steps. Keep Codex first (issue title surface). +OPERATOR_UI_STEPS: dict[str, tuple[str, ...]] = { + "codex": ( + "In Codex, open the MCP / Developer tools panel for this workspace.", + "Locate the named Gitea MCP server entry (namespace) that needs reconnect " + "(e.g. gitea-author, gitea-reviewer, gitea-merger, gitea-tools, " + "gitea-controller, gitea-reconciler).", + "Click 'Reload Developer Tools' or the server reconnect/reload control " + "for that entry so the client spawns a fresh MCP subprocess.", + "If per-server reconnect is unavailable, fully restart the Codex client " + "(quit and relaunch) so all MCP namespaces reattach.", + "After reconnect, rerun the blocked workflow from preflight: " + "gitea_whoami → gitea_resolve_task_capability → the original task. " + "Do not resume mid-mutation.", + ), + "claude_code": ( + "Run `/mcp` (or open the MCP servers UI) in Claude Code.", + "Reconnect the affected gitea-* server entry so the client reopens stdio.", + "If reconnect fails, relaunch the Claude Code session entirely.", + "After reconnect, restart the workflow from gitea_whoami → " + "gitea_resolve_task_capability → task.", + ), + "generic": ( + "Use the host/IDE MCP reconnect or reload control for the named namespace.", + "If no per-namespace control exists, restart the MCP client/editor.", + "After reconnect, restart the workflow from identity/capability preflight.", + ), +} + +DEFAULT_CLIENT = "codex" + + +def normalize_reason(reason: str | None) -> str: + """Map free-form reason strings onto the closed vocabulary.""" + raw = (reason or "").strip().lower() + if not raw: + return REASON_UNSPECIFIED + if raw in VALID_REASONS: + return raw + text = raw.replace(" ", "_").replace("-", "_") + aliases = { + "stale_runtime": REASON_STALE_RUNTIME, + "staleruntime": REASON_STALE_RUNTIME, + "runtime_stale": REASON_STALE_RUNTIME, + "stale": REASON_STALE_RUNTIME, + "transport_eof": REASON_TRANSPORT_EOF, + "transport_closed": REASON_TRANSPORT_EOF, + "eof": REASON_TRANSPORT_EOF, + "client_is_closing": REASON_TRANSPORT_EOF, + "missing_namespace": REASON_MISSING_NAMESPACE, + "namespace_missing": REASON_MISSING_NAMESPACE, + "not_required": REASON_NOT_REQUIRED, + "healthy": REASON_NOT_REQUIRED, + "ok": REASON_NOT_REQUIRED, + "unspecified": REASON_UNSPECIFIED, + } + if text in aliases: + return aliases[text] + hyphenated = text.replace("_", "-") + if hyphenated in VALID_REASONS: + return hyphenated + return REASON_UNSPECIFIED + + +def normalize_client(client: str | None) -> str: + """Return a known client key for operator UI steps.""" + text = (client or "").strip().lower().replace(" ", "_").replace("-", "_") + if text in ("codex", "openai_codex", "openai"): + return "codex" + if text in ("claude", "claude_code", "claude_desktop", "anthropic"): + return "claude_code" + if text in OPERATOR_UI_STEPS: + return text + return DEFAULT_CLIENT + + +def classify_boundary_status( + *, + startup_sha: str | None, + current_master_sha: str | None, + live_stale: bool | None = None, + in_parity: bool | None = None, +) -> str: + """Derive boundary_status from parity evidence.""" + if live_stale is True or in_parity is False: + return BOUNDARY_STALE + start = (startup_sha or "").strip().lower() + current = (current_master_sha or "").strip().lower() + if start and current and start != current: + return BOUNDARY_MISMATCH + if start and current and start == current: + return BOUNDARY_CLEAN + if in_parity is True: + return BOUNDARY_CLEAN + return BOUNDARY_UNKNOWN + + +def operator_ui_steps(client: str | None, *, namespace: str | None = None) -> list[str]: + """Exact operator UI steps for the named client.""" + key = normalize_client(client) + steps = list(OPERATOR_UI_STEPS.get(key) or OPERATOR_UI_STEPS[DEFAULT_CLIENT]) + ns = (namespace or "").strip() + if ns: + steps = [ + s.replace("named Gitea MCP server entry (namespace)", f"namespace '{ns}'") + .replace("affected gitea-* server entry", f"server entry '{ns}'") + .replace("named namespace", f"namespace '{ns}'") + for s in steps + ] + return steps + + +def build_reconnect_request( + *, + namespace: str, + profile: str | None = None, + pid: int | str | None = None, + session_id: str | None = None, + startup_sha: str | None = None, + current_master_sha: str | None = None, + boundary_status: str | None = None, + reason: str | None = None, + client: str | None = DEFAULT_CLIENT, + live_stale: bool | None = None, + in_parity: bool | None = None, + restart_required: bool | None = None, + stop_required: bool | None = None, + extra: Mapping[str, Any] | None = None, +) -> dict[str, Any]: + """Build the structured reconnect-request / typed-blocker payload (#678). + + Never mutates process, config, or session state. Always side-effect free. + """ + ns = (namespace or "").strip() or "unknown" + normalized_reason = normalize_reason(reason) + boundary = (boundary_status or "").strip() or classify_boundary_status( + startup_sha=startup_sha, + current_master_sha=current_master_sha, + live_stale=live_stale, + in_parity=in_parity, + ) + + reconnect_needed = True + if normalized_reason == REASON_NOT_REQUIRED and boundary == BOUNDARY_CLEAN: + reconnect_needed = False + if restart_required is False and stop_required is False and boundary == BOUNDARY_CLEAN: + # Explicit healthy probe + if normalized_reason in (REASON_NOT_REQUIRED, REASON_UNSPECIFIED): + reconnect_needed = False + normalized_reason = REASON_NOT_REQUIRED + + if restart_required is True or stop_required is True: + reconnect_needed = True + if normalized_reason in (REASON_NOT_REQUIRED, REASON_UNSPECIFIED): + normalized_reason = REASON_STALE_RUNTIME + + client_key = normalize_client(client) + steps = operator_ui_steps(client_key, namespace=ns) + + result: dict[str, Any] = { + "success": True, + "read_only": True, + "reconnect_performed": False, + "mutation_performed": False, + "reconnect_needed": reconnect_needed, + "namespace": ns, + "profile": (profile or "").strip() or None, + "pid": pid, + "session_id": (session_id or "").strip() or None, + "startup_sha": (startup_sha or "").strip() or None, + "current_master_sha": (current_master_sha or "").strip() or None, + "boundary_status": boundary, + "reason": normalized_reason, + "client": client_key, + "forbidden_recovery_paths": list(FORBIDDEN_RECOVERY_PATHS), + "post_reconnect_preflight": [ + "gitea_whoami", + "gitea_resolve_task_capability", + "original_task", + ], + "exact_safe_next_action": None, + "blocker_kind": BLOCKER_NONE, + "operator_ui_steps": steps, + "typed_blocker": None, + } + + if reconnect_needed: + result["blocker_kind"] = BLOCKER_OPERATOR_RECONNECT + result["stop_required"] = True + result["restart_required"] = True + result["exact_safe_next_action"] = ( + f"blocker_kind={BLOCKER_OPERATOR_RECONNECT}: operator must reconnect " + f"MCP namespace '{ns}' via the host UI (client={client_key}). " + "Do not pkill, touch configs, edit session state, or use raw API. " + "After reconnect, restart from gitea_whoami → " + "gitea_resolve_task_capability → task." + ) + result["typed_blocker"] = { + "blocker_kind": BLOCKER_OPERATOR_RECONNECT, + "namespaces": [ns], + "why_reconnect_required": normalized_reason, + "operator_ui_steps": steps, + "client": client_key, + "forbidden_recovery_paths": list(FORBIDDEN_RECOVERY_PATHS), + "instruction_after_reconnect": ( + "Rerun the blocked workflow from preflight " + "(gitea_whoami → gitea_resolve_task_capability → task). " + "Do not continue mid-mutation from pre-reconnect state." + ), + } + else: + result["stop_required"] = False + result["restart_required"] = False + result["exact_safe_next_action"] = ( + f"Reconnect not required for namespace '{ns}' " + f"(boundary_status={boundary}). Proceed with the original task." + ) + + if extra: + for key, value in extra.items(): + if key not in result: + result[key] = value + + return result + + +def reasons_never_suggest_forbidden(text: str) -> bool: + """Return True when *text* does not recommend a forbidden recovery path. + + Mentions that *ban* a path (e.g. ``Do not pkill`` / ``never edit session + state``) are allowed. Positive recommendations such as ``use pkill`` or + ``run killall`` fail. + """ + import re + + lowered = (text or "").lower() + # Strip common ban prefixes so "do not pkill" does not trip positive checks. + scrubbed = re.sub( + r"\b(?:do not|don't|never|must not|forbid(?:den)?|ban(?:ned)?)\b" + r"[^.!;\n]{0,80}", + " ", + lowered, + ) + # Positive imperative / advisory forms that would tell an agent to do harm. + positive_suggestions = ( + "use pkill", + "run pkill", + "try pkill", + "pkill -f", + "use killall", + "run killall", + "killall mcp", + "use kill ", + "run kill ", + "touch the mcp", + "touch mcp config", + "utime(", + "edit the mcp config to recover", + "edit .env to recover", + "import gitea_mcp_server", + "python -c 'import gitea_mcp", + ) + return not any(frag in scrubbed for frag in positive_suggestions) diff --git a/mcp_restart_paths.py b/mcp_restart_paths.py index 087fe72..817131d 100644 --- a/mcp_restart_paths.py +++ b/mcp_restart_paths.py @@ -225,8 +225,33 @@ _RESTART_PATHS: tuple[RestartPath, ...] = ( "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"), + locations=( + "gitea_mcp_server.py (gitea_resolve_task_capability)", + "gitea_mcp_server.py (gitea_request_mcp_reconnect)", + "mcp_client_reconnect.py", + ), + references=("#685", "#657", "#678"), + ), + RestartPath( + path_id="codex_client_reconnect_request", + title="Sanctioned Codex/LLM reconnect request tool", + mechanism=( + "gitea_request_mcp_reconnect: agents invoke a report-only tool that " + "returns namespace/profile/pid/startup SHA/master SHA/boundary " + "status plus a typed operator blocker with exact client UI steps." + ), + classification=CLASS_GUARDED_FAIL_CLOSED, + guard=( + "Report-only (#678): never restarts, kills, reloads, or edits " + "config; recovery is always host/operator reconnect. Forbidden " + "paths (pkill, touch, .env/config/session-state hacks) are listed " + "and never recommended." + ), + locations=( + "mcp_client_reconnect.py", + "gitea_mcp_server.py (gitea_request_mcp_reconnect)", + ), + references=("#678", "#630", "#685", "#657"), ), RestartPath( path_id="manual_daemon_kill", @@ -271,7 +296,8 @@ _RESTART_PATHS: tuple[RestartPath, ...] = ( title="Host/IDE MCP reconnect", mechanism=( "A manual `/mcp reconnect` (or equivalent host action) that the " - "IDE performs to recreate the MCP client connection." + "IDE performs to recreate the MCP client connection. Agents obtain " + "exact UI steps via gitea_request_mcp_reconnect (#678)." ), classification=CLASS_HOST_RESIDUAL, guard=( @@ -279,8 +305,12 @@ _RESTART_PATHS: tuple[RestartPath, ...] = ( "gates point operators toward; documented as residual host " "behavior. No in-process code initiates it." ), - locations=("host/IDE",), - references=("#584", "#656", "#657"), + locations=( + "host/IDE", + "mcp_client_reconnect.py", + "gitea_mcp_server.py (gitea_request_mcp_reconnect)", + ), + references=("#584", "#656", "#657", "#678"), residual_host=True, ), RestartPath( diff --git a/tests/test_issue_678_mcp_client_reconnect.py b/tests/test_issue_678_mcp_client_reconnect.py new file mode 100644 index 0000000..5affc0e --- /dev/null +++ b/tests/test_issue_678_mcp_client_reconnect.py @@ -0,0 +1,323 @@ +"""Tests for sanctioned Codex MCP reconnect request surface (#678).""" + +from __future__ import annotations + +import os +import unittest +from unittest import mock + +import mcp_client_reconnect as mcr + + +class NormalizeReasonTests(unittest.TestCase): + def test_stale_runtime_aliases(self): + self.assertEqual(mcr.normalize_reason("stale-runtime"), mcr.REASON_STALE_RUNTIME) + self.assertEqual(mcr.normalize_reason("stale_runtime"), mcr.REASON_STALE_RUNTIME) + self.assertEqual(mcr.normalize_reason("STALE"), mcr.REASON_STALE_RUNTIME) + + def test_transport_eof_aliases(self): + self.assertEqual(mcr.normalize_reason("transport_eof"), mcr.REASON_TRANSPORT_EOF) + self.assertEqual(mcr.normalize_reason("EOF"), mcr.REASON_TRANSPORT_EOF) + self.assertEqual( + mcr.normalize_reason("client_is_closing"), mcr.REASON_TRANSPORT_EOF + ) + + def test_missing_namespace(self): + self.assertEqual( + mcr.normalize_reason("missing_namespace"), mcr.REASON_MISSING_NAMESPACE + ) + + def test_empty_is_unspecified(self): + self.assertEqual(mcr.normalize_reason(None), mcr.REASON_UNSPECIFIED) + self.assertEqual(mcr.normalize_reason(""), mcr.REASON_UNSPECIFIED) + + +class BoundaryClassificationTests(unittest.TestCase): + def test_clean_when_shas_match(self): + self.assertEqual( + mcr.classify_boundary_status( + startup_sha="abc", current_master_sha="abc" + ), + mcr.BOUNDARY_CLEAN, + ) + + def test_mismatch_when_shas_differ(self): + self.assertEqual( + mcr.classify_boundary_status( + startup_sha="aaa", current_master_sha="bbb" + ), + mcr.BOUNDARY_MISMATCH, + ) + + def test_stale_when_live_stale(self): + self.assertEqual( + mcr.classify_boundary_status( + startup_sha="aaa", + current_master_sha="aaa", + live_stale=True, + ), + mcr.BOUNDARY_STALE, + ) + + +class BuildReconnectRequestTests(unittest.TestCase): + def test_stale_runtime_returns_typed_blocker_with_codex_steps(self): + result = mcr.build_reconnect_request( + namespace="gitea-author", + profile="prgs-author", + pid=1234, + session_id="sess-1", + startup_sha="aaa111", + current_master_sha="bbb222", + reason="stale-runtime", + client="codex", + restart_required=True, + stop_required=True, + ) + self.assertTrue(result["success"]) + self.assertTrue(result["read_only"]) + self.assertFalse(result["reconnect_performed"]) + self.assertFalse(result["mutation_performed"]) + self.assertTrue(result["reconnect_needed"]) + self.assertEqual(result["namespace"], "gitea-author") + self.assertEqual(result["profile"], "prgs-author") + self.assertEqual(result["pid"], 1234) + self.assertEqual(result["session_id"], "sess-1") + self.assertEqual(result["startup_sha"], "aaa111") + self.assertEqual(result["current_master_sha"], "bbb222") + self.assertEqual(result["boundary_status"], mcr.BOUNDARY_MISMATCH) + self.assertEqual(result["blocker_kind"], mcr.BLOCKER_OPERATOR_RECONNECT) + self.assertIsNotNone(result["typed_blocker"]) + blocker = result["typed_blocker"] + self.assertEqual(blocker["namespaces"], ["gitea-author"]) + self.assertEqual(blocker["why_reconnect_required"], mcr.REASON_STALE_RUNTIME) + self.assertTrue(any("Codex" in s or "Reload" in s for s in blocker["operator_ui_steps"])) + self.assertIn("pkill", " ".join(result["forbidden_recovery_paths"]).lower()) + self.assertTrue( + mcr.reasons_never_suggest_forbidden(result["exact_safe_next_action"] or "") + ) + # Must not recommend forbidden recovery. + for step in blocker["operator_ui_steps"]: + self.assertTrue(mcr.reasons_never_suggest_forbidden(step), step) + + def test_transport_eof_typed_blocker(self): + result = mcr.build_reconnect_request( + namespace="gitea-reviewer", + reason="transport_eof", + client="claude_code", + ) + self.assertTrue(result["reconnect_needed"]) + self.assertEqual(result["reason"], mcr.REASON_TRANSPORT_EOF) + self.assertEqual(result["client"], "claude_code") + steps = " ".join(result["operator_ui_steps"]).lower() + self.assertIn("/mcp", steps) + + def test_missing_namespace_typed_blocker(self): + result = mcr.build_reconnect_request( + namespace="gitea-merger", + reason="missing_namespace", + client="codex", + ) + self.assertTrue(result["reconnect_needed"]) + self.assertEqual(result["reason"], mcr.REASON_MISSING_NAMESPACE) + self.assertEqual( + result["typed_blocker"]["blocker_kind"], mcr.BLOCKER_OPERATOR_RECONNECT + ) + + def test_healthy_not_required(self): + result = mcr.build_reconnect_request( + namespace="gitea-tools", + startup_sha="deadbeef", + current_master_sha="deadbeef", + reason="not_required", + client="codex", + in_parity=True, + restart_required=False, + stop_required=False, + ) + self.assertFalse(result["reconnect_needed"]) + self.assertEqual(result["blocker_kind"], mcr.BLOCKER_NONE) + self.assertIsNone(result["typed_blocker"]) + self.assertFalse(result["stop_required"]) + self.assertFalse(result["restart_required"]) + self.assertIn("not required", (result["exact_safe_next_action"] or "").lower()) + + def test_successful_reconnect_report_fields_present(self): + """AC2: reconnect result reports required fields (even when needed).""" + result = mcr.build_reconnect_request( + namespace="gitea-controller", + profile="prgs-controller", + pid=99, + session_id="sid", + startup_sha="s" * 40, + current_master_sha="c" * 40, + reason="stale-runtime", + ) + for key in ( + "namespace", + "profile", + "pid", + "session_id", + "startup_sha", + "current_master_sha", + "boundary_status", + ): + self.assertIn(key, result) + self.assertIsNotNone(result[key], key) + + +class ToolSurfaceTests(unittest.TestCase): + """Exercise gitea_request_mcp_reconnect with a stubbed server context.""" + + def test_tool_is_registered_and_side_effect_free(self): + import gitea_mcp_server as srv + + self.assertTrue(hasattr(srv, "gitea_request_mcp_reconnect")) + with mock.patch.object(srv, "_profile_operation_gate", return_value=None): + with mock.patch.object( + srv, + "get_profile", + return_value={ + "profile_name": "prgs-author", + "role_kind": "author", + "role": "author", + }, + ): + with mock.patch.object( + srv, + "_current_master_parity", + return_value={ + "startup_head": "a" * 40, + "current_head": "a" * 40, + "daemon_start_head": "a" * 40, + "local_head": "a" * 40, + "in_parity": True, + "stale": False, + "restart_required": False, + "determinable": True, + "live_stale": False, + "live_known": True, + "reasons": [], + }, + ): + with mock.patch.object( + srv.master_parity_gate, + "format_parity", + return_value="in parity", + ): + with mock.patch.object( + srv.role_namespace_gate, + "infer_mcp_namespace", + return_value="gitea-author", + ): + with mock.patch.object( + srv.session_ctx, + "mutation_context_audit_fields", + return_value={"session_profile": "prgs-author"}, + ): + result = srv.gitea_request_mcp_reconnect( + namespace="gitea-author", + reason="not_required", + client="codex", + remote="prgs", + ) + self.assertTrue(result.get("success")) + self.assertFalse(result.get("reconnect_performed")) + self.assertFalse(result.get("mutation_performed")) + self.assertEqual(result.get("namespace"), "gitea-author") + self.assertEqual(result.get("profile"), "prgs-author") + self.assertEqual(result.get("pid"), os.getpid()) + self.assertIn("startup_sha", result) + self.assertIn("current_master_sha", result) + self.assertIn("boundary_status", result) + self.assertTrue( + mcr.reasons_never_suggest_forbidden( + result.get("exact_safe_next_action") or "" + ) + ) + + def test_tool_stale_returns_typed_blocker(self): + import gitea_mcp_server as srv + + with mock.patch.object(srv, "_profile_operation_gate", return_value=None): + with mock.patch.object( + srv, + "get_profile", + return_value={ + "profile_name": "prgs-reconciler", + "role_kind": "reconciler", + "role": "reconciler", + }, + ): + with mock.patch.object( + srv, + "_current_master_parity", + return_value={ + "startup_head": "a" * 40, + "current_head": "b" * 40, + "daemon_start_head": "a" * 40, + "local_head": "b" * 40, + "in_parity": False, + "stale": True, + "restart_required": True, + "determinable": True, + "live_stale": True, + "live_known": True, + "reasons": ["stale"], + }, + ): + with mock.patch.object( + srv.master_parity_gate, + "format_parity", + return_value="stale", + ): + with mock.patch.object( + srv.role_namespace_gate, + "infer_mcp_namespace", + return_value="gitea-reconciler", + ): + with mock.patch.object( + srv.session_ctx, + "mutation_context_audit_fields", + return_value={}, + ): + result = srv.gitea_request_mcp_reconnect( + reason="stale-runtime", + client="codex", + ) + self.assertTrue(result["reconnect_needed"]) + self.assertEqual( + result["blocker_kind"], mcr.BLOCKER_OPERATOR_RECONNECT + ) + self.assertIsNotNone(result["typed_blocker"]) + self.assertIn("gitea-reconciler", result["typed_blocker"]["namespaces"]) + self.assertTrue(result["stop_required"]) + self.assertTrue(result["restart_required"]) + self.assertTrue( + mcr.reasons_never_suggest_forbidden( + result.get("exact_safe_next_action") or "" + ) + ) + + +class InventoryRegistrationTests(unittest.TestCase): + def test_reconnect_path_in_restart_inventory(self): + import mcp_restart_paths as mrp + + ids = {p.path_id for p in mrp.iter_restart_paths()} + self.assertIn("codex_client_reconnect_request", ids) + self.assertIn("ide_client_reconnect", ids) + + def test_tool_name_in_documented_inventory(self): + import mcp_tool_inventory as inv + + doc_path = os.path.join( + os.path.dirname(os.path.dirname(__file__)), inv.INVENTORY_DOC_PATH + ) + with open(doc_path, encoding="utf-8") as handle: + documented = inv.parse_documented_inventory(handle.read()) + self.assertIn("gitea_request_mcp_reconnect", documented) + + +if __name__ == "__main__": + unittest.main()