[harness] - close four unhandled-error paths and add first tests for main()'s bind guard

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:
@@ -65,12 +76,17 @@ 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 = {}
     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=_token_count(usage.get("prompt_tokens")),
+        completion_tokens=_token_count(usage.get("completion_tokens")),
     )
 
 

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

tests/test_harness_robustness.py

@@ -0,0 +1,161 @@
+"""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", "expected_prompt", "expected_completion"),
+    [
+        ("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, 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",
+            "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 == expected_prompt
+    assert result.completion_tokens == expected_completion