fix(strix): retry transient OpenRouter upstream 502s - #1359
Conversation
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Fresh cross-repository reproduction from ContextualWisdomLab/OriginWeave PR #218 exact head |
|
Fresh ScopeWeave consumer canary confirms this owner path is production-relevant on the protected-main workflow source Exact evidence:
This reproduces the same OpenRouter-502 classification gap that #1359 repairs, so use ScopeWeave #610 as an unchanged downstream GREEN canary after the owner fix integrates. Acceptance should require a real complete Strix report if governed fallback is available, otherwise an explicit non-passing provider-unavailable result; never count infrastructure exhaustion as merge-ready security evidence. Current-head review note also matters before owner integration: the Devin finding on |
|
New owner-repository canary makes this an upstream integration blocker, not only downstream consumer friction. Fresh exact evidence:
The failure chain is the same causal class #1359 owns: NVIDIA NIM Nemotron failed three bounded attempts with HTTP 429; So #897 cannot obtain merge-grade current-head security evidence while the OpenRouter 502 same-model retry gap remains. After #1359 addresses the still-unresolved Devin metadata-order/nesting finding and integrates, use an unchanged/current #897 head as a central canary: require an actual complete Strix vulnerability result (or remain non-passing on provider exhaustion), never an infrastructure-only green. |
| if grep -Eiq 'Vulnerabilities[[:space:]]+[1-9][0-9]*' "$STRIX_LOG" || | ||
| has_blocking_vulnerability_reports; then | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
📝 Info: Vulnerability guard suppresses retry for every transient family
The new guard in is_transient_same_model_retry_error (strix_quick_gate.sh) blocks same-model retry for every transient family, not just OpenRouter 502, whenever the log shows Vulnerabilities N (N>=1) or a blocking report exists. This scope is broader than the PR title implies. It reads as intentional and no fixture regresses.
Was this helpful? React with 👍 or 👎 to provide feedback.
| inline_start = line.find("{", match.end()) | ||
| fragments = [line[inline_start:]] if inline_start >= 0 else lines[index + 1 : index + 33] | ||
| if not fragments or not fragments[0].lstrip().startswith("{"): |
There was a problem hiding this comment.
🔍 OpenRouter 502 JSON detection assumes adjacency
is_openrouter_upstream_502_error (strix_quick_gate.sh) only finds the error JSON when it is inline or begins on the immediately following line. A blank line, traceback, or truncated inline body between the OpenrouterException line and the JSON makes detection silently fail, so the retry never fires. Fixtures keep them adjacent; real logs can differ.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
502responses with provider metadata as transient service unavailabilitySTRIX_TRANSIENT_RETRY_PER_MODEL, backoff, and total-budget path for fallback modelsRoot cause and exact evidence
#1357 merged as
31e5f5337d8a8d844c456fe03f123c51b62416c9. In repository-dispatch run 33006163284, the primary NVIDIA NIM model received three bounded rate-limit attempts. The first fallback,openrouter/free, then returned:It was attempted once and the gate immediately moved to direct OpenAI, which then used the same bounded retry helper three times before failing on
insufficient_quota/credit_balance_exhausted.Both the primary and every fallback already call
run_strix_with_transient_retry; the defect was not primary-only retry wiring.is_llm_service_unavailable_errorrecognized LiteLLMServiceUnavailableError/503 shapes but not this OpenRouterAPIError/502 shape, so the shared transient helper declined a same-model retry.The new classifier requires all of the following: LiteLLM
APIErrorwithOpenrouterException, JSON code502, and non-empty OpenRoutermetadata.provider_name. A target-like 502 without the provider exception remains non-retryable. No ephemeral explicit model is pinned and no provider failure becomes passing evidence.Gap: G-03 in
docs/product-technical-gap-baseline.md.Verification
vertex_ai/missing-primary|openrouter/free(2 calls), exit 1.STRIX_TEST_CASE_FILTER=openrouter-502-fallback-retry-same-model-success STRIX_TEST_TRACE_CASES=1 bash scripts/ci/test_strix_quick_gate.shSTRIX_TEST_CASE_FILTER=service-unavailable-no-llm-marker-nonrecoverable bash scripts/ci/test_strix_quick_gate.shbash scripts/ci/strix_required_workflow_smoke.shbash -n scripts/ci/strix_quick_gate.shbash -n scripts/ci/test_strix_quick_gate.shgit diff --check