diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index fd0e5ff75d..d86497b3f4 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -5636,6 +5636,24 @@ jobs: fi finding_index=$((finding_index + 1)) + + # The gate emits a second token for the class it can tell apart: + # STRIX_SANDBOX_UNAVAILABLE means Strix's own sandbox container + # never reached its Caido proxy, so the run died before the + # gateway served anything. Reporting that as "the gateway or its + # provider pool was unavailable" sends the reader to the wrong + # component -- the misattribution #1953 fixed in the gate itself, + # which survived here because this text was fixed for every + # STRIX_PROVIDER_UNAVAILABLE line. + if grep -q "STRIX_SANDBOX_UNAVAILABLE" "$strix_evidence_file"; then + printf '### %s. HIGH %s:%s - Strix sandbox bootstrap blocked current-head security evidence\n' "$finding_index" "$path" "$line" + printf -- '- Problem: Strix failed before producing vulnerability reports. The failed log reported STRIX_SANDBOX_UNAVAILABLE, which the gate emits when the run ended in Strix sandbox bootstrap after its bounded sandbox-specific retries.\n' + printf -- '- Root cause: Strix sandbox container did not reach its Caido proxy on 127.0.0.1, so the scan ended before any Vulnerability Report window was produced. This verdict names Strix sandbox, not the contextual-orchestrator gateway, and there is no application source line to patch from this evidence.\n' + printf -- '- Fix: Do not approve from this failed scan. Re-run Strix; the sandbox bootstrap is a startup race and the gate already retries it once. Do not change gateway or provider configuration on the strength of this finding.\n' + printf -- '- Regression test: Keep the gate emitting STRIX_SANDBOX_UNAVAILABLE for sandbox bootstrap failures and keep this consumer reading it, so a sandbox outage is never reported as a gateway outage.\n\n' + return 0 + fi + printf '### %s. HIGH %s:%s - Contextual-orchestrator provider availability blocked current-head security evidence\n' "$finding_index" "$path" "$line" printf -- '- Problem: Strix failed before producing vulnerability reports. The failed log reported LLM CONNECTION FAILED, RateLimitError or Too many requests, budget-limit output, gateway exhaustion, and Configured model and fallback models were unavailable.\n' printf -- '- Root cause: The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run; no Strix Vulnerability Report window was produced, so there is no application source line to patch from this evidence.\n' diff --git a/CHANGELOG.md b/CHANGELOG.md index f03bd5104c..bf192f6a9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Failed-check finding names the Strix sandbox instead of the gateway + +- `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. + ### Strix gate keeps a recovered transient model error from failing a completed scan - `scripts/ci/strix_quick_gate.sh` `sanitize_known_strix_report_warnings` now also strips strix-agent's `strix.core.execution: transient model/provider error for ; replaying turn (attempt n/m, backoff Ns): …` WARNING lines before the report failure-signal scan. strix-agent 1.5.3 (`strix/core/execution.py:763`) emits that line only inside its bounded transient-retry branch, immediately before the replay runs; an exhausted retry logs `agent run failed for …; marking failed` at ERROR with a traceback and exits non-zero, and both of those still fail the gate. Observed on `.github#1689` run `34013778497`: a completed 63-minute scan (`run.json` `completed`, SARIF 0 results, attempt exit 0) was failed closed as `STRIX_PROVIDER_UNAVAILABLE … exhausted` on three such warnings, and the scheduler then dispatched another same-head scan. The pattern is anchored before the exception repr so the same class keeps matching after a gateway pin advance changes the exception type; re-verify the message format on every strix-agent bump. One documented side effect: when a provider's 503 body appears only inside a retry line's exception repr, removing that line also removes the only text `has_strix_report_provider_failure_signal` would have matched in the report log, which can make `is_model_retryable_error`'s report-only branch read a genuine outage as non-retryable. The direction is fail-closed (an exhausted retry still exits non-zero with its ERROR and traceback retained), and with a contextual-orchestrator primary the verdict branch answers before that classifier is consulted, so no path today changes its outcome; if fallback-model classification is ever wanted for a non-gateway primary, read the pre-sanitize attempt copy that `preserve_attempt_log` already keeps. Tests: `tests/test_strix_recovered_transient_sanitizer.py`. diff --git a/tests/test_opencode_dispatch_strix_sandbox_finding.py b/tests/test_opencode_dispatch_strix_sandbox_finding.py new file mode 100644 index 0000000000..ae73576cb0 --- /dev/null +++ b/tests/test_opencode_dispatch_strix_sandbox_finding.py @@ -0,0 +1,94 @@ +"""The failed-check finding must name Strix sandbox when the gate named it. + +`#1953` gave the Strix sandbox bootstrap failure its own verdict token, +`STRIX_SANDBOX_UNAVAILABLE`, precisely because reporting it as +`contextual-orchestrator/orchestrator/free exhausted` sent readers to a +component the run never reached. This consumer rendered one fixed finding for +every `STRIX_PROVIDER_UNAVAILABLE` line, so the corrected verdict was being +re-attributed to the gateway one step downstream, and no test covered the text +at all. These tests pin both directions. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +from tests.test_opencode_workflow_shell_syntax import _extract_run_block + +WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") +STEP_NAME = "Publish OpenCode review outcome" +FUNCTION = "emit_strix_provider_failure_finding" + + +def _emitter_source() -> str: + """Return the emitter function's shell source from the published run block.""" + script = _extract_run_block(WORKFLOW.read_text(encoding="utf-8"), STEP_NAME) + start = script.index(f"{FUNCTION}() {{") + # ``_extract_run_block`` dedents the YAML block scalar, leaving the + # function body at two spaces and its closing brace on a line of its own. + closing = "\n }\n" + end = script.index(closing, start) + len(closing) + return script[start:end] + + +def _run_emitter(evidence: str, tmp_path: Path) -> str: + """Run the production emitter against one evidence file and return its finding text.""" + evidence_file = tmp_path / "strix-evidence.txt" + evidence_file.write_text(evidence, encoding="utf-8") + harness = tmp_path / "harness.sh" + harness.write_text( + "set -euo pipefail\n" + f'strix_evidence_file="{evidence_file}"\n' + f'repo_root="{tmp_path}"\n' + "finding_index=0\n" + f"{_emitter_source()}\n" + f"{FUNCTION}\n", + encoding="utf-8", + ) + result = subprocess.run( + ["bash", str(harness)], capture_output=True, text=True, check=True + ) + return result.stdout + + +def test_sandbox_token_reports_the_sandbox_not_the_gateway(tmp_path: Path) -> None: + """A `STRIX_SANDBOX_UNAVAILABLE` verdict never blames the gateway or its provider pool.""" + finding = _run_emitter( + "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 1 " + "sandbox-specific same-model retries (budget 1); this verdict names " + "Strix's sandbox, not the LLM gateway.\n", + tmp_path, + ) + + assert "Strix sandbox bootstrap blocked current-head security evidence" in finding + assert "STRIX_SANDBOX_UNAVAILABLE" in finding + assert "names Strix sandbox, not the contextual-orchestrator gateway" in finding + assert "gateway or its discovered provider pool was unavailable" not in finding + # The reader must not be sent to change gateway configuration. + assert "Do not change gateway or provider configuration" in finding + assert finding.startswith("### 1. HIGH .github/workflows/strix.yml:") + + +def test_gateway_failure_keeps_its_existing_finding(tmp_path: Path) -> None: + """Without the sandbox token the previous gateway text is emitted unchanged.""" + finding = _run_emitter( + "STRIX_PROVIDER_UNAVAILABLE: contextual-orchestrator/orchestrator/free " + "exhausted; the gateway owns provider discovery and failover.\n", + tmp_path, + ) + + assert ( + "Contextual-orchestrator provider availability blocked current-head security evidence" + in finding + ) + assert "gateway or its discovered provider pool was unavailable" in finding + assert "STRIX_SANDBOX_UNAVAILABLE" not in finding + assert "Strix sandbox bootstrap blocked" not in finding + + +def test_unrelated_evidence_emits_no_finding(tmp_path: Path) -> None: + """Evidence with no provider-unavailable signal still produces nothing.""" + assert _run_emitter("Strix run succeeded for model 'x' in 12s.\n", tmp_path) == "" diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index af49190142..2e733ac9e9 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "fd0e5ff75d9ae6ec68cc6945174dff5c9d487662" +REVIEW_DISPATCH_BLOB_SHA = "d86497b3f43bebbabbb4f504eb5132cdf3b7b293" def _workflow_text(path: Path) -> str: