diff --git a/docs/superpowers/plans/2026-07-23-opencode-client-bugfixes.md b/docs/superpowers/plans/2026-07-23-opencode-client-bugfixes.md new file mode 100644 index 0000000..d018d84 --- /dev/null +++ b/docs/superpowers/plans/2026-07-23-opencode-client-bugfixes.md @@ -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") +```