From 548b860e530b86831069746595caa4669f171815 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 12 Aug 2026 02:39:35 -0400 Subject: [PATCH 1/3] feat: probe capability-gated session close --- .github/workflows/daily-protocol-matrix.yml | 4 + .github/workflows/protocol_matrix.py | 250 ++++++-- .../workflows/tests/test_protocol_matrix.py | 16 +- .../tests/test_protocol_matrix_close.py | 577 ++++++++++++++++++ 4 files changed, 801 insertions(+), 46 deletions(-) create mode 100644 .github/workflows/tests/test_protocol_matrix_close.py diff --git a/.github/workflows/daily-protocol-matrix.yml b/.github/workflows/daily-protocol-matrix.yml index 39120d2ca..195d68e83 100644 --- a/.github/workflows/daily-protocol-matrix.yml +++ b/.github/workflows/daily-protocol-matrix.yml @@ -58,6 +58,10 @@ jobs: with: node-version: "lts/*" + - name: Test protocol matrix close planning + working-directory: .github/workflows + run: python3 -m unittest tests.test_protocol_matrix_close -v + - name: Generate matrix env: CI: "1" diff --git a/.github/workflows/protocol_matrix.py b/.github/workflows/protocol_matrix.py index e08664b53..bce51b0cc 100644 --- a/.github/workflows/protocol_matrix.py +++ b/.github/workflows/protocol_matrix.py @@ -6,7 +6,7 @@ 1. Launching each registered agent 2. Running `initialize` 3. Running a basic `session/new` check -4. Probing selected unstable methods +4. Probing selected feature methods Outputs: - .protocol-matrix/snapshots/YYYY-MM-DD.json @@ -19,7 +19,6 @@ import argparse import copy -import fcntl import json import os import select @@ -27,11 +26,17 @@ import subprocess import sys import time +from collections.abc import Callable from dataclasses import asdict, dataclass from datetime import datetime, timezone from pathlib import Path from typing import Any +try: + import fcntl +except ImportError: # pragma: no cover - unavailable on Windows + fcntl = None + from registry_utils import subprocess_group_kwargs, terminate_process_group from verify_agents import ( build_agent_command, @@ -49,6 +54,7 @@ DEFAULT_OUTPUT_DIR = ".protocol-matrix" DEFAULT_TABLE_MODE = "full" PROTOCOL_VERSION = 1 +PROBE_SCHEMA_VERSION = 2 TABLE_MODE_CHOICES = ("full", "capabilities") @@ -56,8 +62,8 @@ "session/list", "session/fork", "session/resume", - "session/stop", "session/set_model", + "session/close", ) SUCCESS_STATUSES = {"success", "invalid_params", "resource_not_found"} @@ -67,7 +73,7 @@ ("sessionList", "session/list"), ("sessionFork", "session/fork"), ("sessionResume", "session/resume"), - ("sessionStop", "session/stop"), + ("sessionClose", "session/close"), ) @@ -80,6 +86,12 @@ class ProbeOutcome: message: str | None = None +ProbeRequest = Callable[ + [int, str, dict[str, Any], float], + tuple[ProbeOutcome, dict[str, Any] | None], +] + + def short_message(message: str, max_len: int = 220) -> str: """Compact long messages for JSON/markdown outputs.""" compact = " ".join(message.split()) @@ -165,6 +177,14 @@ def index_snapshot_agents(snapshot: dict[str, Any] | None) -> dict[str, dict[str return indexed +def snapshot_schema_is_current(snapshot: dict[str, Any] | None) -> bool: + """Return whether a previous snapshot can be reused without translation.""" + if snapshot is None: + return False + version = snapshot.get("probeSchemaVersion") + return type(version) is int and version == PROBE_SCHEMA_VERSION + + def should_probe_agent( agent: dict[str, Any], previous_record: dict[str, Any] | None, @@ -233,6 +253,15 @@ def capability_present(capabilities: dict[str, Any], key: str) -> bool: return key in capabilities and capabilities[key] is not None +def close_capability_advertised(session_capabilities: dict[str, Any]) -> bool: + """Return whether stable ACP v1 close support is advertised as an object.""" + close_capability = session_capabilities.get("close") + if not isinstance(close_capability, dict): + return False + meta = close_capability.get("_meta") + return "_meta" not in close_capability or meta is None or isinstance(meta, dict) + + def build_initialize_params() -> dict[str, Any]: """Build the client initialize payload used for protocol probing.""" return { @@ -468,6 +497,9 @@ def collect_stderr_tail(proc: subprocess.Popen, max_chars: int = 1200) -> str | if proc.poll() is None: return None + if fcntl is None: + return None + fd = proc.stderr.fileno() try: flags = fcntl.fcntl(fd, fcntl.F_GETFL) @@ -514,7 +546,7 @@ def probe_params_for_method( "cwd": cwd, "mcpServers": [], } - if method == "session/stop": + if method == "session/close": return {"sessionId": session_id} if method == "session/set_model": return { @@ -535,12 +567,18 @@ def status_short(status: str) -> str: "no_response": "timeout", "decode_error": "decode", "process_error": "proc_err", + "invalid_response": "invalid", + "not_applicable": "n/a", "not_probed": "-", } return mapping.get(status, "err") -def feature_cell(advertised: bool, outcome: ProbeOutcome) -> str: +def feature_cell( + advertised: bool, + outcome: ProbeOutcome, + method: str | None = None, +) -> str: """Render `signal/probe` feature status, e.g. `Y/yes`, `N/no`.""" advertised_part = "Y" if advertised else "N" @@ -552,9 +590,17 @@ def feature_cell(advertised: bool, outcome: ProbeOutcome) -> str: "method_not_found": "no", "no_response": "timeout", "decode_error": "decode", + "invalid_response": "invalid", + "not_applicable": "n/a", "not_probed": "-", } - return f"{advertised_part}/{probe_map.get(outcome.status, 'err')}" + probe_value = probe_map.get(outcome.status, "err") + if method == "session/close" and outcome.status in { + "invalid_params", + "resource_not_found", + }: + probe_value = status_short(outcome.status) + return f"{advertised_part}/{probe_value}" def format_capabilities(capabilities: dict[str, bool]) -> str: @@ -563,6 +609,75 @@ def format_capabilities(capabilities: dict[str, bool]) -> str: return ", ".join(advertised) if advertised else "-" +def normalize_close_outcome( + outcome: ProbeOutcome, + message: dict[str, Any] | None, +) -> ProbeOutcome: + """Require a successful close response to contain an object result.""" + if outcome.status != "success": + return outcome + if message is None or not isinstance(message.get("result"), dict): + return ProbeOutcome( + status="invalid_response", + message="session/close result must be an object", + ) + + result = message["result"] + meta = result.get("_meta") + if "_meta" in result and meta is not None and not isinstance(meta, dict): + return ProbeOutcome( + status="invalid_response", + message="session/close result _meta must be an object or null", + ) + return outcome + + +def run_method_probes( + request: ProbeRequest, + request_id: int, + probe_session_id: str, + close_session_id: str | None, + cwd: str, + timeout: float, + close_advertised: bool, +) -> tuple[int, dict[str, ProbeOutcome], bool]: + """Run feature probes while keeping close gated, real-session-only, and last.""" + outcomes = {method: ProbeOutcome(status="not_probed") for method in METHOD_PROBES} + set_model_signal = False + + for method in METHOD_PROBES: + session_id = probe_session_id + if method == "session/close": + if not close_advertised: + outcomes[method] = ProbeOutcome(status="not_applicable") + continue + if close_session_id is None: + outcomes[method] = ProbeOutcome( + status="not_probed", + message="No active session was created", + ) + continue + session_id = close_session_id + + params = probe_params_for_method(method=method, session_id=session_id, cwd=cwd) + outcome, message = request(request_id, method, params, timeout) + if method == "session/close": + outcome = normalize_close_outcome(outcome, message) + if response_exposes_models(message): + set_model_signal = True + outcomes[method] = outcome + request_id += 1 + + return request_id, outcomes, set_model_signal + + +def probe_indicates_support(method: str, status: str) -> bool: + """Interpret probe outcomes without treating close errors as support.""" + if method == "session/close": + return status == "success" + return status in SUCCESS_STATUSES + + def render_aligned_table(headers: list[str], rows: list[list[str]]) -> list[str]: """Render a fixed-width table inside a markdown code block.""" widths = [len(header) for header in headers] @@ -599,6 +714,8 @@ def summarize_results(records: list[dict[str, Any]]) -> dict[str, Any]: "supported": 0, "authRequired": 0, "methodNotFound": 0, + "notApplicable": 0, + "notProbed": 0, "other": 0, } @@ -619,18 +736,42 @@ def summarize_results(records: list[dict[str, Any]]) -> dict[str, Any]: for method in METHOD_PROBES: status = record["methodProbes"][method]["status"] counters = summary["features"][method] - if status in SUCCESS_STATUSES: + if probe_indicates_support(method, status): counters["supported"] += 1 elif status == "auth_required": counters["authRequired"] += 1 elif status == "method_not_found": counters["methodNotFound"] += 1 + elif status == "not_applicable": + counters["notApplicable"] += 1 + elif status == "not_probed": + counters["notProbed"] += 1 else: counters["other"] += 1 return summary +def build_snapshot( + records: list[dict[str, Any]], + summary: dict[str, Any], + date_str: str, + generated_at: str, + table_mode: str, + changed_only: bool, +) -> dict[str, Any]: + """Build the versioned machine-readable matrix report.""" + return { + "probeSchemaVersion": PROBE_SCHEMA_VERSION, + "date": date_str, + "generatedAt": generated_at, + "tableMode": table_mode, + "changedOnly": changed_only, + "summary": summary, + "agents": records, + } + + def render_markdown( records: list[dict[str, Any]], summary: dict[str, Any], @@ -647,6 +788,7 @@ def render_markdown( "", f"_Generated at {generated_at}_", "", + f"- Probe schema version: **{PROBE_SCHEMA_VERSION}**", f"- Agents in report: **{summary['agentsProbed']}**", f"- Probed this run: **{summary['agentsProbedThisRun']}**", f"- Reused unchanged versions: **{summary['agentsReused']}**", @@ -671,10 +813,11 @@ def render_markdown( [ ( "Legend: feature cells use `Signal/Probe` format. " - "For `session/list`, `session/fork`, `session/resume`, and `session/stop`, " + "For `session/list`, `session/fork`, `session/resume`, and `session/close`, " "`Y`/`N` means the capability was advertised. For `session/set_model`, " "`Y`/`N` means session responses exposed `models`. Probe values: " - "`yes`, `no`, `auth`, `timeout`, `decode`, `err`, `-`." + "`yes`, `no`, `auth`, `params`, `missing`, `n/a`, `timeout`, " + "`decode`, `invalid`, `err`, `-`." ), ( "`Capabilities` lists the capabilities advertised in the " @@ -721,7 +864,11 @@ def render_markdown( feature_cell(caps["sessionList"], ProbeOutcome(**probes["session/list"])), feature_cell(caps["sessionFork"], ProbeOutcome(**probes["session/fork"])), feature_cell(caps["sessionResume"], ProbeOutcome(**probes["session/resume"])), - feature_cell(caps["sessionStop"], ProbeOutcome(**probes["session/stop"])), + feature_cell( + caps["sessionClose"], + ProbeOutcome(**probes["session/close"]), + method="session/close", + ), feature_cell( set_model_advertised, ProbeOutcome(**probes["session/set_model"]), @@ -742,7 +889,7 @@ def render_markdown( "session/list", "session/fork", "session/resume", - "session/stop", + "session/close", "session/set_model", ] @@ -753,8 +900,11 @@ def render_markdown( "", "## Method Probe Summary", "", - "| Method | Supported | Auth Required | Method Not Found | Other |", - "| --- | ---: | ---: | ---: | ---: |", + ( + "| Method | Supported | Auth Required | Method Not Found | " + "Not Applicable | Not Probed | Other |" + ), + "| --- | ---: | ---: | ---: | ---: | ---: | ---: |", ] ) @@ -768,6 +918,8 @@ def render_markdown( str(counters["supported"]), str(counters["authRequired"]), str(counters["methodNotFound"]), + str(counters["notApplicable"]), + str(counters["notProbed"]), str(counters["other"]), ] ) @@ -846,7 +998,7 @@ def probe_agent( "sessionList": False, "sessionFork": False, "sessionResume": False, - "sessionStop": False, + "sessionClose": False, "setModel": False, }, "sessionNew": asdict(ProbeOutcome(status="not_probed")), @@ -931,6 +1083,7 @@ def probe_agent( default_row["initialize"] = asdict(init_outcome) session_id = "sess_matrix_probe" + close_session_id = None if init_outcome.status == "success" and init_message and "result" in init_message: result = init_message["result"] if isinstance(result, dict): @@ -955,7 +1108,7 @@ def probe_agent( "sessionList": capability_present(session_caps, "list"), "sessionFork": capability_present(session_caps, "fork"), "sessionResume": capability_present(session_caps, "resume"), - "sessionStop": capability_present(session_caps, "stop"), + "sessionClose": close_capability_advertised(session_caps), "setModel": False, } @@ -982,25 +1135,37 @@ def probe_agent( maybe_session_id = session_result.get("sessionId") if isinstance(maybe_session_id, str) and maybe_session_id: session_id = maybe_session_id - - for method in METHOD_PROBES: - params = probe_params_for_method( - method=method, - session_id=session_id, - cwd=str(workspace_dir), - ) - outcome, message = request_with_timeout( + close_session_id = maybe_session_id + + def request_probe( + probe_request_id: int, + method: str, + params: dict[str, Any], + timeout: float, + ) -> tuple[ProbeOutcome, dict[str, Any] | None]: + return request_with_timeout( proc, - request_id, + probe_request_id, method, params, - rpc_timeout, + timeout, ) - if response_exposes_models(message): - default_row["setModelSignal"] = True - default_row["capabilities"]["setModel"] = True - default_row["methodProbes"][method] = asdict(outcome) - request_id += 1 + + _, probe_outcomes, probes_expose_models = run_method_probes( + request=request_probe, + request_id=request_id, + probe_session_id=session_id, + close_session_id=close_session_id, + cwd=str(workspace_dir), + timeout=rpc_timeout, + close_advertised=default_row["capabilities"]["sessionClose"], + ) + default_row["methodProbes"] = { + method: asdict(outcome) for method, outcome in probe_outcomes.items() + } + if probes_expose_models: + default_row["setModelSignal"] = True + default_row["capabilities"]["setModel"] = True except Exception as exc: # noqa: BLE001 default_row["initialize"] = asdict( ProbeOutcome( @@ -1115,6 +1280,13 @@ def main() -> int: agents.sort(key=lambda item: item["id"]) latest_json_path = output_base / "latest.json" previous_snapshot = load_previous_snapshot(latest_json_path) if args.changed_only else None + if previous_snapshot is not None and not snapshot_schema_is_current(previous_snapshot): + previous_version = previous_snapshot.get("probeSchemaVersion", 1) + print( + "Previous snapshot probe schema " + f"{previous_version!r} is incompatible with {PROBE_SCHEMA_VERSION}; probing all agents" + ) + previous_snapshot = None previous_records = index_snapshot_agents(previous_snapshot) previous_generated_at = None if previous_snapshot is not None: @@ -1153,14 +1325,14 @@ def main() -> int: records.append(record) summary = summarize_results(records) - snapshot = { - "date": date_str, - "generatedAt": generated_at, - "tableMode": args.table_mode, - "changedOnly": args.changed_only, - "summary": summary, - "agents": records, - } + snapshot = build_snapshot( + records=records, + summary=summary, + date_str=date_str, + generated_at=generated_at, + table_mode=args.table_mode, + changed_only=args.changed_only, + ) markdown = render_markdown( records, diff --git a/.github/workflows/tests/test_protocol_matrix.py b/.github/workflows/tests/test_protocol_matrix.py index 5a890496b..b7a0c62b6 100644 --- a/.github/workflows/tests/test_protocol_matrix.py +++ b/.github/workflows/tests/test_protocol_matrix.py @@ -36,7 +36,7 @@ def make_record( list_status: str = "success", fork_status: str = "method_not_found", resume_status: str = "method_not_found", - stop_status: str = "method_not_found", + close_status: str = "method_not_found", set_model_status: str = "method_not_found", set_model_signal: bool = False, reused_from_previous: bool = False, @@ -57,14 +57,14 @@ def make_record( "sessionList": True, "sessionFork": False, "sessionResume": False, - "sessionStop": False, + "sessionClose": False, "setModel": False, }, "methodProbes": { "session/list": {"status": list_status, "code": None, "message": None}, "session/fork": {"status": fork_status, "code": None, "message": None}, "session/resume": {"status": resume_status, "code": None, "message": None}, - "session/stop": {"status": stop_status, "code": None, "message": None}, + "session/close": {"status": close_status, "code": None, "message": None}, "session/set_model": {"status": set_model_status, "code": None, "message": None}, }, "reusedFromPrevious": reused_from_previous, @@ -203,7 +203,7 @@ def test_probe_params_for_methods_match_schema(): "cwd": cwd, "mcpServers": [], } - assert probe_params_for_method("session/stop", session_id, cwd) == { + assert probe_params_for_method("session/close", session_id, cwd) == { "sessionId": session_id, } assert probe_params_for_method("session/set_model", session_id, cwd) == { @@ -325,7 +325,7 @@ def test_format_capabilities_lists_advertised_initialize_capabilities(): "sessionList": True, "sessionFork": False, "sessionResume": True, - "sessionStop": False, + "sessionClose": False, } ) @@ -339,7 +339,7 @@ def test_summarize_results_counts_statuses(): list_status="success", fork_status="auth_required", resume_status="method_not_found", - stop_status="error", + close_status="error", set_model_status="invalid_params", ) ] @@ -352,7 +352,7 @@ def test_summarize_results_counts_statuses(): assert summary["features"]["session/list"]["supported"] == 1 assert summary["features"]["session/fork"]["authRequired"] == 1 assert summary["features"]["session/resume"]["methodNotFound"] == 1 - assert summary["features"]["session/stop"]["other"] == 1 + assert summary["features"]["session/close"]["other"] == 1 assert summary["features"]["session/set_model"]["supported"] == 1 @@ -382,6 +382,8 @@ def test_render_markdown_full_mode_contains_matrix_headers_and_signal_legend(): assert "Capabilities" in header_line assert "session/new" in header_line assert "session/list" in header_line + assert "session/close" in header_line + assert "session/stop" not in md assert "session/set_model" in header_line assert "agent-1" in md assert "1.2.3" in md diff --git a/.github/workflows/tests/test_protocol_matrix_close.py b/.github/workflows/tests/test_protocol_matrix_close.py new file mode 100644 index 000000000..ea2aae967 --- /dev/null +++ b/.github/workflows/tests/test_protocol_matrix_close.py @@ -0,0 +1,577 @@ +"""Focused standard-library tests for capability-aware session closing.""" + +import json +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +from protocol_matrix import ( + METHOD_PROBES, + PROBE_SCHEMA_VERSION, + ProbeOutcome, + build_snapshot, + close_capability_advertised, + feature_cell, + main, + probe_agent, + probe_indicates_support, + render_markdown, + reuse_previous_record, + run_method_probes, + snapshot_schema_is_current, + summarize_results, +) + + +def make_record(close_status: str = "not_applicable") -> dict: + """Build one schema-v2 result row for rendering and summary tests.""" + return { + "id": "agent-1", + "name": "Agent One", + "registryVersion": "1.2.3", + "repository": None, + "website": None, + "distribution": "npx", + "initialize": {"status": "success", "code": None, "message": None}, + "protocolVersion": 1, + "agentInfoVersion": "1.2.3", + "authMethods": ["agent"], + "setModelSignal": False, + "capabilities": { + "loadSession": True, + "sessionList": True, + "sessionFork": False, + "sessionResume": False, + "sessionClose": close_status != "not_applicable", + "setModel": False, + }, + "sessionNew": {"status": "success", "code": None, "message": None}, + "methodProbes": { + "session/list": {"status": "success", "code": None, "message": None}, + "session/fork": { + "status": "method_not_found", + "code": -32601, + "message": "Method not found", + }, + "session/resume": { + "status": "method_not_found", + "code": -32601, + "message": "Method not found", + }, + "session/set_model": { + "status": "invalid_params", + "code": -32602, + "message": "Unknown model", + }, + "session/close": {"status": close_status, "code": None, "message": None}, + }, + "stderrTail": None, + "commandPreview": "npx agent", + "workspaceCwd": "/tmp/workspace", + "durationSeconds": 0.1, + "processExitCode": 0, + "probedAt": "2026-08-12T06:00:00+00:00", + "reusedFromPrevious": False, + } + + +class CloseCapabilityTests(unittest.TestCase): + def test_only_present_objects_advertise_close(self): + self.assertTrue(close_capability_advertised({"close": {}})) + self.assertTrue(close_capability_advertised({"close": {"_meta": None}})) + self.assertTrue(close_capability_advertised({"close": {"_meta": {"vendor": "example"}}})) + for capabilities in ( + {}, + {"close": None}, + {"close": True}, + {"close": "yes"}, + {"close": {"_meta": "invalid"}}, + ): + with self.subTest(capabilities=capabilities): + self.assertFalse(close_capability_advertised(capabilities)) + + def test_advertised_close_runs_once_last_with_created_session_id(self): + calls = [] + + def request(request_id, method, params, timeout): + calls.append((request_id, method, params, timeout)) + result = {"_meta": {"trace": "ok"}} if method == "session/close" else {} + return ProbeOutcome(status="success"), {"result": result} + + next_id, outcomes, exposes_models = run_method_probes( + request=request, + request_id=10, + probe_session_id="legacy-fallback", + close_session_id="created-session", + cwd="/tmp/workspace", + timeout=2.5, + close_advertised=True, + ) + + self.assertEqual([method for _, method, _, _ in calls], list(METHOD_PROBES)) + self.assertEqual(calls[-1][1], "session/close") + self.assertEqual(calls[-1][2], {"sessionId": "created-session"}) + self.assertEqual(sum(method == "session/close" for _, method, _, _ in calls), 1) + self.assertEqual(outcomes["session/close"].status, "success") + self.assertEqual(next_id, 15) + self.assertFalse(exposes_models) + + def test_probe_agent_wires_capability_and_created_session_id_to_close(self): + calls = [] + + def request_with_timeout(proc, request_id, method, params, timeout): + calls.append((request_id, method, params, timeout)) + if method == "initialize": + return ProbeOutcome(status="success"), { + "result": { + "protocolVersion": 1, + "agentCapabilities": { + "sessionCapabilities": {"close": {"_meta": {"source": "test"}}} + }, + } + } + if method == "session/new": + return ProbeOutcome(status="success"), {"result": {"sessionId": "created-session"}} + return ProbeOutcome(status="success"), {"result": {}} + + fake_process = SimpleNamespace(returncode=0) + agent = { + "id": "agent-1", + "name": "Agent One", + "version": "1.2.3", + "distribution": {"npx": {"package": "agent-one"}}, + } + + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("protocol_matrix.ensure_distribution_runtime", return_value=None), + patch( + "protocol_matrix.build_agent_command", + return_value=(["fake-agent"], None, {}), + ), + patch("protocol_matrix.build_agent_process_env", return_value={}), + patch("protocol_matrix.subprocess.Popen", return_value=fake_process), + patch( + "protocol_matrix.request_with_timeout", + side_effect=request_with_timeout, + ), + patch("protocol_matrix.stop_process"), + patch("protocol_matrix.collect_stderr_tail", return_value=None), + ): + record = probe_agent( + agent=agent, + sandbox_base=Path(temp_dir), + init_timeout=5.0, + rpc_timeout=1.0, + ) + + self.assertTrue(record["capabilities"]["sessionClose"]) + self.assertEqual(record["methodProbes"]["session/close"]["status"], "success") + self.assertEqual(calls[-1][1], "session/close") + self.assertEqual(calls[-1][2], {"sessionId": "created-session"}) + self.assertNotEqual(calls[-1][2]["sessionId"], "sess_matrix_probe") + + def test_probe_agent_does_not_close_when_session_creation_fails(self): + agent = { + "id": "agent-1", + "name": "Agent One", + "version": "1.2.3", + "distribution": {"npx": {"package": "agent-one"}}, + } + + for session_new_status in ("error", "auth_required"): + with self.subTest(session_new_status=session_new_status): + calls = [] + + def request_with_timeout( + proc, + request_id, + method, + params, + timeout, + *, + calls=calls, + session_new_status=session_new_status, + ): + calls.append((request_id, method, params, timeout)) + if method == "initialize": + return ProbeOutcome(status="success"), { + "result": { + "protocolVersion": 1, + "agentCapabilities": {"sessionCapabilities": {"close": {}}}, + } + } + if method == "session/new": + return ProbeOutcome(status=session_new_status), None + return ProbeOutcome(status="success"), {"result": {}} + + fake_process = SimpleNamespace(returncode=0) + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("protocol_matrix.ensure_distribution_runtime", return_value=None), + patch( + "protocol_matrix.build_agent_command", + return_value=(["fake-agent"], None, {}), + ), + patch("protocol_matrix.build_agent_process_env", return_value={}), + patch("protocol_matrix.subprocess.Popen", return_value=fake_process), + patch( + "protocol_matrix.request_with_timeout", + side_effect=request_with_timeout, + ), + patch("protocol_matrix.stop_process"), + patch("protocol_matrix.collect_stderr_tail", return_value=None), + ): + record = probe_agent( + agent=agent, + sandbox_base=Path(temp_dir), + init_timeout=5.0, + rpc_timeout=1.0, + ) + + methods = [method for _, method, _, _ in calls] + self.assertNotIn("session/close", methods) + self.assertEqual(record["sessionNew"]["status"], session_new_status) + self.assertEqual( + record["methodProbes"]["session/close"]["status"], + "not_probed", + ) + + def test_probe_agent_does_not_close_when_capability_is_omitted_or_null(self): + agent = { + "id": "agent-1", + "name": "Agent One", + "version": "1.2.3", + "distribution": {"npx": {"package": "agent-one"}}, + } + + for session_capabilities in ({}, {"close": None}): + with self.subTest(session_capabilities=session_capabilities): + calls = [] + + def request_with_timeout( + proc, + request_id, + method, + params, + timeout, + *, + calls=calls, + session_capabilities=session_capabilities, + ): + calls.append((request_id, method, params, timeout)) + if method == "initialize": + return ProbeOutcome(status="success"), { + "result": { + "protocolVersion": 1, + "agentCapabilities": {"sessionCapabilities": session_capabilities}, + } + } + if method == "session/new": + return ProbeOutcome(status="success"), { + "result": {"sessionId": "created-session"} + } + return ProbeOutcome(status="success"), {"result": {}} + + fake_process = SimpleNamespace(returncode=0) + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("protocol_matrix.ensure_distribution_runtime", return_value=None), + patch( + "protocol_matrix.build_agent_command", + return_value=(["fake-agent"], None, {}), + ), + patch("protocol_matrix.build_agent_process_env", return_value={}), + patch("protocol_matrix.subprocess.Popen", return_value=fake_process), + patch( + "protocol_matrix.request_with_timeout", + side_effect=request_with_timeout, + ), + patch("protocol_matrix.stop_process"), + patch("protocol_matrix.collect_stderr_tail", return_value=None), + ): + record = probe_agent( + agent=agent, + sandbox_base=Path(temp_dir), + init_timeout=5.0, + rpc_timeout=1.0, + ) + + self.assertNotIn("session/close", [method for _, method, _, _ in calls]) + self.assertFalse(record["capabilities"]["sessionClose"]) + self.assertEqual( + record["methodProbes"]["session/close"]["status"], + "not_applicable", + ) + + def test_unadvertised_close_is_not_applicable_and_makes_no_call(self): + calls = [] + + def request(request_id, method, params, timeout): + calls.append((request_id, method, params, timeout)) + return ProbeOutcome(status="success"), {"result": {}} + + next_id, outcomes, _ = run_method_probes( + request=request, + request_id=20, + probe_session_id="legacy-fallback", + close_session_id="created-session", + cwd="/tmp/workspace", + timeout=1.0, + close_advertised=False, + ) + + self.assertNotIn("session/close", [method for _, method, _, _ in calls]) + self.assertEqual(outcomes["session/close"].status, "not_applicable") + self.assertEqual(next_id, 24) + + def test_advertised_close_without_active_session_is_not_probed(self): + calls = [] + + def request(request_id, method, params, timeout): + calls.append((request_id, method, params, timeout)) + return ProbeOutcome(status="success"), {"result": {}} + + next_id, outcomes, _ = run_method_probes( + request=request, + request_id=30, + probe_session_id="legacy-fallback", + close_session_id=None, + cwd="/tmp/workspace", + timeout=1.0, + close_advertised=True, + ) + + self.assertNotIn("session/close", [method for _, method, _, _ in calls]) + self.assertEqual(outcomes["session/close"].status, "not_probed") + self.assertIn("No active session", outcomes["session/close"].message or "") + self.assertEqual(next_id, 34) + + def test_close_error_remains_visible_and_does_not_count_as_support(self): + for status, code, message, expected_cell in ( + ("invalid_params", -32602, "Invalid params", "Y/params"), + ("resource_not_found", -32002, "Session not found", "Y/missing"), + ): + with self.subTest(status=status): + + def request( + request_id, + method, + params, + timeout, + *, + status=status, + code=code, + message=message, + ): + if method == "session/close": + return ( + ProbeOutcome(status=status, code=code, message=message), + {"error": {"code": code, "message": message}}, + ) + return ProbeOutcome(status="success"), {"result": {}} + + _, outcomes, _ = run_method_probes( + request=request, + request_id=40, + probe_session_id="legacy-fallback", + close_session_id="created-session", + cwd="/tmp/workspace", + timeout=1.0, + close_advertised=True, + ) + + close_outcome = outcomes["session/close"] + self.assertEqual(close_outcome.status, status) + self.assertEqual(close_outcome.code, code) + self.assertEqual(close_outcome.message, message) + self.assertFalse(probe_indicates_support("session/close", close_outcome.status)) + self.assertTrue(probe_indicates_support("session/list", status)) + self.assertEqual( + feature_cell(True, close_outcome, method="session/close"), + expected_cell, + ) + + record = make_record(close_status=status) + record["methodProbes"]["session/close"] = { + "status": status, + "code": code, + "message": message, + } + close_summary = summarize_results([record])["features"]["session/close"] + self.assertEqual(close_summary["supported"], 0) + self.assertEqual(close_summary["other"], 1) + + def test_close_success_requires_an_object_response(self): + def request(request_id, method, params, timeout): + result = None if method == "session/close" else {} + return ProbeOutcome(status="success"), {"result": result} + + _, outcomes, _ = run_method_probes( + request=request, + request_id=50, + probe_session_id="legacy-fallback", + close_session_id="created-session", + cwd="/tmp/workspace", + timeout=1.0, + close_advertised=True, + ) + + self.assertEqual(outcomes["session/close"].status, "invalid_response") + + def test_close_success_rejects_invalid_meta(self): + def request(request_id, method, params, timeout): + result = {"_meta": "invalid"} if method == "session/close" else {} + return ProbeOutcome(status="success"), {"result": result} + + _, outcomes, _ = run_method_probes( + request=request, + request_id=60, + probe_session_id="legacy-fallback", + close_session_id="created-session", + cwd="/tmp/workspace", + timeout=1.0, + close_advertised=True, + ) + + self.assertEqual(outcomes["session/close"].status, "invalid_response") + + +class ProbeSchemaTests(unittest.TestCase): + def test_only_schema_2_snapshot_is_reusable(self): + self.assertTrue(snapshot_schema_is_current({"probeSchemaVersion": 2, "agents": []})) + for snapshot in ( + None, + {"agents": []}, + {"probeSchemaVersion": 1, "agents": []}, + {"probeSchemaVersion": 3, "agents": []}, + {"probeSchemaVersion": 2.0, "agents": []}, + {"probeSchemaVersion": "2", "agents": []}, + ): + with self.subTest(snapshot=snapshot): + self.assertFalse(snapshot_schema_is_current(snapshot)) + + def test_snapshot_and_markdown_emit_close_schema_without_stop_keys(self): + records = [make_record()] + summary = summarize_results(records) + snapshot = build_snapshot( + records=records, + summary=summary, + date_str="2026-08-12", + generated_at="2026-08-12T06:00:00+00:00", + table_mode="full", + changed_only=True, + ) + markdown = render_markdown( + records, + summary, + "2026-08-12", + "2026-08-12T06:00:00+00:00", + ) + serialized = json.dumps(snapshot) + + self.assertEqual(snapshot["probeSchemaVersion"], PROBE_SCHEMA_VERSION) + self.assertIn('"sessionClose"', serialized) + self.assertIn('"session/close"', serialized) + self.assertNotIn("sessionStop", serialized) + self.assertNotIn("session/stop", serialized) + self.assertIn("Probe schema version: **2**", markdown) + self.assertIn("session/close", markdown) + self.assertNotIn("session/stop", markdown) + + def test_summary_distinguishes_not_applicable_and_not_probed(self): + not_applicable = make_record(close_status="not_applicable") + not_probed = make_record(close_status="not_probed") + summary = summarize_results([not_applicable, not_probed]) + close_summary = summary["features"]["session/close"] + + self.assertEqual(close_summary["supported"], 0) + self.assertEqual(close_summary["notApplicable"], 1) + self.assertEqual(close_summary["notProbed"], 1) + + def test_reuse_deep_copies_previous_schema_2_record(self): + previous = make_record(close_status="success") + original_name = previous["name"] + reused = reuse_previous_record( + { + "id": "agent-1", + "name": "Renamed Agent", + "version": "1.2.3", + "repository": "https://example.com/repository", + "website": None, + }, + previous, + "npx", + "2026-08-11T06:00:00+00:00", + ) + + self.assertEqual(previous["name"], original_name) + self.assertFalse(previous["reusedFromPrevious"]) + self.assertEqual(reused["name"], "Renamed Agent") + self.assertTrue(reused["reusedFromPrevious"]) + + def test_main_reuses_only_schema_2_rows(self): + agent = { + "id": "agent-1", + "name": "Agent One", + "version": "1.2.3", + "repository": None, + "website": None, + "distribution": {"npx": {"package": "agent-one"}}, + } + + for schema_version, should_reuse in ( + (2, True), + (None, False), + (1, False), + (3, False), + ): + with ( + self.subTest(schema_version=schema_version), + tempfile.TemporaryDirectory() as temp_dir, + ): + output_dir = Path(temp_dir) / "matrix" + output_dir.mkdir() + previous_snapshot = { + "generatedAt": "2026-08-11T06:00:00+00:00", + "agents": [make_record(close_status="success")], + } + if schema_version is not None: + previous_snapshot["probeSchemaVersion"] = schema_version + (output_dir / "latest.json").write_text( + json.dumps(previous_snapshot), encoding="utf-8" + ) + + args = SimpleNamespace( + agent=None, + skip_agent=None, + max_agents=0, + init_timeout=5.0, + rpc_timeout=1.0, + sandbox_dir=str(Path(temp_dir) / "sandbox"), + output_dir=str(output_dir), + table_mode="full", + changed_only=True, + date="2026-08-12", + ) + fresh_record = make_record(close_status="success") + with ( + patch("protocol_matrix.parse_args", return_value=args), + patch("protocol_matrix.load_registry", return_value=[agent]), + patch( + "protocol_matrix.probe_agent", + return_value=fresh_record, + ) as probe, + ): + self.assertEqual(main(), 0) + + written = json.loads((output_dir / "latest.json").read_text()) + self.assertEqual(written["probeSchemaVersion"], 2) + self.assertEqual(written["agents"][0]["reusedFromPrevious"], should_reuse) + self.assertEqual(probe.call_count, 0 if should_reuse else 1) + + +if __name__ == "__main__": + unittest.main() From 0c2725dc2c57e190490815c2b72df02e2435138b Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 12 Aug 2026 03:31:47 -0400 Subject: [PATCH 2/3] fix: reject ambiguous close responses --- .github/workflows/protocol_matrix.py | 5 ++ .../tests/test_protocol_matrix_close.py | 90 +++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/.github/workflows/protocol_matrix.py b/.github/workflows/protocol_matrix.py index bce51b0cc..7380b885b 100644 --- a/.github/workflows/protocol_matrix.py +++ b/.github/workflows/protocol_matrix.py @@ -614,6 +614,11 @@ def normalize_close_outcome( message: dict[str, Any] | None, ) -> ProbeOutcome: """Require a successful close response to contain an object result.""" + if message is not None and "result" in message and "error" in message: + return ProbeOutcome( + status="invalid_response", + message="session/close response must contain exactly one of result or error", + ) if outcome.status != "success": return outcome if message is None or not isinstance(message.get("result"), dict): diff --git a/.github/workflows/tests/test_protocol_matrix_close.py b/.github/workflows/tests/test_protocol_matrix_close.py index ea2aae967..e1d62c4a7 100644 --- a/.github/workflows/tests/test_protocol_matrix_close.py +++ b/.github/workflows/tests/test_protocol_matrix_close.py @@ -239,6 +239,72 @@ def request_with_timeout( "not_probed", ) + def test_probe_agent_does_not_close_without_a_valid_session_id(self): + agent = { + "id": "agent-1", + "name": "Agent One", + "version": "1.2.3", + "distribution": {"npx": {"package": "agent-one"}}, + } + + for session_result in ({}, {"sessionId": ""}, {"sessionId": 42}): + with self.subTest(session_result=session_result): + calls = [] + + def request_with_timeout( + proc, + request_id, + method, + params, + timeout, + *, + calls=calls, + session_result=session_result, + ): + calls.append((request_id, method, params, timeout)) + if method == "initialize": + return ProbeOutcome(status="success"), { + "result": { + "protocolVersion": 1, + "agentCapabilities": {"sessionCapabilities": {"close": {}}}, + } + } + if method == "session/new": + return ProbeOutcome(status="success"), {"result": session_result} + return ProbeOutcome(status="success"), {"result": {}} + + fake_process = SimpleNamespace(returncode=0) + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("protocol_matrix.ensure_distribution_runtime", return_value=None), + patch( + "protocol_matrix.build_agent_command", + return_value=(["fake-agent"], None, {}), + ), + patch("protocol_matrix.build_agent_process_env", return_value={}), + patch("protocol_matrix.subprocess.Popen", return_value=fake_process), + patch( + "protocol_matrix.request_with_timeout", + side_effect=request_with_timeout, + ), + patch("protocol_matrix.stop_process"), + patch("protocol_matrix.collect_stderr_tail", return_value=None), + ): + record = probe_agent( + agent=agent, + sandbox_base=Path(temp_dir), + init_timeout=5.0, + rpc_timeout=1.0, + ) + + methods = [method for _, method, _, _ in calls] + self.assertNotIn("session/close", methods) + self.assertEqual(record["sessionNew"]["status"], "success") + self.assertEqual( + record["methodProbes"]["session/close"]["status"], + "not_probed", + ) + def test_probe_agent_does_not_close_when_capability_is_omitted_or_null(self): agent = { "id": "agent-1", @@ -438,6 +504,30 @@ def request(request_id, method, params, timeout): self.assertEqual(outcomes["session/close"].status, "invalid_response") + def test_close_response_rejects_result_and_error(self): + def request(request_id, method, params, timeout): + if method == "session/close": + return ProbeOutcome(status="success"), { + "result": {}, + "error": {"code": -32602, "message": "Invalid params"}, + } + return ProbeOutcome(status="success"), {"result": {}} + + _, outcomes, _ = run_method_probes( + request=request, + request_id=70, + probe_session_id="legacy-fallback", + close_session_id="created-session", + cwd="/tmp/workspace", + timeout=1.0, + close_advertised=True, + ) + + close_outcome = outcomes["session/close"] + self.assertEqual(close_outcome.status, "invalid_response") + self.assertIn("exactly one", close_outcome.message or "") + self.assertFalse(probe_indicates_support("session/close", close_outcome.status)) + class ProbeSchemaTests(unittest.TestCase): def test_only_schema_2_snapshot_is_reusable(self): From b76b603d12908f881fa9a71ab614a556af014847 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 12 Aug 2026 03:36:34 -0400 Subject: [PATCH 3/3] fix: reject ambiguous RPC responses --- .github/workflows/protocol_matrix.py | 8 ++- .../workflows/tests/test_protocol_matrix.py | 17 ++++++ .../tests/test_protocol_matrix_close.py | 60 +++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/.github/workflows/protocol_matrix.py b/.github/workflows/protocol_matrix.py index 7380b885b..29d47d8ac 100644 --- a/.github/workflows/protocol_matrix.py +++ b/.github/workflows/protocol_matrix.py @@ -286,7 +286,7 @@ def build_initialize_params() -> dict[str, Any]: def response_exposes_models(message: dict[str, Any] | None) -> bool: """Return whether a successful response includes session model state.""" - if not message or "result" not in message: + if not message or "result" not in message or "error" in message: return False result = message["result"] @@ -295,6 +295,12 @@ def response_exposes_models(message: dict[str, Any] | None) -> bool: def classify_rpc_response(message: dict[str, Any]) -> ProbeOutcome: """Convert a JSON-RPC response payload into a normalized probe outcome.""" + if ("result" in message) == ("error" in message): + return ProbeOutcome( + status="invalid_response", + message="JSON-RPC response must contain exactly one of result or error", + ) + if "result" in message: return ProbeOutcome(status="success") diff --git a/.github/workflows/tests/test_protocol_matrix.py b/.github/workflows/tests/test_protocol_matrix.py index b7a0c62b6..fbf832a0f 100644 --- a/.github/workflows/tests/test_protocol_matrix.py +++ b/.github/workflows/tests/test_protocol_matrix.py @@ -225,6 +225,12 @@ def test_response_exposes_models_only_when_present(): } ) assert not response_exposes_models({"result": {"sessionId": "sess-123"}}) + assert not response_exposes_models( + { + "result": {"models": {"currentModelId": "model-a"}}, + "error": {"code": -32602, "message": "Invalid params"}, + } + ) assert not response_exposes_models({"error": {"code": -32601, "message": "Method not found"}}) @@ -245,6 +251,17 @@ def test_classify_rpc_response_success(): assert outcome.status == "success" +def test_classify_rpc_response_rejects_result_and_error(): + outcome = classify_rpc_response( + { + "result": {"sessionId": "ambiguous-session"}, + "error": {"code": -32602, "message": "Invalid params"}, + } + ) + assert outcome.status == "invalid_response" + assert "exactly one" in (outcome.message or "") + + def test_request_with_timeout_reports_exited_process(): proc = subprocess.Popen( [sys.executable, "-c", "import sys; sys.exit(7)"], diff --git a/.github/workflows/tests/test_protocol_matrix_close.py b/.github/workflows/tests/test_protocol_matrix_close.py index e1d62c4a7..896a666ae 100644 --- a/.github/workflows/tests/test_protocol_matrix_close.py +++ b/.github/workflows/tests/test_protocol_matrix_close.py @@ -12,6 +12,7 @@ PROBE_SCHEMA_VERSION, ProbeOutcome, build_snapshot, + classify_rpc_response, close_capability_advertised, feature_cell, main, @@ -305,6 +306,65 @@ def request_with_timeout( "not_probed", ) + def test_probe_agent_does_not_close_after_ambiguous_session_new_response(self): + calls = [] + + def request_with_timeout(proc, request_id, method, params, timeout): + calls.append((request_id, method, params, timeout)) + if method == "initialize": + return ProbeOutcome(status="success"), { + "result": { + "protocolVersion": 1, + "agentCapabilities": {"sessionCapabilities": {"close": {}}}, + } + } + if method == "session/new": + message = { + "result": {"sessionId": "ambiguous-session"}, + "error": {"code": -32602, "message": "Invalid params"}, + } + return classify_rpc_response(message), message + return ProbeOutcome(status="success"), {"result": {}} + + fake_process = SimpleNamespace(returncode=0) + agent = { + "id": "agent-1", + "name": "Agent One", + "version": "1.2.3", + "distribution": {"npx": {"package": "agent-one"}}, + } + + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("protocol_matrix.ensure_distribution_runtime", return_value=None), + patch( + "protocol_matrix.build_agent_command", + return_value=(["fake-agent"], None, {}), + ), + patch("protocol_matrix.build_agent_process_env", return_value={}), + patch("protocol_matrix.subprocess.Popen", return_value=fake_process), + patch( + "protocol_matrix.request_with_timeout", + side_effect=request_with_timeout, + ), + patch("protocol_matrix.stop_process"), + patch("protocol_matrix.collect_stderr_tail", return_value=None), + ): + record = probe_agent( + agent=agent, + sandbox_base=Path(temp_dir), + init_timeout=5.0, + rpc_timeout=1.0, + ) + + methods = [method for _, method, _, _ in calls] + self.assertNotIn("session/close", methods) + self.assertEqual(record["sessionNew"]["status"], "invalid_response") + self.assertEqual( + record["methodProbes"]["session/close"]["status"], + "not_probed", + ) + def test_probe_agent_does_not_close_when_capability_is_omitted_or_null(self): agent = { "id": "agent-1",