docs: v4 plan for OpenCodeClient auth + close() bugfixes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-23 17:13:41 +08:00
parent 5306ada0c4
commit ff932d9095
1 changed files with 330 additions and 0 deletions
@@ -0,0 +1,330 @@
+# v4 — 修 OpenCodeClient fetch kwargs bug + SIGINT 悬挂
+
+**Goal:** 修两个服务端 bug:
+1. `OpenCodeClient._request` 在传 `HTTPRequest` 时又传 `**fetch_kwargs`(auth_username/auth_password)→ tornado 报 `kwargs can't be used if request is an HTTPRequest object`。
+2. `jupyter lab` Ctrl+C 后 "received signal 2, stopping" 但进程不退出——`AsyncHTTPClient()` 单例持有持久连接,IOLoop 关不掉。
+
+**Architecture:**
+- `opencode_client.py`:`_request` 把 auth 挂到 `HTTPRequest` 构造参数,`fetch(request)` 不再带 kwargs;`OpenCodeClient` 加 `close()` 关闭它持有的 `AsyncHTTPClient`。
+- `opencode_bridge/__init__.py`:模块加载时 `atexit.register(_close_opencode_http)`,在进程正常退出(含 jupyter 捕到 SIGINT 后有序退出)时关掉 tornado `AsyncHTTPClient` 单例,释放连接,IOLoop 能完成关闭。
+- 测试:`test_client.py` 补 `close()` 行为 + 验证 fetch 用的 request 上带了 auth(而不是 fetch kwargs)。
+
+**Tech Stack:** Python 3.12 / tornado AsyncHTTPClient / pytest-jupyter
+
+**Spec:** 即时修复,无独立 spec doc(本 plan 即规格)。
+
+## Global Constraints
+
+- 仅改 `opencode_bridge/opencode_client.py`、`opencode_bridge/__init__.py`、`opencode_bridge/tests/test_client.py`(如需);`test_routes.py` 不动(除非路由测试也依赖 client 行为)。
+- `OpenCodeClient.close()` 幂等:重复调用安全;若 `_http_client` 已关闭则跳过。
+- `atexit` handler 捕获所有异常(关 client 不应阻断退出),用 `try/except Exception: pass`。
+- 不新增依赖;不破坏现有 API 签名(`health` / `list_providers` / `create_session` / `send_message_sync` / `abort` / `delete_session` / `_request` 保持)。
+- 现有 `FakeOpenCodeClient` 测试桩不动(它模拟 client,不直接用 `AsyncHTTPClient`)。
+- 不动前端;不动 schema/CSS/design.md;这是纯服务端修复。
+
+---
+
+### Task 1: 修 `opencode_client.py` — auth 挂到 request + `close()`
+
+**Files:** `opencode_bridge/opencode_client.py`
+
+完整新内容(用 `HTTPRequest` 的 `auth_username`/`auth_password` 参数,`fetch(request)` 不带 kwargs;加 `close()`):
+
+```python
+"""Async HTTP client for the local OpenCode server."""
+
+import atexit
+import json
+from typing import Any, Optional
+
+from tornado.httpclient import AsyncHTTPClient, HTTPRequest
+from tornado.httputil import HTTPHeaders
+
+from .config import OpenCodeConfig
+
+
+class OpenCodeError(Exception):
+    """Raised when an OpenCode API request fails."""
+
+
+# Ensure the tornado AsyncHTTPClient singleton is closed at interpreter
+# shutdown so the IOLoop can finish and the process can exit cleanly
+# (otherwise Ctrl+C on `jupyter lab` hangs after "received signal 2,
+# stopping" because the singleton keeps persistent connections alive).
+def _close_opencode_http() -> None:
+    try:
+        AsyncHTTPClient().close()
+    except Exception:
+        # Never block interpreter shutdown on close errors.
+        pass
+
+
+atexit.register(_close_opencode_http)
+
+
+class OpenCodeClient:
+    def __init__(
+        self,
+        config: OpenCodeConfig,
+        http_client: Optional[AsyncHTTPClient] = None,
+    ) -> None:
+        self._config = config
+        # Use a dedicated client (not the singleton) so close() can
+        # deterministically release its connections without affecting
+        # other components. The module-level atexit still closes the
+        # singleton as a safety net.
+        self._http_client = http_client or AsyncHTTPClient(force_instance=True)
+        self._owns_http_client = http_client is None
+
+    def close(self) -> None:
+        """Release the underlying HTTP client's connections. Idempotent."""
+        if getattr(self, "_http_client", None) is None:
+            return
+        try:
+            if self._owns_http_client:
+                self._http_client.close()
+        except Exception:
+            pass
+        self._http_client = None  # type: ignore[assignment]
+        self._owns_http_client = False
+
+    async def health(self) -> dict[str, Any]:
+        return await self._request("GET", "/global/health")
+
+    async def list_providers(self) -> list[dict[str, Any]]:
+        return await self._request("GET", "/config/providers")
+
+    async def create_session(self, title: str) -> dict[str, Any]:
+        return await self._request("POST", "/session", {"title": title})
+
+    async def send_message_sync(
+        self,
+        session_id: str,
+        parts: list[dict[str, Any]],
+        provider_id: Optional[str] = None,
+        model_id: Optional[str] = None,
+        system: Optional[str] = None,
+    ) -> dict[str, Any]:
+        body: dict[str, Any] = {"parts": parts}
+        if provider_id is not None and model_id is not None:
+            body["model"] = {"providerID": provider_id, "modelID": model_id}
+        if system is not None:
+            body["system"] = system
+        return await self._request(
+            "POST",
+            "/session/%s/message" % session_id,
+            body,
+        )
+
+    async def abort(self, session_id: str) -> bool:
+        result = await self._request("POST", "/session/%s/abort" % session_id)
+        return result is not None
+
+    async def delete_session(self, session_id: str) -> bool:
+        result = await self._request("DELETE", "/session/%s" % session_id)
+        return result is not None
+
+    @property
+    def endpoint(self) -> str:
+        return self._config.url
+
+    async def _request(
+        self,
+        method: str,
+        path: str,
+        body: Optional[dict[str, Any]] = None,
+    ) -> Optional[Any]:
+        url = self._config.url + path
+        headers = HTTPHeaders(
+            {
+                "Content-Type": "application/json",
+                "Accept": "application/json",
+            }
+        )
+        request_kwargs: dict[str, Any] = {
+            "method": method,
+            "headers": headers,
+            "request_timeout": self._config.request_timeout_seconds,
+        }
+        if body is not None:
+            request_kwargs["body"] = json.dumps(body).encode("utf-8")
+        # HTTP Basic Auth must be set on the HTTPRequest itself; passing
+        # auth_* as **kwargs to fetch() when the first arg is already an
+        # HTTPRequest is rejected by tornado ("kwargs can't be used if
+        # request is an HTTPRequest object").
+        if self._config.auth is not None:
+            request_kwargs["auth_username"] = self._config.auth[0]
+            request_kwargs["auth_password"] = self._config.auth[1]
+
+        request = HTTPRequest(url, **request_kwargs)
+
+        response = await self._http_client.fetch(request)
+
+        if 200 <= response.code < 300:
+            if not response.body:
+                return True
+            return json.loads(response.body.decode("utf-8"))
+        if response.code == 404:
+            return None
+
+        response_body = response.body.decode("utf-8") if response.body else ""
+        raise OpenCodeError(
+            "OpenCode request %s %s failed with status %s: %s"
+            % (method, url, response.code, response_body)
+        )
+```
+
+- [ ] **Step 1: 应用上面完整新内容到 `opencode_bridge/opencode_client.py`**
+
+- [ ] **Step 2: 跑现有 pytest 确认没破**
+
+Run: `.venv/bin/python -m pytest opencode_bridge/tests/ -q`
+Expected: 32 passed(注意:`test_client.py` 用 `FakeOpenCodeClient`,不直接调 `AsyncHTTPClient`,所以现有测试不应受影响)。
+
+- [ ] **Step 3: Commit**
+
+```bash
+git add opencode_bridge/opencode_client.py
+git commit -m "fix: set Basic Auth on HTTPRequest, not as fetch kwargs; close client cleanly
+
+tornado rejects fetch(request, **kwargs) when the first arg is already an
+HTTPRequest and the kwargs overlap request construction (auth_username /
+auth_password). Move auth onto the HTTPRequest. Add OpenCodeClient.close()
+to release its AsyncHTTPClient; module-level atexit closes the tornado
+singleton so 'jupyter lab' can exit cleanly after SIGINT instead of
+hanging on 'received signal 2, stopping'."
+```
+
+---
+
+### Task 2: 补 `test_client.py` 测试
+
+**Files:** `opencode_bridge/tests/test_client.py`(补 close + auth-on-request 行为测试)
+
+**Interfaces:**
+- Consumes: 现有 `FakeOpenCodeClient`(保留,它不直接用 `AsyncHTTPClient`)。
+- Produces: 新测试验证 `OpenCodeClient.close()` 幂等 + 调用后 `_http_client` 为 None;验证带 auth 的 `_request` 构造的 `HTTPRequest` 上有 `auth_username`/`auth_password`(而不是传给 `fetch`)。
+
+- [ ] **Step 1: Read** `opencode_bridge/tests/test_client.py` 看现有结构,只追加测试,**不改** `FakeOpenCodeClient`。
+
+- [ ] **Step 2: 追加测试**(放在文件末尾):
+
+```python
+from unittest.mock import AsyncMock, MagicMock
+import pytest
+from tornado.httpclient import HTTPRequest
+
+from opencode_bridge.config import OpenCodeConfig
+from opencode_bridge.opencode_client import OpenCodeClient
+
+
+def _cfg(password: str = "") -> OpenCodeConfig:
+    return OpenCodeConfig(
+        url="http://fake-opencode",
+        user="opencode",
+        password=password,
+        request_timeout_seconds=10,
+    )
+
+
+@pytest.mark.asyncio
+async def test_request_sets_auth_on_request_not_fetch_kwargs(monkeypatch):
+    """Bug fix: tornado rejects fetch(request, **kwargs) for auth_*; auth
+    must live on the HTTPRequest itself."""
+    captured: dict = {}
+
+    class FakeClient:
+        async def fetch(self, request, **kwargs):
+            captured["request"] = request
+            captured["fetch_kwargs"] = kwargs
+            r = MagicMock()
+            r.code = 200
+            r.body = b'{"id": "s1"}'
+            return r
+
+    cfg = _cfg(password="secret")
+    client = OpenCodeClient(cfg, http_client=FakeClient())  # type: ignore[arg-type]
+    await client._request("GET", "/x")
+
+    # No kwargs passed to fetch.
+    assert captured["fetch_kwargs"] == {}
+    # Auth is on the HTTPRequest.
+    assert isinstance(captured["request"], HTTPRequest)
+    assert captured["request"].auth_username == "opencode"
+    assert captured["request"].auth_password == "secret"
+
+
+@pytest.mark.asyncio
+async def test_request_omits_auth_when_no_password(monkeypatch):
+    """When password is empty, no auth_* is set on the request."""
+    captured: dict = {}
+
+    class FakeClient:
+        async def fetch(self, request, **kwargs):
+            captured["request"] = request
+            captured["fetch_kwargs"] = kwargs
+            r = MagicMock()
+            r.code = 200
+            r.body = b'{"id": "s1"}'
+            return r
+
+    client = OpenCodeClient(_cfg(password=""), http_client=FakeClient())  # type: ignore[arg-type]
+    await client._request("GET", "/x")
+
+    assert captured["fetch_kwargs"] == {}
+    assert captured["request"].auth_username is None
+    assert captured["request"].auth_password is None
+
+
+def test_close_is_idempotent():
+    """close() can be called multiple times safely."""
+    client = OpenCodeClient(_cfg())
+    client.close()
+    client.close()  # must not raise
+    assert client._http_client is None
+
+
+def test_close_closes_owned_client():
+    """close() calls .close() on the AsyncHTTPClient we own."""
+    fake = MagicMock()
+    client = OpenCodeClient(_cfg(), http_client=fake)
+    # We didn't create it, so close() should NOT call .close() on it
+    # (we don't own it). But _http_client is reset.
+    client.close()
+    fake.close.assert_not_called()
+    assert client._http_client is None
+```
+
+- [ ] **Step 3: 跑 pytest**
+
+Run: `.venv/bin/python -m pytest opencode_bridge/tests/ -q`
+Expected: 32 + 4 = 36 passed(若现有测试数变化,以实际输出为准,全部 pass)。
+
+- [ ] **Step 4: Commit**
+
+```bash
+git add opencode_bridge/tests/test_client.py
+git commit -m "test: cover auth-on-request and close() in OpenCodeClient"
+```
+
+---
+
+### Task 3: 全量验证
+
+```bash
+.venv/bin/python -m pytest opencode_bridge/tests/ -v
+PATH="$PWD/.venv/bin:$PATH" jlpm test
+```
+Expected: pytest 全过;jest 全过(前端未动,仍是 25 passed)。
+
+---
+
+### 手动验收
+
+```bash
+export OPENCODE_BRIDGE_URL=http://127.0.0.1:4096
+export OPENCODE_BRIDGE_PASSWORD=yourpassword   # 若 OpenCode Serve 启了 auth
+jupyter lab
+# 在 notebook 里点 🪄 → 选 model → 输入指令 → 发送:不再报 "kwargs can't be used..."
+# Ctrl+C: 进程应快速干净退出(不再 hang "received signal 2, stopping")
+```