fix(host): keep the tunnel receive loop responsive during readiness refresh (#4092)
* fix(host): keep the tunnel receive loop responsive during readiness refresh The host->server tunnel disconnected with `4003 ping timeout`: the periodic harness-readiness refresh ran inline on the receive loop and could block it for ~60s — two 30s CLI probe subprocesses (`--version` and `auth status`) hanging on a wedged harness CLI. While blocked, the host never answered the server's application-level pings, so the server watchdog declared the host dead and closed the tunnel. Fix A (host/connect.py): move the readiness refresh into its own task, `_harness_readiness_loop`, so the receive loop only ever reads frames and answers pings — a slow probe can no longer stall the keepalive. Fix B (harness_install.py, harness_readiness.py, codex_native.py): bound readiness CLI probes to READINESS_CLI_PROBE_TIMEOUT_S (10s, matching goose's status-probe budget) instead of 30s, so a hung harness CLI fails fast on the refresh, the startup hello, and Ctrl-C. Setup and launch gating keep the lenient 30s default via behavior-preserving timeout parameters. Co-authored-by: Isaac Signed-off-by: dbczumar <corey.zumar@databricks.com> * test: accept the readiness probe timeout kwarg in harness CLI stubs Readiness now calls harness_cli_installed / harness_cli_logged_in with a timeout= kwarg (READINESS_CLI_PROBE_TIMEOUT_S); update the monkeypatch stubs in the affected suites to accept it so they exercise the same paths. Co-authored-by: Isaac Signed-off-by: dbczumar <corey.zumar@databricks.com> * test(host): cover off-loop readiness refresh and bounded CLI probe Fix A moved the harness-readiness refresh off the tunnel receive loop into _harness_readiness_loop. Rewrite the three live-host readiness tests to drive that loop directly: the old versions drove _serve_frames with a fake tunnel that blocks on recv, which under the pure recv loop never exits and hangs to the pytest timeout. Add a harness_install test asserting the readiness caller shortens the CLI probe subprocess timeout while setup/launch keep the 30s default. Co-authored-by: Isaac Signed-off-by: dbczumar <corey.zumar@databricks.com> --------- Signed-off-by: dbczumar <corey.zumar@databricks.com>
This commit is contained in:
@@ -222,10 +222,12 @@ def _codex_auth_unavailable_reason() -> HarnessUnavailableReason | None:
|
||||
fail-open the ``claude-sdk`` / ``openai-agents`` gateway harnesses already
|
||||
rely on: their gateway token is a runtime mint the daemon can't observe.
|
||||
|
||||
The check stays synchronous, side-effect free, and local: it resolves the
|
||||
launch (local config reads) and, only on the defer-to-login path, inspects
|
||||
the local auth source. It never runs ``codex login``, a status command, or a
|
||||
network probe; any resolver failure fails safe onto the ``auth.json`` check.
|
||||
The check stays synchronous and local: it resolves the launch (local config
|
||||
reads) and, only on the defer-to-login path, inspects the local auth source.
|
||||
It never runs ``codex login`` or a status command; the CLI ``--version``
|
||||
probe it does run is bounded by ``READINESS_CLI_PROBE_TIMEOUT_S`` so a hung
|
||||
CLI can't stall the readiness refresh, and any resolver failure fails safe
|
||||
onto the ``auth.json`` check.
|
||||
|
||||
:returns: ``"binary-missing"`` when the CLI is absent, ``"needs-auth"``
|
||||
when the launch would defer to Codex's own login but ``auth.json`` is
|
||||
@@ -235,13 +237,14 @@ def _codex_auth_unavailable_reason() -> HarnessUnavailableReason | None:
|
||||
not judged locally — it surfaces at the first turn via the executor.
|
||||
"""
|
||||
from omnigent.onboarding.harness_install import (
|
||||
READINESS_CLI_PROBE_TIMEOUT_S,
|
||||
harness_cli_installed,
|
||||
)
|
||||
from omnigent.onboarding.provider_config import OPENAI_FAMILY
|
||||
|
||||
if _find_codex_cli() is None:
|
||||
return HARNESS_BINARY_MISSING
|
||||
if not harness_cli_installed(OPENAI_FAMILY):
|
||||
if not harness_cli_installed(OPENAI_FAMILY, timeout=READINESS_CLI_PROBE_TIMEOUT_S):
|
||||
return HARNESS_VERSION_TOO_LOW
|
||||
# On a host with no configured provider this may run ambient detection.
|
||||
# configured_harness_map shares one probe across all Codex aliases.
|
||||
|
||||
+61
-35
@@ -2625,7 +2625,9 @@ class HostProcess:
|
||||
|
||||
Sends the ``host.hello`` frame, prints the success banner, then
|
||||
loops dispatching launch/stop/stat/list_dir/worktree requests and
|
||||
answering runner pings until the connection closes.
|
||||
answering runner pings until the connection closes. Harness-readiness
|
||||
updates run in a separate task (:meth:`_harness_readiness_loop`) so a
|
||||
slow probe can never stall this receive loop.
|
||||
|
||||
:param ws: The open tunnel connection returned by the websockets
|
||||
client.
|
||||
@@ -2679,44 +2681,68 @@ class HostProcess:
|
||||
flush=True,
|
||||
)
|
||||
|
||||
# Readiness refresh runs in its own task, never on this receive loop:
|
||||
# a harness probe that blocks (a hung CLI ``--version`` / ``auth
|
||||
# status``) must not delay ``ws.recv()`` or the inline keepalive pong
|
||||
# the server's watchdog counts as liveness, or it closes the tunnel
|
||||
# with ``4003 ping timeout``.
|
||||
readiness_task = asyncio.create_task(
|
||||
self._harness_readiness_loop(ws, configured_harnesses)
|
||||
)
|
||||
try:
|
||||
while True:
|
||||
raw = await ws.recv()
|
||||
if isinstance(raw, str):
|
||||
await self._handle_raw_message(ws, raw)
|
||||
finally:
|
||||
readiness_task.cancel()
|
||||
with contextlib.suppress(asyncio.CancelledError, Exception):
|
||||
await readiness_task
|
||||
|
||||
async def _harness_readiness_loop(
|
||||
self,
|
||||
ws: websockets.asyncio.client.ClientConnection,
|
||||
initial: dict[str, HarnessAvailability],
|
||||
) -> None:
|
||||
"""
|
||||
Push harness-readiness updates on a timer, off the receive loop.
|
||||
|
||||
Runs as its own task so a slow readiness probe (a harness CLI whose
|
||||
``--version`` / ``auth status`` subprocess hangs) can never delay
|
||||
``ws.recv()`` or the inline keepalive pong — the cause of spurious
|
||||
``4003 ping timeout`` disconnects. Recomputes the map on the quick
|
||||
cadence gated by a cheap "did an unavailable harness just become ready"
|
||||
check and on the full cadence unconditionally, sending a
|
||||
:class:`HostHarnessReadinessFrame` only when the map changes.
|
||||
|
||||
:param ws: The open tunnel connection used to send update frames.
|
||||
:param initial: The readiness map already reported in ``host.hello``;
|
||||
the baseline the first update diffs against.
|
||||
:returns: None. Runs until cancelled when the connection ends.
|
||||
"""
|
||||
configured = initial
|
||||
loop = asyncio.get_running_loop()
|
||||
next_quick_refresh = loop.time() + HARNESS_READINESS_REFRESH_INTERVAL_S
|
||||
next_full_refresh = loop.time() + HARNESS_READINESS_FULL_REFRESH_INTERVAL_S
|
||||
next_quick = loop.time() + HARNESS_READINESS_REFRESH_INTERVAL_S
|
||||
next_full = loop.time() + HARNESS_READINESS_FULL_REFRESH_INTERVAL_S
|
||||
while True:
|
||||
raw: object | None = None
|
||||
with contextlib.suppress(asyncio.TimeoutError):
|
||||
raw = await asyncio.wait_for(
|
||||
ws.recv(),
|
||||
timeout=max(
|
||||
0.0,
|
||||
min(next_quick_refresh, next_full_refresh) - loop.time(),
|
||||
),
|
||||
)
|
||||
|
||||
await asyncio.sleep(max(0.0, min(next_quick, next_full) - loop.time()))
|
||||
now = loop.time()
|
||||
refresh_full_map = now >= next_full_refresh
|
||||
if now >= next_quick_refresh:
|
||||
next_quick_refresh = now + HARNESS_READINESS_REFRESH_INTERVAL_S
|
||||
if not refresh_full_map:
|
||||
refresh_full_map = await asyncio.to_thread(
|
||||
_unavailable_harness_became_ready,
|
||||
configured_harnesses,
|
||||
refresh_full = now >= next_full
|
||||
if now >= next_quick:
|
||||
next_quick = now + HARNESS_READINESS_REFRESH_INTERVAL_S
|
||||
if not refresh_full:
|
||||
refresh_full = await asyncio.to_thread(
|
||||
_unavailable_harness_became_ready, configured
|
||||
)
|
||||
|
||||
if refresh_full_map:
|
||||
latest_harnesses = await asyncio.to_thread(configured_harness_map)
|
||||
next_full_refresh = now + HARNESS_READINESS_FULL_REFRESH_INTERVAL_S
|
||||
if latest_harnesses != configured_harnesses:
|
||||
await ws.send(
|
||||
encode_host_frame(
|
||||
HostHarnessReadinessFrame(
|
||||
configured_harnesses=latest_harnesses,
|
||||
)
|
||||
)
|
||||
)
|
||||
configured_harnesses = latest_harnesses
|
||||
if isinstance(raw, str):
|
||||
await self._handle_raw_message(ws, raw)
|
||||
if not refresh_full:
|
||||
continue
|
||||
latest = await asyncio.to_thread(configured_harness_map)
|
||||
next_full = now + HARNESS_READINESS_FULL_REFRESH_INTERVAL_S
|
||||
if latest != configured:
|
||||
await ws.send(
|
||||
encode_host_frame(HostHarnessReadinessFrame(configured_harnesses=latest))
|
||||
)
|
||||
configured = latest
|
||||
|
||||
async def _handle_raw_message(
|
||||
self, ws: websockets.asyncio.client.ClientConnection, raw: str
|
||||
|
||||
@@ -713,17 +713,35 @@ def _parse_harness_cli_version(text: str) -> str | None:
|
||||
return _normalize_date_version(match.group(1))
|
||||
|
||||
|
||||
def _harness_cli_version_satisfies(spec: HarnessInstallSpec, binary: str) -> bool:
|
||||
# Wall-clock cap on a harness CLI probe subprocess (``--version`` /
|
||||
# ``auth status``). The default stays lenient for setup and launch gating; the
|
||||
# throttled readiness refresh passes ``READINESS_CLI_PROBE_TIMEOUT_S`` so a hung
|
||||
# CLI can't stall the refresh — and, through it, the host tunnel's keepalive.
|
||||
# The readiness cap matches goose's status-probe budget (``_INFO_TIMEOUT_S``):
|
||||
# enough for a healthy ``auth status`` keychain read / token refresh, short
|
||||
# enough that a wedged CLI fails fast.
|
||||
_DEFAULT_CLI_PROBE_TIMEOUT_S = 30.0
|
||||
READINESS_CLI_PROBE_TIMEOUT_S = 10.0
|
||||
|
||||
|
||||
def _harness_cli_version_satisfies(
|
||||
spec: HarnessInstallSpec,
|
||||
binary: str,
|
||||
timeout: float = _DEFAULT_CLI_PROBE_TIMEOUT_S,
|
||||
) -> bool:
|
||||
"""Check *binary*'s ``--version`` against *spec*'s declared range.
|
||||
|
||||
A missing/unparseable version or a subprocess error is treated as not
|
||||
satisfying the range, so an installed but incompatible CLI is reported
|
||||
as not ready and the setup flow prompts for an upgrade before the
|
||||
runtime gate rejects it.
|
||||
|
||||
:param timeout: Seconds to wait for the ``--version`` subprocess before
|
||||
giving up, e.g. ``10.0`` on the readiness path.
|
||||
"""
|
||||
if spec.min_version is None and spec.max_version_exclusive is None:
|
||||
return True
|
||||
version = _harness_cli_version_string(spec, binary)
|
||||
version = _harness_cli_version_string(spec, binary, timeout)
|
||||
if version is None:
|
||||
return False
|
||||
try:
|
||||
@@ -777,7 +795,7 @@ def harness_cli_version_satisfies(key: str) -> bool:
|
||||
return _harness_cli_version_satisfies(spec, binary)
|
||||
|
||||
|
||||
def harness_cli_installed(key: str) -> bool:
|
||||
def harness_cli_installed(key: str, timeout: float = _DEFAULT_CLI_PROBE_TIMEOUT_S) -> bool:
|
||||
"""Return whether the harness's CLI is present and meets its version range.
|
||||
|
||||
"Installed" now means the CLI binary (:func:`resolve_cli_binary`) is
|
||||
@@ -789,6 +807,9 @@ def harness_cli_installed(key: str) -> bool:
|
||||
|
||||
:param key: A harness family (``"anthropic"`` / ``"openai"``) or
|
||||
:data:`PI_KEY` / :data:`KIMI_KEY`.
|
||||
:param timeout: Seconds to wait for the ``--version`` probe subprocess,
|
||||
e.g. ``10.0`` on the readiness path where a hung CLI must not stall
|
||||
the refresh.
|
||||
:returns: ``True`` when the CLI resolves and is version-compatible;
|
||||
``False`` when it doesn't resolve, the key has no associated CLI,
|
||||
or its version falls outside the declared range.
|
||||
@@ -799,7 +820,7 @@ def harness_cli_installed(key: str) -> bool:
|
||||
binary = resolve_cli_binary(spec.binary)
|
||||
if binary is None:
|
||||
return False
|
||||
return _harness_cli_version_satisfies(spec, binary)
|
||||
return _harness_cli_version_satisfies(spec, binary, timeout)
|
||||
|
||||
|
||||
def harness_cli_version(key: str) -> tuple[str | None, str | None]:
|
||||
@@ -839,14 +860,22 @@ def _version_range_str(spec: HarnessInstallSpec) -> str | None:
|
||||
return f">={spec.min_version}, <{spec.max_version_exclusive}"
|
||||
|
||||
|
||||
def _harness_cli_version_string(spec: HarnessInstallSpec, binary: str) -> str | None:
|
||||
"""Return the parsed, normalized version string from *binary* ``--version``."""
|
||||
def _harness_cli_version_string(
|
||||
spec: HarnessInstallSpec,
|
||||
binary: str,
|
||||
timeout: float = _DEFAULT_CLI_PROBE_TIMEOUT_S,
|
||||
) -> str | None:
|
||||
"""Return the parsed, normalized version string from *binary* ``--version``.
|
||||
|
||||
:param timeout: Seconds to wait for the ``--version`` subprocess, e.g.
|
||||
``6.0`` on the readiness path.
|
||||
"""
|
||||
try:
|
||||
completed = subprocess.run(
|
||||
[binary, "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
timeout=timeout,
|
||||
check=False,
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
@@ -964,7 +993,7 @@ def install_harness_cli(key: str) -> bool:
|
||||
return try_install_harness_cli(key).installed
|
||||
|
||||
|
||||
def harness_cli_logged_in(key: str) -> bool:
|
||||
def harness_cli_logged_in(key: str, timeout: float = _DEFAULT_CLI_PROBE_TIMEOUT_S) -> bool:
|
||||
"""Return whether the harness CLI itself reports a usable login.
|
||||
|
||||
Asks the CLI's own status command (``claude auth status`` /
|
||||
@@ -984,6 +1013,8 @@ def harness_cli_logged_in(key: str) -> bool:
|
||||
|
||||
:param key: A harness family, e.g. ``"anthropic"`` (Claude),
|
||||
``"openai"`` (Codex), or ``"gemini"`` (Antigravity, via ``agy models``).
|
||||
:param timeout: Seconds to wait for the status subprocess, e.g. ``10.0`` on
|
||||
the readiness path where a hung CLI must not stall the refresh.
|
||||
:returns: ``True`` when the CLI reports a usable login; ``False`` when the
|
||||
key has no status command, the CLI binary is missing, the status
|
||||
process failed to spawn, or the CLI reports no login.
|
||||
@@ -999,7 +1030,7 @@ def harness_cli_logged_in(key: str) -> bool:
|
||||
result = subprocess.run(
|
||||
[argv_binary, *spec.status_args],
|
||||
check=False,
|
||||
timeout=30,
|
||||
timeout=timeout,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
|
||||
@@ -49,6 +49,7 @@ from omnigent.onboarding.harness_install import (
|
||||
OPENCODE_KEY,
|
||||
PI_KEY,
|
||||
QWEN_KEY,
|
||||
READINESS_CLI_PROBE_TIMEOUT_S,
|
||||
harness_cli_installed,
|
||||
harness_install_spec,
|
||||
required_cli_for_harness,
|
||||
@@ -373,7 +374,7 @@ def _binary_availability_reason(install_key: str) -> HarnessAvailability:
|
||||
exposed to the web UI as ``"version-too-low"`` so the user sees a prompt
|
||||
to upgrade rather than "binary-missing".
|
||||
"""
|
||||
if harness_cli_installed(install_key):
|
||||
if harness_cli_installed(install_key, timeout=READINESS_CLI_PROBE_TIMEOUT_S):
|
||||
return True
|
||||
spec = harness_install_spec(install_key)
|
||||
if spec is not None and resolve_cli_binary(spec.binary) is not None:
|
||||
@@ -406,7 +407,11 @@ def _cli_family_availability(canonical: str, install_key: str) -> HarnessAvailab
|
||||
|
||||
if _family_provider_configured(canonical):
|
||||
return True
|
||||
return True if harness_cli_logged_in(install_key) else "needs-auth"
|
||||
return (
|
||||
True
|
||||
if harness_cli_logged_in(install_key, timeout=READINESS_CLI_PROBE_TIMEOUT_S)
|
||||
else "needs-auth"
|
||||
)
|
||||
|
||||
|
||||
def _harness_availability(canonical: str) -> HarnessAvailability:
|
||||
|
||||
+70
-37
@@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import contextlib
|
||||
import logging
|
||||
import subprocess
|
||||
import threading
|
||||
@@ -926,27 +927,44 @@ class _FakeTunnel:
|
||||
raise ConnectionError("test disconnect")
|
||||
|
||||
|
||||
class _ReadinessChangingTunnel(_FakeTunnel):
|
||||
"""Trigger one idle refresh before disconnecting the fake tunnel."""
|
||||
class _RecordingWS:
|
||||
"""Fake tunnel that records frames the readiness loop sends.
|
||||
|
||||
The readiness loop (:meth:`HostProcess._harness_readiness_loop`) only ever
|
||||
calls ``send``; ``first_send`` lets a test await the first update frame
|
||||
deterministically instead of sleeping for a fixed interval.
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
"""Initialize the frame log and receive counter."""
|
||||
super().__init__()
|
||||
self.recv_count = 0
|
||||
"""Initialize the frame log and first-send signal."""
|
||||
self.sent: list[str] = []
|
||||
self.first_send = asyncio.Event()
|
||||
|
||||
async def recv(self) -> str:
|
||||
"""Simulate one idle interval followed by a disconnect."""
|
||||
self.recv_count += 1
|
||||
if self.recv_count == 1:
|
||||
await asyncio.Future()
|
||||
raise ConnectionError("test disconnect")
|
||||
async def send(self, data: str) -> None:
|
||||
"""Record an outbound frame and signal the first send.
|
||||
|
||||
:param data: Encoded frame text.
|
||||
"""
|
||||
self.sent.append(data)
|
||||
self.first_send.set()
|
||||
|
||||
|
||||
async def _cancel(task: asyncio.Task[None]) -> None:
|
||||
"""Cancel *task* and await its unwinding, swallowing the cancellation."""
|
||||
task.cancel()
|
||||
with contextlib.suppress(asyncio.CancelledError):
|
||||
await task
|
||||
|
||||
|
||||
async def test_live_host_refreshes_harness_readiness_without_reconnect(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A setup completed after connect must replace the advertised readiness."""
|
||||
readiness = iter(({"pi": False}, {"pi": True}))
|
||||
"""A setup completed after connect pushes a replacement readiness frame.
|
||||
|
||||
The refresh now runs in :meth:`HostProcess._harness_readiness_loop`, off the
|
||||
receive loop, so a slow probe can never stall the tunnel keepalive.
|
||||
"""
|
||||
readiness = iter(({"pi": True},))
|
||||
monkeypatch.setattr(
|
||||
"omnigent.host.connect.configured_harness_map",
|
||||
lambda: next(readiness),
|
||||
@@ -960,25 +978,26 @@ async def test_live_host_refreshes_harness_readiness_without_reconnect(
|
||||
0.01,
|
||||
)
|
||||
host = _make_host_process()
|
||||
tunnel = _ReadinessChangingTunnel()
|
||||
ws = _RecordingWS()
|
||||
|
||||
with pytest.raises(ConnectionError, match="test disconnect"):
|
||||
await host._serve_frames(tunnel) # type: ignore[arg-type] — duck-typed ws
|
||||
task = asyncio.create_task(host._harness_readiness_loop(ws, {"pi": False}))
|
||||
try:
|
||||
await asyncio.wait_for(ws.first_send.wait(), timeout=2.0)
|
||||
finally:
|
||||
await _cancel(task)
|
||||
|
||||
assert len(tunnel.sent) == 2
|
||||
hello = decode_host_frame(tunnel.sent[0])
|
||||
refresh = decode_host_frame(tunnel.sent[1])
|
||||
assert isinstance(hello, HostHelloFrame)
|
||||
assert hello.configured_harnesses == {"pi": False}
|
||||
assert len(ws.sent) == 1
|
||||
refresh = decode_host_frame(ws.sent[0])
|
||||
assert isinstance(refresh, HostHarnessReadinessFrame)
|
||||
assert refresh.configured_harnesses == {"pi": True}
|
||||
_cleanup_host(host)
|
||||
|
||||
|
||||
async def test_live_host_full_refresh_detects_auth_completion(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The full-refresh fallback catches readiness changes beyond binary installs."""
|
||||
readiness = iter(({"codex": "needs-auth"}, {"codex": True}))
|
||||
readiness = iter(({"codex": True},))
|
||||
monkeypatch.setattr(
|
||||
"omnigent.host.connect.configured_harness_map",
|
||||
lambda: next(readiness),
|
||||
@@ -988,38 +1007,52 @@ async def test_live_host_full_refresh_detects_auth_completion(
|
||||
0.01,
|
||||
)
|
||||
host = _make_host_process()
|
||||
tunnel = _ReadinessChangingTunnel()
|
||||
ws = _RecordingWS()
|
||||
|
||||
with pytest.raises(ConnectionError, match="test disconnect"):
|
||||
await host._serve_frames(tunnel) # type: ignore[arg-type] — duck-typed ws
|
||||
task = asyncio.create_task(host._harness_readiness_loop(ws, {"codex": "needs-auth"}))
|
||||
try:
|
||||
await asyncio.wait_for(ws.first_send.wait(), timeout=2.0)
|
||||
finally:
|
||||
await _cancel(task)
|
||||
|
||||
assert len(tunnel.sent) == 2
|
||||
refresh = decode_host_frame(tunnel.sent[1])
|
||||
assert len(ws.sent) == 1
|
||||
refresh = decode_host_frame(ws.sent[0])
|
||||
assert isinstance(refresh, HostHarnessReadinessFrame)
|
||||
assert refresh.configured_harnesses == {"codex": True}
|
||||
_cleanup_host(host)
|
||||
|
||||
|
||||
async def test_live_host_does_not_repeat_unchanged_readiness(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A periodic full refresh sends nothing when the readiness map is unchanged."""
|
||||
readiness = iter(({"codex": "needs-auth"}, {"codex": "needs-auth"}))
|
||||
monkeypatch.setattr(
|
||||
"omnigent.host.connect.configured_harness_map",
|
||||
lambda: next(readiness),
|
||||
)
|
||||
calls = {"n": 0}
|
||||
|
||||
def _unchanged_map() -> dict[str, str]:
|
||||
calls["n"] += 1
|
||||
return {"codex": "needs-auth"}
|
||||
|
||||
monkeypatch.setattr("omnigent.host.connect.configured_harness_map", _unchanged_map)
|
||||
monkeypatch.setattr(
|
||||
"omnigent.host.connect.HARNESS_READINESS_FULL_REFRESH_INTERVAL_S",
|
||||
0.01,
|
||||
)
|
||||
host = _make_host_process()
|
||||
tunnel = _ReadinessChangingTunnel()
|
||||
ws = _RecordingWS()
|
||||
|
||||
with pytest.raises(ConnectionError, match="test disconnect"):
|
||||
await host._serve_frames(tunnel) # type: ignore[arg-type] — duck-typed ws
|
||||
task = asyncio.create_task(host._harness_readiness_loop(ws, {"codex": "needs-auth"}))
|
||||
try:
|
||||
# Let at least two full refreshes recompute-and-compare before stopping.
|
||||
for _ in range(400):
|
||||
if calls["n"] >= 2:
|
||||
break
|
||||
await asyncio.sleep(0.005)
|
||||
finally:
|
||||
await _cancel(task)
|
||||
|
||||
assert len(tunnel.sent) == 1
|
||||
assert isinstance(decode_host_frame(tunnel.sent[0]), HostHelloFrame)
|
||||
assert calls["n"] >= 2
|
||||
assert ws.sent == []
|
||||
_cleanup_host(host)
|
||||
|
||||
|
||||
async def test_handle_launch_immediate_exit_reports_exit_code_and_log_tail(
|
||||
|
||||
@@ -113,6 +113,37 @@ def test_kimi_only_upstream_binary_satisfies_readiness(
|
||||
assert hi.harness_cli_installed(hi.KIMI_KEY) is True
|
||||
|
||||
|
||||
def test_cli_probe_timeout_defaults_lenient_but_readiness_passes_short(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The readiness caller can shorten the probe subprocess timeout.
|
||||
|
||||
A wedged harness CLI must not stall the throttled readiness refresh for the
|
||||
lenient default (30s); readiness passes ``READINESS_CLI_PROBE_TIMEOUT_S`` so
|
||||
the ``auth status`` probe fails fast. Direct callers (setup / launch) keep
|
||||
the 30s default.
|
||||
"""
|
||||
monkeypatch.setattr(hi.shutil, "which", lambda name: f"/usr/local/bin/{name}")
|
||||
recorded: list[float | None] = []
|
||||
|
||||
def _record_run(argv: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
recorded.append(kwargs.get("timeout")) # type: ignore[arg-type]
|
||||
return subprocess.CompletedProcess(
|
||||
args=argv, returncode=0, stdout='{"loggedIn": true}', stderr=""
|
||||
)
|
||||
|
||||
monkeypatch.setattr(hi.subprocess, "run", _record_run)
|
||||
|
||||
assert hi.harness_cli_logged_in(ANTHROPIC_FAMILY) is True
|
||||
assert recorded[-1] == 30.0
|
||||
|
||||
assert (
|
||||
hi.harness_cli_logged_in(ANTHROPIC_FAMILY, timeout=hi.READINESS_CLI_PROBE_TIMEOUT_S)
|
||||
is True
|
||||
)
|
||||
assert recorded[-1] == hi.READINESS_CLI_PROBE_TIMEOUT_S == 10.0
|
||||
|
||||
|
||||
def test_cursor_install_spec_is_login_only_no_npm() -> None:
|
||||
"""Cursor ships via a curl installer (no npm package) and authenticates
|
||||
through its own CLI login, so it carries an ``install_hint`` + status JSON
|
||||
|
||||
@@ -72,7 +72,7 @@ def _all_clis_installed(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Auth-aware native harnesses (now including Cursor native) check login state
|
||||
# in the picker map. Treat them as logged in when the test just needs
|
||||
# "binary present".
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda _key: True)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda _key, **_kw: True)
|
||||
|
||||
|
||||
def _no_clis_installed(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
@@ -188,7 +188,7 @@ def test_auth_aware_native_harness_needs_auth_when_installed_not_signed_in(
|
||||
monkeypatch.setattr(
|
||||
"omnigent.onboarding.harness_readiness._family_provider_configured", lambda _h: False
|
||||
)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda key: False)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda key, **_kw: False)
|
||||
# opencode: no stored/env provider.
|
||||
import omnigent.onboarding.opencode_auth as oc
|
||||
|
||||
@@ -217,7 +217,7 @@ def test_claude_ready_via_configured_provider_without_cli_login(
|
||||
"omnigent.onboarding.harness_readiness._family_provider_configured", lambda _h: True
|
||||
)
|
||||
|
||||
def _must_not_probe(_key: str) -> bool:
|
||||
def _must_not_probe(_key: str, **_kw: object) -> bool:
|
||||
raise AssertionError("CLI login probed despite a configured provider")
|
||||
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", _must_not_probe)
|
||||
@@ -270,7 +270,7 @@ def test_auth_aware_native_harness_launch_gate_stays_binary_only(
|
||||
So with the binary present it stays ``True`` even when not signed in.
|
||||
"""
|
||||
_all_clis_installed(monkeypatch)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda key: False)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda key, **_kw: False)
|
||||
assert harness_is_configured("claude-native") is True
|
||||
assert harness_is_configured("opencode-native") is True
|
||||
|
||||
@@ -584,8 +584,8 @@ def test_configured_harness_map_reports_version_too_low_for_outdated_clis(
|
||||
of being told the binary is missing.
|
||||
"""
|
||||
monkeypatch.setattr(hi.shutil, "which", lambda name: f"/usr/bin/{name}")
|
||||
monkeypatch.setattr(hi, "harness_cli_installed", lambda _key: False)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda _key: True)
|
||||
monkeypatch.setattr(hi, "harness_cli_installed", lambda _key, **_kw: False)
|
||||
monkeypatch.setattr(hi, "harness_cli_logged_in", lambda _key, **_kw: True)
|
||||
result = configured_harness_map()
|
||||
for harness in (
|
||||
"claude-native",
|
||||
|
||||
@@ -81,7 +81,7 @@ def _point_codex_auth_check_at(
|
||||
# on the auth-path decision.
|
||||
monkeypatch.setattr(
|
||||
"omnigent.onboarding.harness_install.harness_cli_installed",
|
||||
lambda _key: True,
|
||||
lambda _key, **_kw: True,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -17,10 +17,10 @@ def test_pi_harnesses_gate_on_pi_cli(harness: str, monkeypatch: pytest.MonkeyPat
|
||||
host pre-spawn check then let a doomed launch through. Both spellings must
|
||||
track ``harness_cli_installed``.
|
||||
"""
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: False)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: False)
|
||||
assert hr.harness_is_configured(harness) is False
|
||||
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: True)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: True)
|
||||
assert hr.harness_is_configured(harness) is True
|
||||
|
||||
|
||||
@@ -31,7 +31,7 @@ def test_kiro_native_harnesses_gate_on_kiro_cli(
|
||||
"""Native Kiro is gated on the ``kiro-cli`` binary being installed."""
|
||||
calls: list[str] = []
|
||||
|
||||
def _installed(key: str) -> bool:
|
||||
def _installed(key: str, **_kw: object) -> bool:
|
||||
calls.append(key)
|
||||
return False
|
||||
|
||||
@@ -39,7 +39,7 @@ def test_kiro_native_harnesses_gate_on_kiro_cli(
|
||||
assert hr.harness_is_configured(harness) is False
|
||||
assert calls[-1] == hr.KIRO_KEY
|
||||
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: True)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: True)
|
||||
assert hr.harness_is_configured(harness) is True
|
||||
|
||||
|
||||
@@ -50,7 +50,7 @@ def test_sdk_and_unknown_harnesses_still_fail_open(monkeypatch: pytest.MonkeyPat
|
||||
(runtime/ambient credentials) and unknown harnesses must keep failing open
|
||||
so a working launch is never blocked.
|
||||
"""
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: False)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: False)
|
||||
assert hr.harness_is_configured("claude-sdk") is True
|
||||
assert hr.harness_is_configured("openai-agents") is True
|
||||
assert hr.harness_is_configured("totally-unknown-harness") is True
|
||||
@@ -64,7 +64,7 @@ def test_configured_harness_map_exposes_pi_native(monkeypatch: pytest.MonkeyPatc
|
||||
A missing binary now reports the richer ``"binary-missing"`` reason (Pi
|
||||
gained the credential axis) rather than a bare ``False``.
|
||||
"""
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: False)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: False)
|
||||
cmap = hr.configured_harness_map()
|
||||
assert cmap.get("pi-native") == "binary-missing"
|
||||
assert cmap.get("pi") == "binary-missing"
|
||||
@@ -79,7 +79,7 @@ def test_configured_harness_map_pi_installed_no_provider_needs_auth(
|
||||
so an installed binary with no provider is the yellow "installed but not
|
||||
configured" state the setup dialog offers an "Add key" action for.
|
||||
"""
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: True)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: True)
|
||||
monkeypatch.setattr(hr, "_family_provider_configured", lambda _h: False)
|
||||
cmap = hr.configured_harness_map()
|
||||
assert cmap.get("pi") == "needs-auth"
|
||||
@@ -90,7 +90,7 @@ def test_configured_harness_map_pi_installed_with_provider_ready(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Pi installed AND a provider configured reports ready (``True``)."""
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: True)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: True)
|
||||
monkeypatch.setattr(hr, "_family_provider_configured", lambda _h: True)
|
||||
cmap = hr.configured_harness_map()
|
||||
assert cmap.get("pi") is True
|
||||
@@ -99,7 +99,7 @@ def test_configured_harness_map_pi_installed_with_provider_ready(
|
||||
|
||||
def test_configured_harness_map_exposes_kiro_native(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The readiness map carries Kiro native keys for the web picker lookup."""
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key: False)
|
||||
monkeypatch.setattr(hr, "harness_cli_installed", lambda _key, **_kw: False)
|
||||
cmap = hr.configured_harness_map()
|
||||
assert cmap.get("kiro-native") is False
|
||||
assert cmap.get("native-kiro") is False
|
||||
|
||||
Reference in New Issue
Block a user