diff --git a/.github/workflows/agent-audit.yml b/.github/workflows/agent-audit.yml index 641d7c2..6fdb779 100644 --- a/.github/workflows/agent-audit.yml +++ b/.github/workflows/agent-audit.yml @@ -9,6 +9,11 @@ jobs: agent-audit: name: Agent Connection Audit runs-on: ubuntu-latest + # A ceiling the script cannot talk its way past. The script's own timeout was + # decorative - it re-checked the clock only between blocking readline() calls - + # so this job sat at GitHub's 6-hour default on every pull request. A script-level + # bound can have a hole in it; a job-level one cannot. + timeout-minutes: 10 steps: - name: Checkout Code uses: actions/checkout@v7 diff --git a/scripts/agent_audit.py b/scripts/agent_audit.py index 07dee32..7135a96 100644 --- a/scripts/agent_audit.py +++ b/scripts/agent_audit.py @@ -1,3 +1,43 @@ +"""Prove a freshly started server hands an agent a usable connection. + +WHAT THIS USED TO DO, AND WHY IT COULD NEVER PASS +------------------------------------------------- +It launched the server with `--tunnel none` and then scraped stdout for a line reading +`Agent setup URL:`. A local-only server has no setup URL *by design* - `mcp_url_launcher` +says so itself when you ask for one: + + "A local-only server has no setup URL by design" + +So the line never arrived. And the wait for it was not bounded either, because + + while time.time() - start_time < timeout: + line = process.stdout.readline() + +only re-checks the clock BETWEEN reads. `readline()` blocks, the server stayed alive and +quiet after its banner, and the pipe never closed. The `timeout = 15` was decorative. + +The result: every pull request started a job that sat for **six hours** and was then killed +at GitHub's ceiling. Run 32043881040 ran 15:57:11 to 21:57:27 and its last output was a +health check three hundredths of a second in. It has been failing this way, slowly and +expensively, on every PR. + +There was a third defect underneath: even if the line HAD arrived, it reads +`Agent setup URL: stored in the owner-readable connection file`, so +`line.split("Agent setup URL:")[1]` yields that sentence and the script then called +`urlopen()` on it. + +WHAT IT DOES NOW +---------------- +Reads the connection file, which is where a local-only server puts exactly what an agent +needs, and which the audit's own later code already expected the shape of. Nothing is +scraped from stdout, so nothing can block on it. + +Every wait has a deadline, and the workflow carries `timeout-minutes` as well, because a +script-level bound is a thing that can have a hole in it and a job-level one cannot. + +Unix only (`os.setsid`), which is what CI runs. +""" + import contextlib import json import os @@ -6,12 +46,19 @@ import socket import subprocess import sys +import threading import time import urllib.request from pathlib import Path SETUP_TOKEN_PATTERN = re.compile(r"(setup_token=)[^&\s]+") +# The server writes its connection file within a second in practice. Ten is slack for a +# cold CI runner; it is a DEADLINE rather than a hope, and it is enforced by a clock the +# reader cannot block. +CONNECTION_TIMEOUT = 30 +HEALTH_TIMEOUT = 10 + def redact_setup_tokens(text: str) -> str: """Remove credential query values before writing diagnostic output.""" @@ -24,20 +71,47 @@ def get_free_port(): return s.getsockname()[1] -def run_audit(): +def drain(stream) -> None: + """Echo server output for CI visibility, on a thread that nothing waits on. + + It runs as a daemon precisely so that a quiet, still-running server cannot hold the + audit open. Draining also stops the pipe buffer filling and blocking the server itself, + which is the other way this shape deadlocks. + """ + try: + for line in iter(stream.readline, ""): + sys.stdout.write(f"SERVER: {redact_setup_tokens(line)}") + sys.stdout.flush() + except (ValueError, OSError): + pass + + +def wait_for_connection(path: Path, deadline: float) -> dict | None: + """The connection file, once it is present AND parses. None if the deadline passes. + + Both conditions matter: the file appears before it is fully written, so a bare + `exists()` races and reads half a JSON document. + """ + while time.time() < deadline: + if path.exists(): + try: + return json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError): + pass + time.sleep(0.2) + return None + + +def run_audit() -> bool: port = get_free_port() - # Use a unique connection file to avoid conflicts - connection_file = f"audit_connection_{port}.json" + connection_file = Path(f"audit_connection_{port}.json") print(f"--- Starting Agent Audit on port {port} ---") - # Start the launcher - # Ensure PYTHONPATH is set so rigout is findable env = os.environ.copy() src_path = str(Path(__file__).resolve().parents[1] / "src") env["PYTHONPATH"] = src_path if "PYTHONPATH" not in env else f"{src_path}{os.pathsep}{env['PYTHONPATH']}" - # We use -u for unbuffered output to ensure we can read the log lines immediately cmd = [ sys.executable, "-u", @@ -46,7 +120,7 @@ def run_audit(): "--port", str(port), "--connection-file", - connection_file, + str(connection_file), "--tunnel", "none", "--public-url", @@ -56,77 +130,57 @@ def run_audit(): process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, bufsize=1, env=env, preexec_fn=os.setsid ) - - setup_url = None - start_time = time.time() - timeout = 15 - - print("Waiting for Agent Setup URL...") - try: - # Read output line by line - while time.time() - start_time < timeout: - line = process.stdout.readline() - if not line: - break - # Print log line for CI visibility - sys.stdout.write(f"SERVER: {redact_setup_tokens(line)}") - sys.stdout.flush() - - if "Agent setup URL:" in line: - setup_url = line.split("Agent setup URL:")[1].strip() - break - except Exception as e: - print(f"Error reading output: {e}") + threading.Thread(target=drain, args=(process.stdout,), daemon=True).start() success = False - if setup_url: - print("\nSUCCESS: Captured Agent Setup URL (credential redacted)") - - # Verify we can fetch the connection file using the setup URL - print("Verifying setup URL access...") - try: - # Add a small delay to ensure server is ready - time.sleep(1) - with urllib.request.urlopen(setup_url) as response: - data = json.loads(response.read().decode()) - print("SUCCESS: Retrieved connection data via setup URL") - - # Extract MCP details - mcp_url = data["mcp"]["url"] - auth_token = data["mcp"]["headers"].get("Authorization") - - print(f"MCP URL: {mcp_url}") - print(f"Auth Token present: {bool(auth_token)}") - - if auth_token and "Bearer" in auth_token: - success = True - print("SUCCESS: Connection JSON is valid and contains credentials") - else: - print("FAIL: Connection data missing Bearer token") - except Exception as e: - print(f"FAIL: Could not fetch connection data: {e}") - else: - print("\nFAIL: Could not find Agent Setup URL in output within timeout") - - print(f"--- Audit Complete: {'SUCCESS' if success else 'FAIL'} ---") - - # Cleanup: Kill the process group try: - os.killpg(os.getpgid(process.pid), signal.SIGTERM) - process.wait(timeout=5) - except (OSError, subprocess.TimeoutExpired): + print(f"Waiting up to {CONNECTION_TIMEOUT}s for the connection file...") + data = wait_for_connection(connection_file, time.time() + CONNECTION_TIMEOUT) + + if data is None: + print(f"FAIL: no readable connection file after {CONNECTION_TIMEOUT}s") + else: + mcp = data.get("mcp") or {} + mcp_url = mcp.get("url") + health_url = mcp.get("health_url") + auth_token = (mcp.get("headers") or {}).get("Authorization") + + print(f"MCP URL: {mcp_url}") + print(f"Auth token present: {bool(auth_token)}") + + if not mcp_url: + print("FAIL: connection file has no mcp.url") + elif not (auth_token and "Bearer" in auth_token): + print("FAIL: connection file has no Bearer token") + elif not health_url: + print("FAIL: connection file has no mcp.health_url") + else: + # A file on disk only proves the launcher wrote something. Answering on the + # advertised health URL is what proves an agent could actually reach it. + try: + with urllib.request.urlopen(health_url, timeout=HEALTH_TIMEOUT) as response: + code = response.status + print(f"Health check: HTTP {code}") + success = code == 200 + if not success: + print(f"FAIL: health URL answered {code}") + except Exception as exc: # noqa: BLE001 - any failure to reach it is a failure + print(f"FAIL: could not reach {health_url}: {type(exc).__name__}: {exc}") + finally: with contextlib.suppress(OSError): - os.killpg(os.getpgid(process.pid), signal.SIGKILL) - - # Cleanup connection file - if Path(connection_file).exists(): - Path(connection_file).unlink() + os.killpg(os.getpgid(process.pid), signal.SIGTERM) + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + with contextlib.suppress(OSError): + os.killpg(os.getpgid(process.pid), signal.SIGKILL) + if connection_file.exists(): + with contextlib.suppress(OSError): + connection_file.unlink() + print(f"--- Audit Complete: {'SUCCESS' if success else 'FAIL'} ---") return success if __name__ == "__main__": - if run_audit(): - sys.exit(0) - else: - sys.exit(1) + sys.exit(0 if run_audit() else 1)