From 8ce917765d9f04619cb7489b6000b439c7e8ac37 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 20:17:31 +0900 Subject: [PATCH] fix(review): name the Strix sandbox class in the failed-check finding `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. This consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census -- the misattribution #1953 fixed in the gate, surviving in the reader. The emitter now branches on the second token. A sandbox verdict gets a finding that names Strix's sandbox, states that 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 four lines verbatim, so the gateway class has no regression surface. No test covered this finding text at all before ("gateway or its discovered provider pool" and "provider availability blocked" both matched nothing under tests/). tests/test_opencode_dispatch_strix_sandbox_finding.py runs the production emitter, extracted from the published run block with the existing _extract_run_block harness, and pins three directions: the sandbox token, a gateway failure without it, and evidence carrying no provider-unavailable signal at all. Editing the workflow moves its blob, so REVIEW_DISPATCH_BLOB_SHA in tests/test_pr_review_autofix_nvidia_nim_contract.py is recomputed to 694c04b1ec967fee24412752cca599d69d92438e with git hash-object (lane peer 1 flagged this pin in advance). Gate on this tree: 2931 passed, 1 skipped, 21 subtests; coverage 100% (0 missed); interrogate 100%. Negative control on origin/main's workflow with this test file: 1 failed, 2 passed -- only the sandbox direction fails there. Refs #1953, #1935. Co-Authored-By: Claude Opus 5 --- .../workflows/opencode-review-dispatch.yml | 18 ++++ CHANGELOG.md | 4 + ...opencode_dispatch_strix_sandbox_finding.py | 94 +++++++++++++++++++ ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 4 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 tests/test_opencode_dispatch_strix_sandbox_finding.py diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 26e8555967..694c04b1ec 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -5614,6 +5614,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 75a4109c9d..d06088795c 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 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. 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 2d2304aaf1..e9e6188006 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 = "26e8555967171a5f3974602ac05700c27bddebf1" +REVIEW_DISPATCH_BLOB_SHA = "694c04b1ec967fee24412752cca599d69d92438e" def _workflow_text(path: Path) -> str: