diff --git a/CHANGELOG.md b/CHANGELOG.md index efd2771e96..75a4109c9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Strix gate names the sandbox bootstrap failure and retries it once + +- `scripts/ci/strix_quick_gate.sh` gives the Caido sandbox bootstrap race (`loginAsGuest failed after 10 attempts` on `127.0.0.1:`, upstream usestrix/strix#1036/#1037/#1056) its own bounded same-model retry budget, `STRIX_SANDBOX_BOOTSTRAP_RETRIES` (default 1), drawn on top of `STRIX_TRANSIENT_RETRY_PER_MODEL`. That budget is 0 in production because the gateway owns model failover, so the documented sandbox retry never ran: `argos` Strix run 34013128112 (2026-09-06) shows one attempt, `Docker image ready`, the proxy never reachable, Strix exiting after 240 s -- while the sidecar reported four ready and four deferred routes that were never called. The budget is charged in the same branch that grants the attempt, so a log matching the sandbox class together with a gateway class cannot extend the loop without charging it (caught by adversarial review of the first draft). The primary-scan verdict for that class now reads `STRIX_PROVIDER_UNAVAILABLE: STRIX_SANDBOX_UNAVAILABLE: the last Strix attempt ended in the sandbox bootstrap (...) after N sandbox-specific same-model retries (budget B); this verdict names Strix's sandbox, not the LLM gateway.` instead of `orchestrator/free exhausted`, stating only what the gate observed; the leading token is unchanged so the workflow's finding-free classification and its tests are untouched, and the second token lets the review census split sandbox outages from gateway ones (two of six recent Strix artifacts were this class). Refs #1948. + ### Review sidecar preflight fills the served set lazily to a readiness target - `_preflight_review_agents` now treats the catalog as a candidate list, probed in its tier-then-round-robin order until `REVIEW_PREFLIGHT_TARGET_READY = 8` routes are ready or `REVIEW_PREFLIGHT_MAX_PROBES = 16` probes are spent (ADR-0029). The two-stage candidate budget rises from 12 to 24 (`REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES`; auto pool split 16 free / 8 priced; the sidecar's and the launcher's `ORCHESTRATOR_CATALOG_LIMIT` defaults follow), the production `free` pool lists all 24 (12 before), and the per-account cap stays 8. An account that answers 429 to `REVIEW_PREFLIGHT_ACCOUNT_SKIP_AFTER_429 = 2` consecutive probes has its remaining candidates skipped without a probe (a 429 is a per-key answer), so the probes it would have spent reach the other accounts' next candidates — under the real 2026-09-06 order that is the difference between about five ready routes and the target of eight — and a fully rate-limited hour costs two probes per account instead of the whole budget; the report gains `skipped_count` and `account_skip_after_429`. The sidecar's job-log echo of the preflight JSON grows from 160 to 400 lines so 16 probed routes are not cut off exactly in the dead hour the summary matters. A permanently dead candidate -- NIM lists `gemma-3-12b`/`gemma-3-4b` and answers 404 on every run -- now costs one probe instead of a served slot, and a healthy pool stops early instead of always probing every candidate. Motivation: after #1939's four-per-account slice each NVIDIA key's slots were its first four models alphabetically, two of them those 404s, so preflight readiness fell from 6/12 to 1–3/12 and `noema-review` on this repository went from 7 successes / 14 failures to 0 / 22. The report gains `candidate_count`, `target_ready` and `probe_budget`; `probed_count` counts probes actually sent. ADR-0003's stage-budget sentence is amended. Refs #1939, #1947, #1948. diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 3a563d7020..617ca7c394 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -46,6 +46,18 @@ STRIX_EXECUTABLE_ROOT="${STRIX_EXECUTABLE_ROOT:-}" STRIX_EXECUTABLE_SHA256="${STRIX_EXECUTABLE_SHA256:-}" STRIX_TRANSIENT_RETRY_PER_MODEL="${STRIX_TRANSIENT_RETRY_PER_MODEL:-0}" STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS="${STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS:-3}" +## Extra same-model attempts granted only to the Caido sandbox bootstrap race +## (is_caido_bootstrap_timing_error), on top of STRIX_TRANSIENT_RETRY_PER_MODEL. +## That budget is 0 in production because the gateway owns model failover, but +## the sandbox never reaches the model: a fresh container is the only cure for +## a proxy that never came up, and without this the documented retry never ran +## (argos run 34013128112, 2026-09-06: one attempt, then the gateway blamed). +## A sandbox retry waits the same STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS as any +## other retry -- a pause between container attempts, not an inference deadline. +STRIX_SANDBOX_BOOTSTRAP_RETRIES="${STRIX_SANDBOX_BOOTSTRAP_RETRIES:-1}" +## Sandbox-specific retries actually taken by the primary model's attempt +## loop; the final verdict reports this observed count, not the budget. +SANDBOX_RETRIES_USED=0 STRIX_FAIL_ON_MIN_SEVERITY="${STRIX_FAIL_ON_MIN_SEVERITY:-MEDIUM}" STRIX_FAIL_ON_PROVIDER_SIGNAL="${STRIX_FAIL_ON_PROVIDER_SIGNAL:-0}" RUN_START_EPOCH=0 @@ -885,6 +897,7 @@ if is_github_models_model "$PRIMARY_MODEL" && [ -z "$LLM_API_BASE_FILE" ]; then fi require_non_negative_integer "$STRIX_TRANSIENT_RETRY_PER_MODEL" "STRIX_TRANSIENT_RETRY_PER_MODEL" +require_non_negative_integer "$STRIX_SANDBOX_BOOTSTRAP_RETRIES" "STRIX_SANDBOX_BOOTSTRAP_RETRIES" require_non_negative_integer "$STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS" "STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS" require_non_negative_integer "$STRIX_PROCESS_TIMEOUT_SECONDS" "STRIX_PROCESS_TIMEOUT_SECONDS" require_non_negative_integer "$STRIX_TOTAL_TIMEOUT_SECONDS" "STRIX_TOTAL_TIMEOUT_SECONDS" @@ -3092,6 +3105,8 @@ run_strix_with_transient_retry() { local model="$1" local max_attempts=$((STRIX_TRANSIENT_RETRY_PER_MODEL + 1)) local attempt=1 + local sandbox_retries_used=0 + SANDBOX_RETRIES_USED=0 while [ "$attempt" -le "$max_attempts" ]; do local run_rc=0 @@ -3107,7 +3122,18 @@ run_strix_with_transient_retry() { fi if [ "$attempt" -ge "$max_attempts" ]; then - return 1 + ## The per-model budget is spent. The sandbox bootstrap race is not + ## a model failure, so it may draw on its own bounded budget. The + ## budget is charged HERE, in the same branch that grants the + ## attempt: charging it anywhere else lets a log that matches the + ## sandbox class together with another class grant without + ## charging, and nothing in production bounds the loop then. + if is_caido_bootstrap_timing_error && [ "$sandbox_retries_used" -lt "$STRIX_SANDBOX_BOOTSTRAP_RETRIES" ]; then + max_attempts=$((max_attempts + 1)) + sandbox_retries_used=$((sandbox_retries_used + 1)) + else + return 1 + fi fi if [ "$STRIX_TOTAL_TIMEOUT_SECONDS" -gt 0 ] && [ "$(remaining_total_budget)" -le 0 ]; then @@ -3138,6 +3164,10 @@ run_strix_with_transient_retry() { retry_reason="Caido sandbox bootstrap timing" fi echo "Retrying model '$model' due to $retry_reason (attempt $((attempt + 1))/$max_attempts)." >&2 + ## Reported only once the retry really runs: a granted attempt can + ## still be vetoed by the timeout / transient checks above, and the + ## verdict must state retries taken, not budget spent. + SANDBOX_RETRIES_USED="$sandbox_retries_used" sleep "$STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS" attempt=$((attempt + 1)) done @@ -4335,6 +4365,21 @@ run_current_target_scan() { local strict_primary_provider_fallback=0 if [ "$INFRA_ERROR_DETECTED" -eq 1 ] && provider_signal_fail_closed_enabled; then if is_contextual_orchestrator_model "$PRIMARY_MODEL"; then + ## Name the component that actually failed. The sandbox race ends + ## the run before any model request, so blaming the gateway here + ## corrupted every census that read this line (2026-09-06: two of + ## six recent Strix artifacts were this class, with the sidecar + ## reporting ready routes that were never called). The leading + ## STRIX_PROVIDER_UNAVAILABLE token is kept: the workflow classifies + ## a finding-free sandbox outage as incomplete infrastructure + ## evidence, and its tests pin that. + if is_caido_bootstrap_timing_error; then + ## Only what the gate observed: the last attempt's log shows the + ## sandbox bootstrap failure, and this many sandbox-specific + ## retries were taken. Nothing is claimed about the gateway. + echo "STRIX_PROVIDER_UNAVAILABLE: STRIX_SANDBOX_UNAVAILABLE: the last Strix attempt ended in the sandbox bootstrap (Caido proxy on 127.0.0.1 unreachable through Strix's loginAsGuest attempts) after ${SANDBOX_RETRIES_USED} sandbox-specific same-model retries (budget ${STRIX_SANDBOX_BOOTSTRAP_RETRIES}); this verdict names Strix's sandbox, not the LLM gateway." >&2 + return 1 + fi echo "STRIX_PROVIDER_UNAVAILABLE: contextual-orchestrator/orchestrator/free exhausted; the gateway owns provider discovery and failover." >&2 return 1 elif is_model_retryable_error "$PRIMARY_MODEL" && has_distinct_fallback_model_for_model "$PRIMARY_MODEL"; then diff --git a/tests/test_strix_caido_bootstrap_timing_retry.py b/tests/test_strix_caido_bootstrap_timing_retry.py index a60b9d801b..3bb7c9e220 100644 --- a/tests/test_strix_caido_bootstrap_timing_retry.py +++ b/tests/test_strix_caido_bootstrap_timing_retry.py @@ -138,5 +138,199 @@ def test_retry_reason_is_logged_for_operators(self) -> None: ) +RATE_LIMIT_LOG = ( + "litellm.RateLimitError: RateLimitError: rate limit exceeded\n" + "Vulnerabilities 0\n" +) + + +def _run_retry_loop(log_text: str, *, per_model: int, sandbox_retries: int) -> tuple[int, str]: + """Drive the production retry loop with a stubbed Strix run and return (calls, stderr). + + The reported sandbox retry count (``SANDBOX_RETRIES_USED``) is echoed to + stdout as ``reported=`` and appended to the returned stderr text so + tests can assert it without a second harness. + + ``run_strix_once`` is replaced by a stub that writes ``log_text`` to the + attempt log and fails, so the loop's own retry decision is what is under + test; every classifier the loop consults is the production function. + """ + + gate_source = STRIX_GATE.read_text(encoding="utf-8") + blocks = [ + _function_block(gate_source, name) + for name in ( + "run_strix_with_transient_retry", + "is_transient_same_model_retry_error", + "is_timeout_error", + "is_llm_api_connection_error", + "is_llm_service_unavailable_error", + "is_rate_limit_error", + "is_midstream_fallback_error", + "is_caido_bootstrap_timing_error", + ) + ] + with tempfile.TemporaryDirectory(prefix="strix-caido-retry-") as temp_dir: + log_path = Path(temp_dir) / "strix.log" + counter = Path(temp_dir) / "calls" + counter.write_text("0", encoding="utf-8") + script = "\n".join( + ( + "set -uo pipefail", + f'STRIX_LOG="{log_path}"', + f'COUNTER="{counter}"', + f"STRIX_TRANSIENT_RETRY_PER_MODEL={per_model}", + f"STRIX_SANDBOX_BOOTSTRAP_RETRIES={sandbox_retries}", + "STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS=0", + "STRIX_TOTAL_TIMEOUT_SECONDS=0", + "TOTAL_TIMEOUT_EXCEEDED=0", + "github_models_rate_limit_should_skip_same_model_retry() { return 1; }", + # The stub caps itself: a runaway loop returns the configuration + # exit code 2 after six calls, which the harness reports as a + # failure instead of hanging the suite. + 'run_strix_once() { n=$(( $(cat "$COUNTER") + 1 )); echo "$n" > "$COUNTER"; printf "%s" "$LOG_TEXT" > "$STRIX_LOG"; [ "$n" -ge 6 ] && return 2; return 1; }', + *blocks, + 'run_strix_with_transient_retry "orchestrator/free"; rc=$?; echo "reported=$SANDBOX_RETRIES_USED"; exit "$rc"', + ) + ) + completed = subprocess.run( + ["bash", "-c", script, "strix-retry"], + check=False, + capture_output=True, + text=True, + env={"PATH": "/usr/bin:/bin", "LOG_TEXT": log_text}, + ) + calls = int(counter.read_text(encoding="utf-8").strip()) + if completed.returncode != 1: + raise AssertionError(f"rc={completed.returncode}\n{completed.stderr}") + return calls, completed.stderr + completed.stdout + + +def _orchestrator_verdict_line(log_text: str) -> str: + """Return the stderr the primary-scan verdict branch emits for a failed orchestrator scan.""" + + gate_source = STRIX_GATE.read_text(encoding="utf-8") + blocks = [ + _function_block(gate_source, name) + for name in ("run_current_target_scan", "is_caido_bootstrap_timing_error") + ] + with tempfile.TemporaryDirectory(prefix="strix-caido-verdict-") as temp_dir: + log_path = Path(temp_dir) / "strix.log" + log_path.write_text(log_text, encoding="utf-8") + script = "\n".join( + ( + "set -uo pipefail", + f'STRIX_LOG="{log_path}"', + 'PRIMARY_MODEL="orchestrator/free"', + "STRIX_SANDBOX_BOOTSTRAP_RETRIES=1", + "SANDBOX_RETRIES_USED=1", + "TOTAL_TIMEOUT_EXCEEDED=0", + # run_current_target_scan resets INFRA_ERROR_DETECTED before the + # scan; the production run_strix_once sets it on a failed attempt, + # so the stub does the same. + "run_strix_with_transient_retry() { INFRA_ERROR_DETECTED=1; return 1; }", + "provider_signal_fail_closed_enabled() { return 0; }", + "is_contextual_orchestrator_model() { return 0; }", + "is_model_retryable_error() { return 1; }", + "has_distinct_fallback_model_for_model() { return 1; }", + # has_detected_infrastructure_error is consulted by run_strix_once, + # which the stub above replaces; the flag is set by that path. + *blocks, + "run_current_target_scan", + ) + ) + completed = subprocess.run( + ["bash", "-c", script, "strix-verdict"], + check=False, + capture_output=True, + text=True, + ) + if completed.returncode != 1: + raise AssertionError(f"rc={completed.returncode}\n{completed.stderr}") + return completed.stderr + + +class StrixSandboxBootstrapRetryAndVerdictTests(unittest.TestCase): + """The sandbox race gets its own bounded retry and its own name in the verdict. + + Evidence (2026-09-06): ``argos`` Strix run 34013128112 and a second + artifact both show a single attempt ending in ``loginAsGuest failed after + 10 attempts`` on ``127.0.0.1:48080`` after ``Docker image ready``, then + ``STRIX_PROVIDER_UNAVAILABLE: … orchestrator/free exhausted`` -- while the + sidecar had four ready and four deferred routes that were never called. + ``STRIX_TRANSIENT_RETRY_PER_MODEL`` defaults to 0 and the workflow does not + raise it, so the documented same-model retry for this class never ran. + """ + + def test_sandbox_bootstrap_failure_is_retried_once_even_with_zero_per_model_budget(self) -> None: + calls, stderr = _run_retry_loop(OBSERVED_LOG, per_model=0, sandbox_retries=1) + self.assertEqual(calls, 2) + self.assertIn("Caido sandbox bootstrap timing", stderr) + self.assertIn("attempt 2/2", stderr) + + def test_sandbox_retry_budget_is_bounded(self) -> None: + calls, _ = _run_retry_loop(OBSERVED_LOG, per_model=0, sandbox_retries=2) + self.assertEqual(calls, 3) + calls, _ = _run_retry_loop(OBSERVED_LOG, per_model=0, sandbox_retries=0) + self.assertEqual(calls, 1) + + def test_mixed_sandbox_and_gateway_log_stays_bounded(self) -> None: + """A log matching the sandbox class AND a gateway class grants at most the sandbox budget. + + Found by adversarial review of the first draft, which charged the + sandbox counter in the retry-reason chain behind the gateway classes: + such a log then extended the budget on every iteration without ever + charging it, and production bounds the loop with nothing but GitHub's + six-hour default. + """ + + calls, stderr = _run_retry_loop(RATE_LIMIT_LOG + OBSERVED_LOG, per_model=0, sandbox_retries=1) + self.assertEqual(calls, 2) + self.assertNotIn("attempt 3/", stderr) + + def test_sandbox_budget_is_granted_on_top_of_the_per_model_budget(self) -> None: + calls, _ = _run_retry_loop(OBSERVED_LOG, per_model=1, sandbox_retries=1) + self.assertEqual(calls, 3) + + def test_reported_sandbox_retries_count_only_retries_that_ran(self) -> None: + """A granted attempt vetoed by the timeout check is not reported as a retry. + + Lane peer 1's verification note: the budget is charged at the grant, + but ``is_transient_same_model_retry_error`` returns 1 for a timeout + signature, so a log carrying both the sandbox and a timeout signature + is granted, charged, and then not retried; the verdict must say 0. + """ + + calls, out = _run_retry_loop( + "litellm.exceptions.Timeout: request timed out\n" + OBSERVED_LOG, + per_model=0, + sandbox_retries=1, + ) + self.assertEqual(calls, 1) + self.assertIn("reported=0", out) + calls, out = _run_retry_loop(OBSERVED_LOG, per_model=0, sandbox_retries=1) + self.assertEqual(calls, 2) + self.assertIn("reported=1", out) + + def test_sandbox_retry_does_not_widen_gateway_retries(self) -> None: + """A rate limit from the gateway still gets no same-model retry at budget 0.""" + + calls, stderr = _run_retry_loop(RATE_LIMIT_LOG, per_model=0, sandbox_retries=1) + self.assertEqual(calls, 1) + self.assertNotIn("Retrying model", stderr) + + def test_verdict_names_the_sandbox_and_keeps_the_workflow_token(self) -> None: + stderr = _orchestrator_verdict_line(OBSERVED_LOG) + self.assertIn("STRIX_PROVIDER_UNAVAILABLE: STRIX_SANDBOX_UNAVAILABLE:", stderr) + self.assertIn("after 1 sandbox-specific same-model retries (budget 1)", stderr) + self.assertIn("names Strix's sandbox, not the LLM gateway", stderr) + self.assertNotIn("orchestrator/free exhausted", stderr) + + def test_verdict_for_a_gateway_failure_is_unchanged(self) -> None: + stderr = _orchestrator_verdict_line(RATE_LIMIT_LOG) + self.assertIn("orchestrator/free exhausted", stderr) + self.assertNotIn("STRIX_SANDBOX_UNAVAILABLE", stderr) + + if __name__ == "__main__": unittest.main()