fix: allow omitting mcpServers in session requests

Author: PsiACE

scripts/gen_schema.py

@@ -2,16 +2,23 @@
 from __future__ import annotations
 
 import ast
+import contextlib
 import json
 import re
 import subprocess
 import sys
+import tempfile
 import textwrap
 from collections.abc import Callable
 from dataclasses import dataclass
 from pathlib import Path
 
 ROOT = Path(__file__).resolve().parents[1]
+if str(ROOT) not in sys.path:
+    sys.path.append(str(ROOT))
+
+from scripts.schema_patches import apply_schema_patches  # noqa: E402
+
 SCHEMA_DIR = ROOT / "schema"
 SCHEMA_JSON = SCHEMA_DIR / "schema.json"
 VERSION_FILE = SCHEMA_DIR / "VERSION"
@@ -136,12 +143,23 @@ def generate_schema() -> None:
         )
         sys.exit(1)
 
+    schema_payload = json.loads(SCHEMA_JSON.read_text(encoding="utf-8"))
+    schema_payload, patch_warnings = apply_schema_patches(schema_payload)
+    for warning in patch_warnings:
+        print(f"Warning: {warning.message}", file=sys.stderr)
+
+    patched_schema_path: Path | None = None
+    with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False, encoding="utf-8") as handle:
+        json.dump(schema_payload, handle, indent=2)
+        handle.write("\n")
+        patched_schema_path = Path(handle.name)
+
     cmd = [
         sys.executable,
         "-m",
         "datamodel_code_generator",
         "--input",
-        str(SCHEMA_JSON),
+        str(patched_schema_path),
         "--input-file-type",
         "jsonschema",
         "--output",
@@ -155,10 +173,15 @@ def generate_schema() -> None:
         "--snake-case-field",
     ]
 
-    subprocess.check_call(cmd)  # noqa: S603
-    warnings = postprocess_generated_schema(SCHEMA_OUT)
-    for warning in warnings:
-        print(f"Warning: {warning}", file=sys.stderr)
+    try:
+        subprocess.check_call(cmd)  # noqa: S603
+        warnings = postprocess_generated_schema(SCHEMA_OUT)
+        for warning in warnings:
+            print(f"Warning: {warning}", file=sys.stderr)
+    finally:
+        if patched_schema_path is not None:
+            with contextlib.suppress(OSError):
+                patched_schema_path.unlink()
 
 
 def postprocess_generated_schema(output_path: Path) -> list[str]:

scripts/schema_patches.py

@@ -0,0 +1,51 @@
+from __future__ import annotations
+
+from dataclasses import dataclass
+from typing import Any
+
+
+@dataclass(frozen=True, slots=True)
+class PatchWarning:
+    message: str
+
+
+def apply_schema_patches(schema: dict[str, Any]) -> tuple[dict[str, Any], list[PatchWarning]]:
+    patched = schema
+    warnings: list[PatchWarning] = []
+
+    patched, warning = _make_defs_field_optional(patched, "NewSessionRequest", "mcpServers")
+    if warning is not None:
+        warnings.append(warning)
+
+    patched, warning = _make_defs_field_optional(patched, "LoadSessionRequest", "mcpServers")
+    if warning is not None:
+        warnings.append(warning)
+
+    return patched, warnings
+
+
+def _make_defs_field_optional(
+    schema: dict[str, Any],
+    model_name: str,
+    field_name: str,
+) -> tuple[dict[str, Any], PatchWarning | None]:
+    defs = schema.get("$defs")
+    if not isinstance(defs, dict):
+        return schema, PatchWarning("schema.$defs missing or invalid; cannot apply patches")
+
+    model = defs.get(model_name)
+    if not isinstance(model, dict):
+        return schema, PatchWarning(f"schema.$defs.{model_name} missing or invalid; cannot patch {field_name}")
+
+    required = model.get("required")
+    if required is None:
+        return schema, None
+    if not isinstance(required, list):
+        return schema, PatchWarning(f"schema.$defs.{model_name}.required invalid; cannot patch {field_name}")
+
+    new_required = [item for item in required if item != field_name]
+    if new_required == required:
+        return schema, None
+
+    model["required"] = new_required
+    return schema, None

src/acp/client/connection.py

@@ -45,6 +45,7 @@
 
 __all__ = ["ClientSideConnection"]
 _CLIENT_CONNECTION_ERROR = "ClientSideConnection requires asyncio StreamWriter/StreamReader"
+_MISSING = object()
 
 
 @final
@@ -93,7 +94,10 @@ async def initialize(
 
     @param_model(NewSessionRequest)
     async def new_session(
-        self, cwd: str, mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio], **kwargs: Any
+        self,
+        cwd: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
     ) -> NewSessionResponse:
         return await request_model(
             self._conn,
@@ -104,12 +108,27 @@ async def new_session(
 
     @param_model(LoadSessionRequest)
     async def load_session(
-        self, cwd: str, mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio], session_id: str, **kwargs: Any
+        self,
+        cwd: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | str | None = None,
+        session_id: str | object = _MISSING,
+        **kwargs: Any,
     ) -> LoadSessionResponse:
+        if session_id is _MISSING:
+            if isinstance(mcp_servers, str):
+                session_id = mcp_servers
+                mcp_servers = None
+            else:
+                raise TypeError("load_session() missing required argument: 'session_id'")
         return await request_model_from_dict(
             self._conn,
             AGENT_METHODS["session_load"],
-            LoadSessionRequest(cwd=cwd, mcp_servers=mcp_servers, session_id=session_id, field_meta=kwargs or None),
+            LoadSessionRequest(
+                cwd=cwd,
+                mcp_servers=cast(list[HttpMcpServer | SseMcpServer | McpServerStdio] | None, mcp_servers),
+                session_id=cast(str, session_id),
+                field_meta=kwargs or None,
+            ),
             LoadSessionResponse,
         )
 

src/acp/interfaces.py

@@ -154,12 +154,19 @@ async def initialize(
 
     @param_model(NewSessionRequest)
     async def new_session(
-        self, cwd: str, mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio], **kwargs: Any
+        self,
+        cwd: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
     ) -> NewSessionResponse: ...
 
     @param_model(LoadSessionRequest)
     async def load_session(
-        self, cwd: str, mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio], session_id: str, **kwargs: Any
+        self,
+        cwd: str,
+        session_id: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
     ) -> LoadSessionResponse | None: ...
 
     @param_model(ListSessionsRequest)

