fix(acp): make session/close and auth errors spec-compliant

CHANGELOG.md

@@ -15,6 +15,11 @@ GitHub Releases page; `0.8.0` is the new starting line.
 
 ## Unreleased
 
+- **ACP session close and auth errors are spec-compliant.** `session/close` now cancels any
+  in-flight work and frees the session's runtime resources (MCP toolset, background refresh)
+  before dropping it, instead of leaking them, per the ACP `session/close` requirement. The
+  `authenticate` failure path now serializes its `authMethods` to plain dicts so the JSON-RPC
+  error response no longer raises `TypeError` on encode.
 - **Auto-mode destructive actions deliberate per turn and context.** Auto-deliberation now
   scopes destructive-command one-shots to the active execution context and LLM generation,
   so duplicate destructive calls in one response keep bouncing while later deliberate retries

docs/en/release-notes/changelog.md

@@ -17,6 +17,11 @@ GitHub Releases page; `0.8.0` is the new starting line.
 
 ## Unreleased
 
+- **ACP session close and auth errors are spec-compliant.** `session/close` now cancels any
+  in-flight work and frees the session's runtime resources (MCP toolset, background refresh)
+  before dropping it, instead of leaking them, per the ACP `session/close` requirement. The
+  `authenticate` failure path now serializes its `authMethods` to plain dicts so the JSON-RPC
+  error response no longer raises `TypeError` on encode.
 - **Auto-mode destructive actions deliberate per turn and context.** Auto-deliberation now
   scopes destructive-command one-shots to the active execution context and LLM generation,
   so duplicate destructive calls in one response keep bouncing while later deliberate retries

src/pythinker_code/acp/server.py

@@ -376,9 +376,18 @@ async def set_session_mode(
     async def close_session(
         self, session_id: str, **kwargs: Any
     ) -> acp.schema.CloseSessionResponse | None:
-        """Drop a session from the in-memory registry (ACP 0.10 session/close)."""
+        """Close a session: cancel in-flight work, then free its resources (ACP 0.10).
+
+        The ``session/close`` spec requires the agent to cancel any ongoing work
+        (as if ``session/cancel`` was called) and then release the resources
+        associated with the session before dropping it from the registry.
+        """
         logger.info("Closing session: {id}", id=session_id)
-        self.sessions.pop(session_id, None)
+        entry = self.sessions.pop(session_id, None)
+        if entry is not None:
+            acp_session, _ = entry
+            await acp_session.cancel()
+            await acp_session.cli.cleanup_runtime_resources()
         return None
 
     async def set_config_option(
@@ -469,7 +478,10 @@ async def authenticate(self, method_id: str, **kwargs: Any) -> acp.AuthenticateR
                 raise acp.RequestError.auth_required(
                     {
                         "message": "Please complete login in terminal first",
-                        "authMethods": self._auth_methods,
+                        "authMethods": [
+                            m.model_dump(by_alias=True, exclude_none=True)
+                            for m in self._auth_methods
+                        ],
                     }
                 )
 

tests/acp/test_server_initialize.py

@@ -2,6 +2,9 @@
 
 from __future__ import annotations
 
+from types import SimpleNamespace
+from unittest.mock import AsyncMock
+
 import acp.schema
 import pytest
 
@@ -34,14 +37,30 @@ async def test_initialize_advertises_terminal_auth_method():
     assert auth_method.env == {}
 
 
-async def test_close_session_drops_session_from_registry():
-    """ACP 0.10 session/close removes the session from the in-memory registry."""
+async def test_close_session_cancels_and_releases_resources():
+    """ACP 0.10 session/close must cancel in-flight work and free per-session
+    resources before dropping the session.
+
+    The spec mandates the agent "must cancel any ongoing work related to the
+    session (treat it as if ``session/cancel`` was called) and then free up any
+    resources associated with the session."
+    """
     server = ACPServer()
-    server.sessions["sess-1"] = ("placeholder",)  # type: ignore[assignment]
+    cli = SimpleNamespace(cleanup_runtime_resources=AsyncMock())
+    acp_session = SimpleNamespace(cancel=AsyncMock(), cli=cli)
+    # Registry stores ``tuple[ACPSession, _ModelIDConv]``.
+    server.sessions["sess-1"] = (acp_session, object())  # type: ignore[assignment]
 
     result = await server.close_session("sess-1")
 
     assert result is None
     assert "sess-1" not in server.sessions
-    # Closing an unknown session is a no-op (must not raise).
+    acp_session.cancel.assert_awaited_once()
+    cli.cleanup_runtime_resources.assert_awaited_once()
+
+
+async def test_close_session_unknown_is_noop():
+    """Closing an unknown session must be a no-op (no raise, no cleanup)."""
+    server = ACPServer()
+
     assert await server.close_session("missing") is None

tests/ui_and_conv/test_acp_server_auth.py

@@ -169,6 +169,33 @@ async def test_authenticate_rejects_expired_token_without_refresh(server: ACPSer
         assert exc_info.value.code == -32000
 
 
+@pytest.mark.asyncio
+async def test_authenticate_auth_required_data_is_json_serializable(server: ACPServer) -> None:
+    """The AUTH_REQUIRED error from authenticate() must carry JSON-serializable
+    ``authMethods`` (plain dicts), not raw Pydantic models.
+
+    The JSON-RPC connection layer encodes the error ``data`` with a plain
+    ``json.dumps`` (no Pydantic-aware default encoder), so raw models would raise
+    ``TypeError`` while building the error response — crashing the agent process.
+    """
+    import json
+
+    token = _make_token(expires_at=time.time() - 100, refresh_token="")
+
+    with (
+        patch("pythinker_code.acp.server.load_config", return_value=Config()),
+        patch("pythinker_code.acp.server.load_tokens", return_value=token),
+        pytest.raises(acp.RequestError) as exc_info,
+    ):
+        await server.authenticate(method_id="login")
+
+    data = exc_info.value.data
+    assert isinstance(data, dict)
+    # Must not raise TypeError on encode (the bug: raw Pydantic models).
+    json.dumps(data)
+    assert all(isinstance(m, dict) for m in data["authMethods"])
+
+
 @pytest.mark.asyncio
 async def test_authenticate_accepts_valid_token(server: ACPServer) -> None:
     """authenticate('login') should succeed for a valid, non-expired token."""