From 3aad9d5348da8cbc5889939266ad01d930962140 Mon Sep 17 00:00:00 2001 From: CyClaw Agent Date: Wed, 5 Aug 2026 11:55:04 +0000 Subject: [PATCH 1/2] fix(harness): close four unhandled-error paths and test main()'s bind guard - /api/harness/runs: guard the glob->mtime sort against a file deleted mid-listing (same race SessionStore.list() already tolerates) instead of 500ing - ollama: a malformed usage block from an OpenAI-compatible proxy now degrades token tallies to 0 instead of raising an unparseable 500 - main(): a non-numeric CYCLAW_HARNESS_PORT now fails closed like the adjacent range check instead of silently binding the stored port - create_app(): missing static/harness.html raises the typed HarnessConfigError instead of a bare FileNotFoundError - tests: first coverage for main()'s loopback/port guards, plus the race, missing-asset, and malformed-usage regressions Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015VHRCPLFdeWvtp21SKo5QQ --- harness/ollama.py | 16 +++- harness/server.py | 32 +++++-- tests/test_harness_robustness.py | 160 +++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 tests/test_harness_robustness.py diff --git a/harness/ollama.py b/harness/ollama.py index 4cb6fa34..7e81e7b5 100644 --- a/harness/ollama.py +++ b/harness/ollama.py @@ -65,12 +65,22 @@ def _parse_chat_response(resp: httpx.Response, fallback_model: str) -> ChatResul body_text = body.get("content") if not isinstance(body_text, str): raise HarnessLLMError("malformed response from model server") - usage = parsed.get("usage") or {} + # usage is cosmetic (console token tally) — a proxy sending a non-dict + # usage block or non-numeric counts must degrade the tally to 0, not turn + # a good answer into the unparseable 500 the guards above exist to prevent + usage = parsed.get("usage") + if not isinstance(usage, dict): + usage = {} + try: + prompt_tokens = int(usage.get("prompt_tokens") or 0) + completion_tokens = int(usage.get("completion_tokens") or 0) + except (TypeError, ValueError): + prompt_tokens = completion_tokens = 0 return ChatResult( body_text=body_text, model=str(parsed.get("model", fallback_model)), - prompt_tokens=int(usage.get("prompt_tokens", 0) or 0), - completion_tokens=int(usage.get("completion_tokens", 0) or 0), + prompt_tokens=prompt_tokens, + completion_tokens=completion_tokens, ) diff --git a/harness/server.py b/harness/server.py index 2facf7f9..6c846ee1 100644 --- a/harness/server.py +++ b/harness/server.py @@ -39,7 +39,7 @@ import subprocess # nosec B404 - imported only for TimeoutExpired; no process is spawned here import sys from collections.abc import AsyncIterator, Callable, Mapping -from contextlib import asynccontextmanager +from contextlib import asynccontextmanager, suppress from functools import lru_cache from pathlib import Path from urllib.parse import urlparse @@ -50,7 +50,7 @@ from starlette.middleware.trustedhost import TrustedHostMiddleware from harness.agent_policy import RUN_ID_RE, CheckProfileError, available_profiles, resolve_check_profiles -from harness.config import _MAX_PORT, _MIN_USER_PORT, HarnessConfig +from harness.config import _MAX_PORT, _MIN_USER_PORT, HarnessConfig, HarnessConfigError from harness.ollama import HarnessChatClient, HarnessLLMError from harness.prompts import compose_system_prompt from harness.registry_view import full_registry @@ -437,9 +437,17 @@ def _current_model() -> str: # token differs per instance) rather than per request. app.state also # carries the raw token for tests that need to attach it without a page # fetch -- see tests/test_harness_auth.py. - console_html = (_STATIC / "harness.html").read_text(encoding="utf-8").replace( - _CSRF_PLACEHOLDER, csrf_token - ) + # A trimmed checkout (e.g. a sparse or partial clone) can lack the static + # console asset; surface that as the typed config error the entry points + # already report cleanly, not a bare FileNotFoundError traceback. + try: + console_source = (_STATIC / "harness.html").read_text(encoding="utf-8") + except FileNotFoundError as exc: + raise HarnessConfigError( + "static/harness.html is missing — incomplete checkout?", + details={"path": str(_STATIC / "harness.html")}, + ) from exc + console_html = console_source.replace(_CSRF_PLACEHOLDER, csrf_token) app.state.csrf_token = csrf_token @app.get("/", response_class=HTMLResponse) @@ -842,7 +850,12 @@ def harness_runs() -> dict: json_files = [ entry for entry in accepted.glob("*.json") if entry.is_file() ] - json_files.sort(key=os.path.getmtime, reverse=True) + # Tolerate a file deleted between the glob and the sort: getmtime + # would otherwise raise OSError straight out of this route as a + # bare 500. Same race guard SessionStore.list() carries for the + # byte-identical glob -> mtime-sort pattern in harness/sessions.py. + with suppress(OSError): + json_files.sort(key=os.path.getmtime, reverse=True) for path in json_files[:_MAX_RUNS]: runs.append({"run_id": path.stem, "path": str(path)}) return {"runs": runs, "count": len(runs)} @@ -894,7 +907,12 @@ def main() -> None: if host not in _LOOPBACK_HOSTS: sys.exit("harness binds loopback only (threat model: single-operator)") port_env = os.environ.get("CYCLAW_HARNESS_PORT", "").strip() - port = int(port_env) if port_env.isdigit() else cfg.port + if port_env and not port_env.isdigit(): + # fail closed like the range check below: a typo'd override used to be + # silently discarded, so the harness bound the stored port instead of + # the one the operator asked for + sys.exit(f"CYCLAW_HARNESS_PORT is not a port number: {port_env}") + port = int(port_env) if port_env else cfg.port if not _MIN_USER_PORT <= port <= _MAX_PORT: # same bounds config.py applies to the stored port; without this the env # override accepts 0 (ephemeral bind — console link breaks) or >65535 diff --git a/tests/test_harness_robustness.py b/tests/test_harness_robustness.py new file mode 100644 index 00000000..0f3109bd --- /dev/null +++ b/tests/test_harness_robustness.py @@ -0,0 +1,160 @@ +"""Robustness tests for the harness entry point and console error paths. + +Covers the previously-untested ``harness.server.main()`` bind/port guards, the +``/api/harness/runs`` glob->mtime-sort race, the missing ``static/harness.html`` +startup path, and malformed ``usage`` blocks from an OpenAI-compatible proxy. +No live services: chat goes over httpx MockTransport, routes over TestClient. +""" + +from __future__ import annotations + +import os + +import httpx +import pytest +import uvicorn +from fastapi.testclient import TestClient + +from harness import server as harness_server +from harness.config import HarnessConfig, HarnessConfigError +from harness.ollama import HarnessChatClient +from harness.server import create_app, main + +_TEST_KEY = "harness-test-key" + + +@pytest.fixture(autouse=True) +def _api_key(monkeypatch): + monkeypatch.setenv("CYCLAW_API_KEY", _TEST_KEY) + + +@pytest.fixture() +def cfg(tmp_path, monkeypatch): + monkeypatch.setenv("CYCLAW_HOME", str(tmp_path / ".CyClaw")) + return HarnessConfig.load() + + +def _chat_client(payload: dict) -> HarnessChatClient: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=payload) + + return HarnessChatClient( + base_url="http://127.0.0.1:11434/v1", + model="qwen3.6:27b", + transport=httpx.MockTransport(handler), + ) + + +# -- /api/harness/runs race ------------------------------------------------------ + + +def test_runs_listing_survives_file_removed_mid_sort(cfg, tmp_path, monkeypatch): + """Regression: a run artifact deleted between glob and mtime-sort raised + OSError straight out of the route as a bare 500. Same race SessionStore + guards in harness/sessions.py.""" + accepted = tmp_path / "runs" / "accepted" + accepted.mkdir(parents=True) + keep = accepted / "aaaaaaaaaaaaaaaa.json" + keep.write_text("{}", encoding="utf-8") + doomed = accepted / "bbbbbbbbbbbbbbbb.json" + doomed.write_text("{}", encoding="utf-8") + monkeypatch.setattr(harness_server, "_RUNS_DIR", tmp_path / "runs") + real_getmtime = os.path.getmtime + + def _getmtime_racing(path): + if os.path.basename(str(path)) == doomed.name: + raise OSError("file vanished between glob and sort") + return real_getmtime(path) + + monkeypatch.setattr(os.path, "getmtime", _getmtime_racing) + app = create_app(cfg, _chat_client({})) + client = TestClient(app, base_url="http://127.0.0.1") + resp = client.get("/api/harness/runs") + assert resp.status_code == 200 + assert "aaaaaaaaaaaaaaaa" in [run["run_id"] for run in resp.json()["runs"]] + + +# -- missing console asset ------------------------------------------------------- + + +def test_create_app_missing_console_asset_raises_typed_error(cfg, tmp_path, monkeypatch): + """A trimmed checkout without static/harness.html must fail with the typed + HarnessConfigError, not a bare FileNotFoundError traceback.""" + monkeypatch.setattr(harness_server, "_STATIC", tmp_path / "no-static") + with pytest.raises(HarnessConfigError) as excinfo: + create_app(cfg, _chat_client({})) + assert "harness.html" in str(excinfo.value) + + +# -- main() bind/port guards ----------------------------------------------------- + + +@pytest.fixture() +def _uvicorn_recorder(cfg, monkeypatch): + calls: list[dict] = [] + + def _record(app, host, port): + calls.append({"host": host, "port": port}) + + monkeypatch.setattr(uvicorn, "run", _record) + monkeypatch.delenv("CYCLAW_HARNESS_HOST", raising=False) + monkeypatch.delenv("CYCLAW_HARNESS_PORT", raising=False) + return calls + + +def test_main_refuses_non_loopback_host(_uvicorn_recorder, monkeypatch): + monkeypatch.setenv("CYCLAW_HARNESS_HOST", "0.0.0.0") # noqa: S104 - asserting the refusal + with pytest.raises(SystemExit, match="loopback"): + main() + assert _uvicorn_recorder == [] + + +@pytest.mark.parametrize("bad_port", ["abc", "-1", "8790x", "70000", "0"]) +def test_main_refuses_bad_port_env(_uvicorn_recorder, monkeypatch, bad_port): + """Regression: a non-numeric CYCLAW_HARNESS_PORT was silently discarded, so + the harness bound the stored port instead of failing closed like the + adjacent out-of-range check.""" + monkeypatch.setenv("CYCLAW_HARNESS_PORT", bad_port) + with pytest.raises(SystemExit, match="CYCLAW_HARNESS_PORT"): + main() + assert _uvicorn_recorder == [] + + +def test_main_uses_valid_port_env(_uvicorn_recorder, monkeypatch): + monkeypatch.setenv("CYCLAW_HARNESS_PORT", "8791") + main() + assert _uvicorn_recorder == [{"host": "127.0.0.1", "port": 8791}] + + +def test_main_defaults_to_stored_port(cfg, _uvicorn_recorder, monkeypatch): + monkeypatch.delenv("CYCLAW_HARNESS_PORT", raising=False) + main() + assert _uvicorn_recorder == [{"host": "127.0.0.1", "port": cfg.port}] + + +# -- malformed usage from an OpenAI-compatible proxy ----------------------------- + + +@pytest.mark.parametrize( + "usage", + [ + "none", # non-dict usage: .get() used to AttributeError -> bare 500 + {"prompt_tokens": "abc", "completion_tokens": 5}, # int() used to ValueError + {"prompt_tokens": None}, + None, + ], +) +def test_chat_survives_malformed_usage_block(usage): + """Token tallies are cosmetic: a proxy sending a malformed usage block must + degrade the counts to 0, never turn a good answer into a 500.""" + chat = _chat_client( + { + "model": "qwen3.6:27b", + "choices": [{"message": {"role": "assistant", "content": "hello"}}], + "usage": usage, + } + ) + result = chat.chat(system_prompt="s", messages=[{"role": "user", "content": "hi"}]) + assert result.body_text == "hello" + assert result.prompt_tokens == 0 + assert result.completion_tokens == 0 From 0c9cf8be36dfcfd05045e8278a29f1ea92e6ce0c Mon Sep 17 00:00:00 2001 From: CyClaw Agent Date: Wed, 5 Aug 2026 11:59:12 +0000 Subject: [PATCH 2/2] fix(harness): satisfy WPS caps by moving usage-count parsing into a helper WPS229/WPS210/WPS231/WPS429 flagged the inline try/except in _parse_chat_response. _token_count degrades each malformed usage field to 0 independently, so a valid sibling count now survives (test updated to pin the per-field behavior). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015VHRCPLFdeWvtp21SKo5QQ --- harness/ollama.py | 20 +++++++++++++------- tests/test_harness_robustness.py | 21 +++++++++++---------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/harness/ollama.py b/harness/ollama.py index 7e81e7b5..c6a3d43e 100644 --- a/harness/ollama.py +++ b/harness/ollama.py @@ -48,6 +48,17 @@ def _is_loopback(url: str) -> bool: return (urlparse(url).hostname or "") in _LOOPBACK_HOSTS +def _token_count(raw_count: object) -> int: + """Best-effort int for one usage counter; malformed values degrade to 0.""" + # A helper rather than inline in _parse_chat_response: WPS229 caps try + # bodies at one statement and WPS210/WPS231 cap that function's locals + # and complexity, so the degrade-to-0 guard has to live here. + try: + return int(raw_count or 0) # type: ignore[call-overload] + except (TypeError, ValueError): + return 0 + + def _parse_chat_response(resp: httpx.Response, fallback_model: str) -> ChatResult: """Extract body text + token usage, or raise a typed error.""" try: @@ -71,16 +82,11 @@ def _parse_chat_response(resp: httpx.Response, fallback_model: str) -> ChatResul usage = parsed.get("usage") if not isinstance(usage, dict): usage = {} - try: - prompt_tokens = int(usage.get("prompt_tokens") or 0) - completion_tokens = int(usage.get("completion_tokens") or 0) - except (TypeError, ValueError): - prompt_tokens = completion_tokens = 0 return ChatResult( body_text=body_text, model=str(parsed.get("model", fallback_model)), - prompt_tokens=prompt_tokens, - completion_tokens=completion_tokens, + prompt_tokens=_token_count(usage.get("prompt_tokens")), + completion_tokens=_token_count(usage.get("completion_tokens")), ) diff --git a/tests/test_harness_robustness.py b/tests/test_harness_robustness.py index 0f3109bd..59a37ce3 100644 --- a/tests/test_harness_robustness.py +++ b/tests/test_harness_robustness.py @@ -136,17 +136,18 @@ def test_main_defaults_to_stored_port(cfg, _uvicorn_recorder, monkeypatch): @pytest.mark.parametrize( - "usage", + ("usage", "expected_prompt", "expected_completion"), [ - "none", # non-dict usage: .get() used to AttributeError -> bare 500 - {"prompt_tokens": "abc", "completion_tokens": 5}, # int() used to ValueError - {"prompt_tokens": None}, - None, + ("none", 0, 0), # non-dict usage: .get() used to AttributeError -> bare 500 + ({"prompt_tokens": "abc", "completion_tokens": 5}, 0, 5), # int() used to ValueError + ({"prompt_tokens": None}, 0, 0), + (None, 0, 0), ], ) -def test_chat_survives_malformed_usage_block(usage): - """Token tallies are cosmetic: a proxy sending a malformed usage block must - degrade the counts to 0, never turn a good answer into a 500.""" +def test_chat_survives_malformed_usage_block(usage, expected_prompt, expected_completion): + """Token tallies are cosmetic: a malformed field in a proxy's usage block + must degrade that count to 0, never turn a good answer into a 500 — and a + still-valid sibling field keeps its real value.""" chat = _chat_client( { "model": "qwen3.6:27b", @@ -156,5 +157,5 @@ def test_chat_survives_malformed_usage_block(usage): ) result = chat.chat(system_prompt="s", messages=[{"role": "user", "content": "hi"}]) assert result.body_text == "hello" - assert result.prompt_tokens == 0 - assert result.completion_tokens == 0 + assert result.prompt_tokens == expected_prompt + assert result.completion_tokens == expected_completion