src/acp/schema.py

@@ -1410,12 +1410,12 @@ class NewSessionRequest(BaseModel):
     ]
     # List of MCP (Model Context Protocol) servers the agent should connect to.
     mcp_servers: Annotated[
-        List[Union[HttpMcpServer, SseMcpServer, McpServerStdio]],
+        Optional[List[Union[HttpMcpServer, SseMcpServer, McpServerStdio]]],
         Field(
             alias="mcpServers",
             description="List of MCP (Model Context Protocol) servers the agent should connect to.",
         ),
-    ]
+    ] = None
 
 
 class PermissionOption(BaseModel):
@@ -2073,12 +2073,12 @@ class LoadSessionRequest(BaseModel):
     cwd: Annotated[str, Field(description="The working directory for this session.")]
     # List of MCP servers to connect to for this session.
     mcp_servers: Annotated[
-        List[Union[HttpMcpServer, SseMcpServer, McpServerStdio]],
+        Optional[List[Union[HttpMcpServer, SseMcpServer, McpServerStdio]]],
         Field(
             alias="mcpServers",
             description="List of MCP servers to connect to for this session.",
         ),
-    ]
+    ] = None
     # The ID of the session to load.
     session_id: Annotated[str, Field(alias="sessionId", description="The ID of the session to load.")]
 

tests/conftest.py

@@ -243,12 +243,19 @@ async def initialize(
         return InitializeResponse(protocol_version=protocol_version, agent_capabilities=None, auth_methods=[])
 
     async def new_session(
-        self, cwd: str, mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio], **kwargs: Any
+        self,
+        cwd: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
     ) -> NewSessionResponse:
         return NewSessionResponse(session_id="test-session-123")
 
     async def load_session(
-        self, cwd: str, mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio], session_id: str, **kwargs: Any
+        self,
+        cwd: str,
+        session_id: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
     ) -> LoadSessionResponse | None:
         return LoadSessionResponse()
 

tests/real_user/test_issue_55_mcp_servers_optional.py

@@ -0,0 +1,68 @@
+import asyncio
+from typing import Any
+
+import pytest
+
+from acp import InitializeResponse, LoadSessionResponse, NewSessionResponse
+from acp.core import AgentSideConnection, ClientSideConnection
+from acp.schema import HttpMcpServer, McpServerStdio, SseMcpServer
+from tests.conftest import TestAgent, TestClient
+
+# Regression from a real-world client run where `mcpServers` is omitted from session requests.
+
+
+class Issue55Agent(TestAgent):
+    def __init__(self) -> None:
+        super().__init__()
+        self.seen_new_session: tuple[str, Any] | None = None
+        self.seen_load_session: tuple[str, str, Any] | None = None
+
+    async def new_session(
+        self,
+        cwd: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
+    ) -> NewSessionResponse:
+        self.seen_new_session = (cwd, mcp_servers)
+        return await super().new_session(cwd=cwd, mcp_servers=mcp_servers, **kwargs)
+
+    async def load_session(
+        self,
+        cwd: str,
+        session_id: str,
+        mcp_servers: list[HttpMcpServer | SseMcpServer | McpServerStdio] | None = None,
+        **kwargs: Any,
+    ) -> LoadSessionResponse | None:
+        self.seen_load_session = (cwd, session_id, mcp_servers)
+        return await super().load_session(cwd=cwd, session_id=session_id, mcp_servers=mcp_servers, **kwargs)
+
+
+@pytest.mark.asyncio
+async def test_session_requests_allow_missing_mcp_servers(server) -> None:
+    client = TestClient()
+    captured_agent: list[Issue55Agent] = []
+
+    agent_conn = ClientSideConnection(client, server._client_writer, server._client_reader)  # type: ignore[arg-type]
+    _agent_side = AgentSideConnection(
+        lambda _conn: captured_agent.append(Issue55Agent()) or captured_agent[-1],
+        server._server_writer,
+        server._server_reader,
+        listening=True,
+    )
+
+    init = await asyncio.wait_for(agent_conn.initialize(protocol_version=1), timeout=1.0)
+    assert isinstance(init, InitializeResponse)
+
+    new_session = await asyncio.wait_for(agent_conn.new_session(cwd="/workspace"), timeout=1.0)
+    assert isinstance(new_session, NewSessionResponse)
+
+    load_session = await asyncio.wait_for(
+        agent_conn.load_session(cwd="/workspace", session_id=new_session.session_id),
+        timeout=1.0,
+    )
+    assert isinstance(load_session, LoadSessionResponse)
+
+    assert captured_agent, "Agent was not constructed"
+    [agent] = captured_agent
+    assert agent.seen_new_session == ("/workspace", None)
+    assert agent.seen_load_session == ("/workspace", new_session.session_id, None)