From be0123c6dd8e8f6b593d2d6209be25fbaeaf8d23 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 9 Jul 2026 08:06:54 -0400 Subject: [PATCH] fix: resolve stale MCP runtime EOF follow-ups (#531) - fail closed when runtime diagnostics fail unexpectedly - move MCP connection healthcheck under scripts/ - document #543 as the remaining IDE namespace proof - keep namespace binding test isolated from mocked git metadata --- docs/wiki/Open-Decisions.md | 3 +- gitea_mcp_server.py | 18 ++++-- test_mcp_conn.py => scripts/test_mcp_conn.py | 59 ++++++++++++++------ tests/test_mcp_stale_runtime.py | 22 ++++---- tests/test_namespace_workspace_binding.py | 17 +++++- 5 files changed, 84 insertions(+), 35 deletions(-) rename test_mcp_conn.py => scripts/test_mcp_conn.py (74%) diff --git a/docs/wiki/Open-Decisions.md b/docs/wiki/Open-Decisions.md index a465ff7..673d9fd 100644 --- a/docs/wiki/Open-Decisions.md +++ b/docs/wiki/Open-Decisions.md @@ -8,4 +8,5 @@ Pending architectural and workflow decisions: - **Operation-scoped role selection (#228)** — Move from launcher-profile-dependent routing to per-operation profile resolution. - **Gitea-Tools wiki for dadeschools remote** — This bootstrap covers `prgs`; dadeschools instance wiki parity is undecided. -- **Server-side self-merge block** — Complement tool gates with Gitea branch protection where available. \ No newline at end of file +- **Server-side self-merge block** — Complement tool gates with Gitea branch protection where available. +- **IDE-namespace callable-tool proof (#543)** — Remaining follow-up. PR #545 (stale MCP runtime race-control) adds runtime staleness guards before mutations and improves diagnostics, but does not include a full cross-namespace callable tool proof for IDE-spawned MCP sessions. Track completion of direct "callable from IDE namespace" verification under #543. diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 3dcc18a..ae82dec 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -694,8 +694,13 @@ def _verify_role_mutation_workspace( raise RuntimeError("; ".join(runtime_reasons)) except Exception as exc: if "stale-runtime:" in str(exc): - raise RuntimeError(str(exc)) - pass + raise RuntimeError(str(exc)) from exc + # Fail-closed: unexpected error while performing stale-runtime guard must not be swallowed. + # Surface an actionable diagnostic instead of allowing a mutation under unknown runtime state. + raise RuntimeError( + f"stale-runtime: runtime diagnostic check failed unexpectedly ({type(exc).__name__}: {exc}). " + "Failing closed. Verify MCP server processes, profile configuration, and OS process visibility; retry after full MCP server restart." + ) from exc role = _effective_workspace_role() git_state = issue_lock_worktree.read_worktree_git_state( @@ -8338,7 +8343,12 @@ def gitea_route_task_session( def _check_mcp_runtimes_diagnostics(task: str, matching_profiles: list[str]) -> list[str]: - """Check running runtimes and return errors if they are missing or stale.""" + """Check running runtimes and return errors if they are missing or stale. + + NOTE: IDE-namespace callable-tool proof for #543 is a documented follow-up + if this change does not fully close the gap for tools invoked from + IDE-launched MCP client namespaces. See docs/wiki/Open-Decisions.md. + """ import subprocess import re from datetime import datetime @@ -8407,7 +8417,7 @@ def _check_mcp_runtimes_diagnostics(task: str, matching_profiles: list[str]) -> if self_stale: reasons.append( "stale-runtime: The active Gitea MCP server process is stale (running code from before changes were merged). " - "Please fully restart the Gitea MCP server (e.g. touch /Users/jasonwalker/.gemini/config/mcp_config.json) and retry." + "Please fully restart the Gitea MCP server (for the affected profile) and retry." ) if matching_profiles: diff --git a/test_mcp_conn.py b/scripts/test_mcp_conn.py similarity index 74% rename from test_mcp_conn.py rename to scripts/test_mcp_conn.py index 7481ce9..d26d17b 100644 --- a/test_mcp_conn.py +++ b/scripts/test_mcp_conn.py @@ -1,10 +1,20 @@ #!/usr/bin/env python3 """Live health-check script to verify Gitea MCP namespace connections. -Spawns the MCP server processes as defined in the IDE's global config, +Spawns the MCP server processes as defined in a config file, performs the JSON-RPC handshake, and queries the tools list to verify that the connection is fully operational and doesn't return EOF. + +Usage: + python scripts/test_mcp_conn.py [config.json] + +Env: + GITEA_MCP_HEALTHCHECK_CONFIG Path to mcpServers JSON config (e.g. ~/.gemini/config/mcp_config.json) + +The config file must contain an object with "mcpServers": { "gitea-author": {...}, ... } +Each server entry has "command", optional "args", optional "env". """ +import argparse import json import os import subprocess @@ -15,11 +25,11 @@ def test_connection(name, config): command = config.get("command") args = config.get("args", []) env = config.get("env", {}) - + # Merge current environment run_env = os.environ.copy() run_env.update(env) - + # Spawn subprocess try: proc = subprocess.Popen( @@ -46,11 +56,11 @@ def test_connection(name, config): }, "id": 1 } - + try: proc.stdin.write(json.dumps(init_req) + "\n") proc.stdin.flush() - + # Read response line = proc.stdout.readline() if not line: @@ -58,9 +68,9 @@ def test_connection(name, config): print(f" [FAIL] Received EOF from process. Stderr:\n{stderr_content}") proc.terminate() return False - + print(f" [OK] Received initialize response: {line.strip()[:150]}...") - + # Send initialized notification init_notif = { "jsonrpc": "2.0", @@ -68,7 +78,7 @@ def test_connection(name, config): } proc.stdin.write(json.dumps(init_notif) + "\n") proc.stdin.flush() - + # Send tools/list request list_req = { "jsonrpc": "2.0", @@ -78,23 +88,23 @@ def test_connection(name, config): } proc.stdin.write(json.dumps(list_req) + "\n") proc.stdin.flush() - + line = proc.stdout.readline() if not line: print(" [FAIL] Received EOF on tools/list request.") proc.terminate() return False - + res = json.loads(line) if "error" in res: print(f" [FAIL] Server returned error: {res['error']}") proc.terminate() return False - + tools = res.get("result", {}).get("tools", []) tool_names = [t.get("name") for t in tools] print(f" [OK] Successfully retrieved {len(tool_names)} tools: {tool_names[:5]}...") - + proc.terminate() return True except Exception as e: @@ -103,14 +113,28 @@ def test_connection(name, config): return False def main(): - config_path = "/Users/jasonwalker/.gemini/config/mcp_config.json" + parser = argparse.ArgumentParser(description="Gitea MCP connection health check") + parser.add_argument( + "config", + nargs="?", + default=os.environ.get("GITEA_MCP_HEALTHCHECK_CONFIG"), + help="Path to MCP config JSON with mcpServers (or set GITEA_MCP_HEALTHCHECK_CONFIG)" + ) + args = parser.parse_args() + + config_path = args.config + if not config_path: + print("ERROR: No config path provided. Pass as argument or set GITEA_MCP_HEALTHCHECK_CONFIG.") + print("Example: GITEA_MCP_HEALTHCHECK_CONFIG=/path/to/mcp_config.json python scripts/test_mcp_conn.py") + sys.exit(2) + try: with open(config_path) as f: mcp_config = json.load(f) except Exception as e: - print(f"Failed to load mcp_config.json: {e}") + print(f"Failed to load config {config_path}: {e}") sys.exit(1) - + servers = mcp_config.get("mcpServers", {}) failed = False for name in ["gitea-author", "gitea-reviewer"]: @@ -118,8 +142,8 @@ def main(): if not test_connection(name, servers[name]): failed = True else: - print(f"Server '{name}' not found in mcp_config.json") - + print(f"Server '{name}' not found in config") + if failed: sys.exit(1) else: @@ -127,3 +151,4 @@ def main(): if __name__ == "__main__": main() + diff --git a/tests/test_mcp_stale_runtime.py b/tests/test_mcp_stale_runtime.py index 19310c2..247797c 100644 --- a/tests/test_mcp_stale_runtime.py +++ b/tests/test_mcp_stale_runtime.py @@ -15,11 +15,11 @@ class TestMcpStaleRuntime(unittest.TestCase): # Setup mocks mock_getpid.return_value = 12345 mock_exists.return_value = True - + # Code modification time: Jul 8 2026, 14:00:00 code_time = datetime(2026, 7, 8, 14, 0, 0) mock_getmtime.return_value = code_time.timestamp() - + # Mock ps -ax output # PID 12345 is self (started at 13:00:00 - stale) # PID 54321 is prgs-author (started at 15:00:00 - fresh) @@ -29,17 +29,17 @@ class TestMcpStaleRuntime(unittest.TestCase): "12345 Wed Jul 8 13:00:00 2026 /path/to/python mcp_server.py\n" "54321 Wed Jul 8 15:00:00 2026 /path/to/python mcp_server.py\n" ) - + mock_run_ps = MagicMock() mock_run_ps.stdout = ps_output - + # Mock env output for ps eww mock_run_env12345 = MagicMock() mock_run_env12345.stdout = "GITEA_MCP_PROFILE=prgs-reconciler" - + mock_run_env54321 = MagicMock() mock_run_env54321.stdout = "GITEA_MCP_PROFILE=prgs-author" - + def side_effect(args, **kwargs): if args[0] == "ps" and "eww" in args: pid = args[2] @@ -50,18 +50,18 @@ class TestMcpStaleRuntime(unittest.TestCase): elif args[0] == "ps": return mock_run_ps raise ValueError(f"Unexpected subprocess run args: {args}") - + mock_run.side_effect = side_effect - + # Test 1: required reviewer role matching prgs-reviewer (which is missing) reasons = gitea_mcp_server._check_mcp_runtimes_diagnostics("review_pr", ["prgs-reviewer"]) - + self.assertTrue(any("stale-runtime: The active Gitea MCP server process is stale" in r for r in reasons)) self.assertTrue(any("stale-runtime: None of the matching profiles for task" in r for r in reasons)) - + # Test 2: required author role matching prgs-author (which is fresh) reasons_author = gitea_mcp_server._check_mcp_runtimes_diagnostics("create_issue", ["prgs-author"]) - + # Still contains self stale error self.assertTrue(any("stale-runtime: The active Gitea MCP server process is stale" in r for r in reasons_author)) # But does NOT contain missing author profile error diff --git a/tests/test_namespace_workspace_binding.py b/tests/test_namespace_workspace_binding.py index b9c270c..d1dca6c 100644 --- a/tests/test_namespace_workspace_binding.py +++ b/tests/test_namespace_workspace_binding.py @@ -234,7 +234,20 @@ class TestNamespaceWorkspaceIntegration(unittest.TestCase): mock_run.return_value = MagicMock(returncode=0, stdout=f"{CONTROL_ROOT}/.git\n") os.environ[nwb.AUTHOR_WORKTREE_ENV] = AUTHOR_DIRTY srv._preflight_resolved_role = "reviewer" + clean_git_state = { + "current_branch": "master", + "porcelain_status": "", + "head_sha": "abc123", + } with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT): with mock.patch("gitea_mcp_server.get_profile", return_value=self._merger_profile()): - resolved = srv._verify_role_mutation_workspace("prgs") - self.assertEqual(resolved, os.path.realpath(MCP_PROCESS_ROOT)) \ No newline at end of file + with mock.patch( + "gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", + return_value=clean_git_state, + ): + with mock.patch( + "gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", + return_value="abc123", + ): + resolved = srv._verify_role_mutation_workspace("prgs") + self.assertEqual(resolved, os.path.realpath(MCP_PROCESS_ROOT))