fix(strix): fail closed on incomplete provider scans - #1153
Conversation
|
Current-main successor for #1138: head |
|
Warning Review limit reached
Next review available in: 45 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughStrix에 PR-head 컨텍스트 수집과 격리된 작업 디렉터리 실행이 추가되었습니다. ChangesStrix 스캔 경계와 컨텍스트
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Current-head review request for PR #1153:
Please provide a fresh independent review for this exact head. Protected current-head checks and qualifying approval remain required before merge. |
|
Current-head verification for |
|
@opencode-agent Review exact current head |
Keep Vulnerabilities [1-9] fail-closed. A scanner-model error without a numbered finding is infrastructure noise, not a security result.
998ad7b to
51c3815
Compare
|
@opencode-agent Review exact current head 51c3815 against main@2cce96f8. Rebased the Strix ModelBehaviorError classifier onto current main; only a qualified exception with no numbered vulnerability/severity finding neutralizes a backend flake, while real findings remain fail-closed. Verified: 63 focused Strix/queue tests, actionlint, compileall, and git diff --check passed. |
|
Exact-head control-plane contradiction on A scanner turn that terminated with Deterministic counter-evidence is preserved at commit Please keep this PR unmerged unless incomplete provider/model execution returns an explicit typed non-passing result. A genuine complete zero-finding scan can still pass; actual findings remain independently fail-closed. |
|
@opencode-agent Review exact current head |
|
@opencode-agent Review current exact head |
|
@opencode-agent Review exact current head SHA 86c262c. Confirm the typed STRIX_PROVIDER_UNAVAILABLE evidence, nonzero required result, current checks, and test/coverage evidence. Return a formal structured current-head verdict. |
|
@opencode-agent Review the current exact head as the sole current-main successor to #1138. Verify the classifier only treats trusted Strix-process |
|
@opencode-agent review exact current head |
|
Integrated current main normally and restored the unrelated main-document merge artifacts; current PR diff is scoped to the Strix workflow, doctoring, changelog, and two contract-test files at 119d563. Focused Strix/queue suite passed (77 tests, 16 subtests), actionlint, ruff, and diff checks passed. Re-review this exact HEAD. @OpenCode review this exact HEAD and report only current-head findings. |
|
@opencode-agent review this exact current HEAD. Use the current commit SHA, current Checks, and current diff; do not reuse prior approvals or prior-head evidence. |
|
Exact-head repair pushed as c1bac0a from 1f8878d. Root cause: the PR branch still pinned pip 26.1.2 in the hashed audit lock, reproducing the known PYSEC-2026-3721 failure. Updated only that lock to pip 26.2.1 with exact hashes. Verification: 79 focused Strix and required-workflow tests passed; pip-audit reported no known vulnerabilities; compileall and git diff check passed. Hosted checks and exact-head independent approval must be re-evaluated for c1bac0a. No force merge or bypass was used. |
|
@claude Please perform a review-only formal review of exact head |
|
pip-audit reported PYSEC-2026-3721 against pip 26.1.2. Updated the exact pinned lock to pip 26.2.1 with both release hashes. Local validation passed: 53 queue-contract tests, locked pip-audit, and diff checks. Hosted Checks and an exact-head independent approval remain required. |
Exact-head maintainer audit
|
|
Exact-head Strix fail-closed repair regenerated at Fresh causal verification:
Hosted checks and a substantive exact-current-head Reviews API verdict are now required. Do not reuse predecessor evidence or merge through a bypass. @opencode-agent review this exact head only. Review-only: do not mutate branches, merge state, rulesets, or releases. |
|
Exact-head regression-harness repair:
The production Strix workflow/gate bytes are unchanged by this follow-up. A repeated standalone quick-gate invocation was stopped after an environment-only hang with no new output; the previously published exact production tree had already completed that suite, while this head's hosted evidence is regenerating and remains non-passing until terminal. @opencode-agent review this exact head only. Review-only; do not mutate or merge the branch. |
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (2 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow (2 files)"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (2 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (2 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (4 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (4 files)"]
R3 --> V3["docs review"]
Evidence --> S4["CI script (3 files)"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script (3 files)"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (6 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (6 files)"]
R5 --> V5["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: affirmative current-head evidence (focused hunks of .github/workflows/strix.yml and .github/workflows/strix-changed-path-quality-ci.yml, CHANGELOG.md entry, diff stat, receipts) supports the fail-closed intent beyond absence of blockers. Verification posture: CodeGraph current-head index (213 files, up to date) plus focused changed hunks reviewed; direct opencode-pr-head reads blocked by runtime policy, recorded as a review source limitation. Linter/static: no completed failed GitHub Checks at this head; quality lane runs bash -n, compileall, git diff --exit-code, and actionlint-covered workflows. TDD/regression: behavior flip is pinned by renamed tests/test_strix_local_proxy_bootstrap_failure_is_classified.py, extended tests/test_strix_backend_unavailable_after_exempted_finding.py, and new tests/test_strix_model_behavior_error.py wired into the quality trigger. Coverage: Coverage execution evidence Result: PASS — supported repository test suites passed (coverage_commands contract python3 -m coverage run -m pytest tests). Docstring coverage: Coverage execution evidence records configured repository docstring gates passed or advisory (interrogate contract present). DAG: Mermaid flowchart maps the base-to-head changed flow from scripts/ci/strix_quick_gate.sh rc=1 through the strix.yml classification branch to fail-closed exits and the quality-CI rerun path. PoC/execution: trusted source-trace/diff probes only; no browser/runtime-tool receipts claimed because none exist in bounded evidence. DDD/domain: scanner-evidence trust boundary kept authoritative — incomplete scans never become passing security evidence. CDD/context: Strix owner lane consolidated; changed-path policy keeps trigger and compileall sets aligned. Similar issues: no unresolved peer threads (Other unresolved review thread evidence empty); historical Devin/CodeRabbit comments reconciled against current-head sections with no corroborated active defect. Claim/concept check: CHANGELOG claim matches executable branch (::error STRIX_PROVIDER_UNAVAILABLE, nonzero exit). Standards search: no external standard claim required; shell/regex behavior verified from hunks. Compatibility/convention: identifiers STRIX_PROVIDER_UNAVAILABLE, model_behavior_error_signal, strix_neutralization_scope_log are multi-word and convention-consistent; no schema/API/name-bearing objects changed. Breaking-change/backcompat: intentional required-check semantics change from neutral skip to failing; changelog documents it and consumers treat nonzero as failure, preserving exit-code vocabulary (0/1/2). Implementation completeness: no placeholder bodies introduced; removed neutral-skip branch fully replaced by classified hard-failure branch. Performance: grep/awk over bounded run logs only; budgets unchanged. Developer experience: typed ::error annotation improves outage triage versus generic warning. User experience: merge-blocking on provider outages is the intended buyer-surface safety trade-off; artifacts and run log remain published via always() collection step. Visual/DOM: non-web interaction surface reviewed (workflow logs, status-check annotations, CLI gate output); no web UI changed. Accessibility/i18n: no UI surface; annotation titles are plain ASCII English consistent with existing annotations. Supply-chain/license: hash-pinned --require-hashes install retained; permissions contents: read unchanged. Packaging: pyproject-driven pytest/coverage/interrogate contracts apply and Coverage execution evidence reports supported suites passed. Security/privacy: fail-closed direction strengthens the security boundary; no secrets, ids, or tenant surfaces touched.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/strix-changed-path-quality-ci.yml, .github/workflows/strix.yml, CHANGELOG.md, docs/doctoring/strix-model-behavior-error.md, docs/doctoring/strix-nvidia-nim-not-found-fallback.md, and 12 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/strix-changed-path-quality-ci.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":".github/workflows/strix.yml","line":906,"hypothesis":"A provider/backend-exhausted scan with zero findings still converts to a neutral passing result (exit 0), regressing the buyer-surface incident.","attack_or_counterexample":"Simulated gate exit 1 whose console-log tail after 'allowing pipeline continuation' contains 'LLM CONNECTION FAILED' and no Vulnerabilities/severity markers.","evidence":"Trusted focused-hunk source trace at .github/workflows/strix.yml:906 observed the classification branch close immediately after '::error title=STRIX_PROVIDER_UNAVAILABLE...' and 'exit \"$strix_rc\"' with strix_rc=1, yielding step exit code 1; the previous 'exit 0' neutral-skip line is absent from the head hunk, and the renamed regression test tests/test_strix_local_proxy_bootstrap_failure_is_classified.py plus Coverage execution evidence Result: PASS corroborate the classified non-passing outcome. Trusted current-head source binding at .github/workflows/strix.yml:906; source-line-sha256=8759fc7a2ae8ae286889cd4dcbf142fc7ed5f87a93b7437debeecfec17311c53","outcome":"falsified"},{"path":".github/workflows/strix.yml","line":856,"hypothesis":"The patch relabels provider failure as benign infrastructure noise that does not affect the required check, contradicting the stated fail-closed intent.","attack_or_counterexample":"Compare the head comment block claiming provider failure 'remains non-passing because no authoritative complete vulnerability result exists' against the executable branch for a rate-limit ('RateLimitError') tail with zero findings.","evidence":"Trusted focused-hunk source trace at .github/workflows/strix.yml:856 observed the head comment block declaring provider failure non-passing, and the executable branch it describes (lines 901-906 of the same hunk) terminates every signal-matched exit-1 outcome in 'exit \"$strix_rc\"'; documentation and code agree with no passing path for incomplete scans. Trusted current-head source binding at .github/workflows/strix.yml:856; source-line-sha256=3e0eab738bf434551a00c6a7f78d861a44c96bfd0e883f92660515364bb6dc81","outcome":"falsified"},{"path":".github/workflows/strix-changed-path-quality-ci.yml","line":8,"hypothesis":"Changes to .github/workflows/strix.yml and the new Strix test/doc files escape exact-head quality verification because the focused quality workflow does not trigger on them.","attack_or_counterexample":"Open a PR touching only .github/workflows/strix.yml, docs/doctoring/strix-model-behavior-error.md, tests/test_strix_model_behavior_error.py, and tests/test_strix_nvidia_nim_not_found_fallback.py and evaluate path-filter matching.","evidence":"Trusted focused-hunk source trace at .github/workflows/strix-changed-path-quality-ci.yml:8 observed '.github/workflows/strix.yml' added to the pull_request paths filter alongside the new doc and test paths, and the companion hunk at line 73 added the two new test modules to the exact-head compileall set after 'bash scripts/ci/test_strix_quick_gate.sh', so every listed surface reruns the exact-head policy job. Trusted current-head source binding at .github/workflows/strix-changed-path-quality-ci.yml:8; source-line-sha256=5c27be631f78e8b8379d9c8eab1d3cb0e615bfec4b316b6936d49312419fe602","outcome":"falsified"}],"residual_risk":"Signal classification is wording-based over scanner console logs; unrecognized provider failure phrasings fall through to the generic hard-failure branch, so residual exposure is limited to diagnostic mislabeling, not to a passing incomplete scan. Direct reads of the PR head tree were blocked by runtime directory policy, leaving out-of-hunk changed files verified via repeated current-head evidence sections, receipts, diff stat, and trusted Coverage PASS rather than byte-level model inspection."}- Result: APPROVE
- Reason: The provider-outage path in .github/workflows/strix.yml now fails closed with typed STRIX_PROVIDER_UNAVAILABLE evidence; adversarial probes on downgrade, finding-suppression, and verification-escape hypotheses were falsified against current-head hunks and trusted Coverage PASS evidence.
- Head SHA:
412370e16c51b2b21d5de27092089590d5efb425 - Workflow run: 32629388594
- Workflow attempt: 1
Buyer-surface incident
Strix provider scans must never publish a successful result when scope evidence is incomplete, stale, or written outside the trusted report boundary. This PR repairs the fail-closed boundary while preserving useful provider diagnostics and the existing scan contract.
Change
docs/doctoring/strix-model-behavior-error.mdrecord and its executable quality/test references.Exact current identity
main@9ad0ad50409561292b424d6f35a95d670a277e77codex/pr1138-current-main-successor@09c34396244fa105d6fdab38a0b7ec05d13f0c1dThe pip-audit lock change is excluded from this owner lane and remains owned by #1198.
Verification
git diff --check: passed.Hosted gate and next action
Hosted workflows must regenerate for
09c34396244fa105d6fdab38a0b7ec05d13f0c1d; pending, queued, skipped, cancelled, absent, neutral, or stale predecessor evidence is non-passing. No qualifying exact-current-head independent formal verdict exists.Merge only after every required exact-head check is terminal-success, all valid threads are resolved, and independent approval is present. No self-approval, guarded bypass, direct protected-branch push, force-push, release, or ruleset change is requested.