mirror of
https://github.com/usestrix/strix.git
synced 2026-09-08 22:21:05 +00:00
fix: bound Caido readiness probe by wall-clock deadline, not fixed attempts
_login_as_guest() retried loginAsGuest exactly 10 times with capped exponential backoff (~68s total), regardless of how long the sandbox actually took to boot Caido. The sandbox entrypoint chowns a large toolchain tree before starting caido-cli, which can take minutes on a loaded host or CI runner, so every scan there failed before a single LLM call was made. Replace the fixed attempt count with a wall-clock deadline, configurable via STRIX_CAIDO_BOOT_WAIT_S (default 180s). Fast hosts are unaffected; slow/shared hosts and CI runners can raise the budget instead of hitting a hardcoded ceiling. The error message now reports elapsed time and attempt count instead of just attempts. Fixes #1036, #1037
This commit is contained in:
parent
7cc9fa9faa
commit
706040acda
4 changed files with 119 additions and 8 deletions
|
|
@ -112,6 +112,12 @@ class RuntimeSettings(BaseSettings):
|
|||
backend: str = Field(default="docker", alias="STRIX_RUNTIME_BACKEND")
|
||||
# Max screenshot/image tool outputs kept live per agent context (0 = none).
|
||||
max_context_images: int = Field(default=3, ge=0, alias="STRIX_MAX_CONTEXT_IMAGES")
|
||||
# Wall-clock budget for the Caido readiness probe (session_manager ->
|
||||
# bootstrap_caido -> _login_as_guest). The sandbox entrypoint chowns a
|
||||
# large toolchain tree before starting caido-cli, which can take minutes
|
||||
# on a loaded host or CI runner; a fixed attempt count gives up too early
|
||||
# there. Raise this on slow/shared hosts instead of patching the retry loop.
|
||||
caido_boot_wait_s: float = Field(default=180.0, gt=0, alias="STRIX_CAIDO_BOOT_WAIT_S")
|
||||
|
||||
|
||||
class TelemetrySettings(BaseSettings):
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import asyncio
|
|||
import contextlib
|
||||
import json
|
||||
import logging
|
||||
import time
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from caido_sdk_client import Client, TokenAuthOptions
|
||||
|
|
@ -35,16 +36,23 @@ async def _login_as_guest(
|
|||
session: BaseSandboxSession,
|
||||
*,
|
||||
container_url: str,
|
||||
attempts: int = 10,
|
||||
max_wait_s: float = 180.0,
|
||||
) -> str:
|
||||
"""``session.exec`` curl to fetch a guest token; retry until ready.
|
||||
|
||||
Caido's GraphQL listener may not be up the instant the container
|
||||
starts. The retry loop also doubles as the Caido readiness probe —
|
||||
no separate TCP healthcheck needed.
|
||||
starts — the sandbox entrypoint chowns a large toolchain tree first,
|
||||
which can take minutes on a loaded host or CI runner. The retry loop
|
||||
also doubles as the Caido readiness probe — no separate TCP
|
||||
healthcheck needed — so it is bounded by wall-clock time rather than
|
||||
a fixed attempt count, to accommodate slow boots without making fast
|
||||
hosts wait needlessly.
|
||||
"""
|
||||
deadline = time.monotonic() + max_wait_s
|
||||
last_err: str | None = None
|
||||
for i in range(1, attempts + 1):
|
||||
attempt = 0
|
||||
while True:
|
||||
attempt += 1
|
||||
result = await session.exec(
|
||||
"curl",
|
||||
"-fsS",
|
||||
|
|
@ -74,10 +82,16 @@ async def _login_as_guest(
|
|||
else:
|
||||
stderr = result.stderr.decode("utf-8", errors="replace")[:200]
|
||||
last_err = f"curl exit {result.exit_code}: {stderr}"
|
||||
logger.debug("loginAsGuest attempt %d/%d failed: %s", i, attempts, last_err)
|
||||
await asyncio.sleep(min(2.0 * i, 8.0))
|
||||
|
||||
raise RuntimeError(f"loginAsGuest failed after {attempts} attempts: {last_err}")
|
||||
remaining = deadline - time.monotonic()
|
||||
if remaining <= 0:
|
||||
break
|
||||
logger.debug("loginAsGuest attempt %d failed: %s", attempt, last_err)
|
||||
await asyncio.sleep(min(2.0 * attempt, 8.0, remaining))
|
||||
|
||||
raise RuntimeError(
|
||||
f"loginAsGuest failed after {attempt} attempts over {max_wait_s:.0f}s: {last_err}"
|
||||
)
|
||||
|
||||
|
||||
async def bootstrap_caido(
|
||||
|
|
@ -85,11 +99,14 @@ async def bootstrap_caido(
|
|||
*,
|
||||
host_url: str,
|
||||
container_url: str,
|
||||
boot_wait_s: float = 180.0,
|
||||
) -> Client:
|
||||
"""Connect to the in-container Caido sidecar and select a fresh project."""
|
||||
logger.info("Bootstrapping Caido client (host=%s, container=%s)", host_url, container_url)
|
||||
|
||||
access_token = await _login_as_guest(session, container_url=container_url)
|
||||
access_token = await _login_as_guest(
|
||||
session, container_url=container_url, max_wait_s=boot_wait_s
|
||||
)
|
||||
|
||||
client = Client(host_url, auth=TokenAuthOptions(token=access_token))
|
||||
await client.connect()
|
||||
|
|
|
|||
|
|
@ -183,6 +183,7 @@ async def create_or_reuse(
|
|||
session,
|
||||
host_url=host_caido_url,
|
||||
container_url=container_caido_url,
|
||||
boot_wait_s=load_settings().runtime.caido_boot_wait_s,
|
||||
)
|
||||
|
||||
bundle = {
|
||||
|
|
|
|||
87
tests/test_caido_bootstrap.py
Normal file
87
tests/test_caido_bootstrap.py
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from strix.runtime.caido_bootstrap import _login_as_guest
|
||||
|
||||
|
||||
@dataclass
|
||||
class _FakeResult:
|
||||
exit_code: int
|
||||
stdout: str = ""
|
||||
stderr: bytes = b""
|
||||
|
||||
def ok(self) -> bool:
|
||||
return self.exit_code == 0
|
||||
|
||||
|
||||
@dataclass
|
||||
class _FakeSession:
|
||||
"""Stands in for BaseSandboxSession; returns queued exec() results."""
|
||||
|
||||
results: list[_FakeResult]
|
||||
sleeps: list[float] = field(default_factory=list)
|
||||
calls: int = 0
|
||||
|
||||
async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeResult:
|
||||
result = self.results[min(self.calls, len(self.results) - 1)]
|
||||
self.calls += 1
|
||||
return result
|
||||
|
||||
|
||||
_FAKE_TOKEN = "guest-token" # noqa: S105 - test fixture, not a real credential
|
||||
|
||||
|
||||
def _token_result() -> _FakeResult:
|
||||
body = {"data": {"loginAsGuest": {"token": {"accessToken": _FAKE_TOKEN}}}}
|
||||
return _FakeResult(exit_code=0, stdout=json.dumps(body))
|
||||
|
||||
|
||||
def _refused_result() -> _FakeResult:
|
||||
return _FakeResult(exit_code=7, stderr=b"curl: (7) Failed to connect")
|
||||
|
||||
|
||||
async def test_login_as_guest_succeeds_once_caido_is_up(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr("strix.runtime.caido_bootstrap.asyncio.sleep", _no_sleep)
|
||||
session = _FakeSession(results=[_refused_result(), _refused_result(), _token_result()])
|
||||
|
||||
token = await _login_as_guest(session, container_url="http://127.0.0.1:48080", max_wait_s=30)
|
||||
|
||||
assert token == _FAKE_TOKEN
|
||||
assert session.calls == 3
|
||||
|
||||
|
||||
async def test_login_as_guest_respects_configurable_deadline(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A slow-booting sandbox (connection refused throughout) should be given
|
||||
the full configured budget rather than giving up after a fixed attempt
|
||||
count, and the raised error should report elapsed time, not attempts.
|
||||
"""
|
||||
monkeypatch.setattr("strix.runtime.caido_bootstrap.asyncio.sleep", _no_sleep)
|
||||
|
||||
# Fake clock: each read advances by 25s, so a 180s budget survives
|
||||
# ~7 attempts (the old fixed-10-attempt loop only had ~68s total).
|
||||
fake_now = [0.0]
|
||||
|
||||
def _fake_monotonic() -> float:
|
||||
fake_now[0] += 25.0
|
||||
return fake_now[0]
|
||||
|
||||
monkeypatch.setattr("strix.runtime.caido_bootstrap.time.monotonic", _fake_monotonic)
|
||||
session = _FakeSession(results=[_refused_result()])
|
||||
|
||||
with pytest.raises(RuntimeError) as exc_info:
|
||||
await _login_as_guest(session, container_url="http://127.0.0.1:48080", max_wait_s=180)
|
||||
|
||||
assert "curl exit 7" in str(exc_info.value)
|
||||
assert "180s" in str(exc_info.value)
|
||||
assert session.calls >= 6
|
||||
|
||||
|
||||
async def _no_sleep(_seconds: float) -> None:
|
||||
"""asyncio.sleep stub so deadline-bound tests run instantly."""
|
||||
Loading…
Add table
Reference in a new issue