fix(strix): bounded provider-outage retries - #1332
Conversation
Transient LLM provider outages (rate limits, connection/warm-up failures, ModelBehaviorError) previously failed the required strix check on the first attempt even when no vulnerability was reported, forcing manual reruns of the whole PR queue. The gate now retries such typed outages up to 3 attempts with linear backoff inside a deterministic SECONDS-based deadline (100 min cap, >=10 min remaining required to retry), all inside the existing 120-minute job budget. Genuine vulnerability reports, configuration failures (exit 2), and unexpected exit codes never retry; every terminal outcome remains fail-closed. Signal patterns are defined before the loop and the post-loop classification is unchanged.
…ed gate - Update stale model assertions from the nonexistent gpt-5.6-luna to the shipped openai-direct/gpt-5.4 fallback (left stale by a724582). - Rework the backend-unavailable tail-scoping test to extract the neutralization/classification block (post-retry) and inject the canonical signal definitions, matching the bounded provider-outage retry loop added for the STRIX_PROVIDER_UNAVAILABLE failure class.
…iew pool Follows a724582's finding that gpt-5.6-luna 404s on the OpenAI API: the dispatch workflow still routed the review agent through that candidate, guaranteeing one wasted attempt per cycle and failing the exact-head-path policy assertions that already expected openai/gpt-5.4. Rename the embedded openai-direct catalog entry to gpt-5.4, update the pool string and the rationale comments.
…tract pool with gpt-5.4 The dispatch workflow's openai-direct slot is now gpt-5.4 (a724582: luna 404s), so update REVIEW_DISPATCH_BLOB_SHA to the recomputed blob hash and align the agent-contract candidate list/expectations.
CodeRabbit review finding: the bounded retry loop inspected the full console log while the terminal classification scopes to the tail after the last pipeline-continuation marker. An already-exempted finding before the marker therefore suppressed retries of a genuine later outage. The loop now computes the same continuation-marker scope per attempt for both the reported- vulnerability and backend/model-error checks; terminal classification is unchanged. Adds STRIX_GATE_RETRY_BACKOFF_SECONDS (default 90) so tests can exercise the loop without real sleeps, plus regression coverage for recovery after an exempted finding and zero-retry on a real tail finding.
|
Warning Review limit reachedNext included review available in 43 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 (4)
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.
|
Exact-current-head update: remote agent advanced this replacement to |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| strix_retry_scope_log="$strix_run_log" | ||
| if grep -Fq 'allowing pipeline continuation' "$strix_run_log"; then | ||
| strix_retry_scope_log="$RUNNER_TEMP/strix_gate_console_tail.log" | ||
| awk '/allowing pipeline continuation/{buf=""; next} {buf=buf $0 "\n"} END{printf "%s", buf}' \ | ||
| "$strix_run_log" > "$strix_retry_scope_log" | ||
| fi | ||
| # A reported vulnerability is authoritative evidence: never retry | ||
| # and never risk downgrading it. | ||
| if grep -Eiq "$reported_vulnerability_signal" "$strix_retry_scope_log"; then | ||
| break | ||
| fi | ||
| # Retry only recognized provider-outage / model-behavior classes. | ||
| if ! grep -Eiq "$backend_unavailable_signal" "$strix_retry_scope_log" \ | ||
| && ! grep -Eq "$model_behavior_error_signal" "$strix_retry_scope_log"; then |
There was a problem hiding this comment.
📝 Info: Tail-scoping logic duplicated in two places
The retry loop's tail-scoping and signal grep (strix.yml) duplicate the terminal classification block (strix.yml). The two are consistent today, but a future edit to one must be mirrored in the other.
Was this helpful? React with 👍 or 👎 to provide feedback.
| CHANGELOG = Path("CHANGELOG.md") | ||
| REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") | ||
| REVIEW_DISPATCH_BLOB_SHA = "ed3f7b44f9afdd6ab295426e5d0440aeca6bdfb5" | ||
| REVIEW_DISPATCH_BLOB_SHA = "0df7a17cc72a79585cec169c8299e0646f93ab02" |
There was a problem hiding this comment.
🔍 Dispatch workflow blob SHA pin updated
REVIEW_DISPATCH_BLOB_SHA was bumped (test_pr_review_autofix_nvidia_nim_contract.py) to track the comment-only edit in the dispatch workflow. Worth confirming the new value equals the actual blob hash, or the contract test fails.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded by replacement PR #1333, which retains the bounded provider-outage retry implementation and fixes the late-retry timeout/log-retention findings on a fresh branch. |
| : > "$strix_attempt_audit_log" | ||
| strix_rc=0 | ||
| strix_gate_attempt=1 | ||
| strix_gate_deadline=$(( SECONDS + 6000 )) |
There was a problem hiding this comment.
📝 Info: Retry deadline equals the step timeout
strix_gate_deadline is SECONDS + 6000 and the step has timeout-minutes: 100 (6000s). If a gate attempt overruns its internal budget, GitHub can kill the step at the same instant the deadline logic would fire, so the graceful STRIX_PROVIDER_UNAVAILABLE message can be lost. The outcome stays fail-closed and the loop's 600s reserve keeps normal paths clear, so this is not a bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if [ "$remaining_seconds" -le 600 ]; then | ||
| : > "$strix_run_log" | ||
| echo "Configured model and fallback models were unavailable before another bounded retry could start: only ${remaining_seconds}s remain in the outer Strix deadline." | tee "$strix_run_log" >&2 | ||
| strix_rc=1 | ||
| { | ||
| printf "=== outer-attempt=%s rc=%s ===\\n" "$strix_gate_attempt" "$strix_rc" | ||
| cat "$strix_run_log" | ||
| } >> "$strix_attempt_audit_log" | ||
| break |
There was a problem hiding this comment.
📝 Info: Deadline-exhaustion path typed as outage
When the top-of-loop deadline guard fires it overwrites strix_run_log with a message containing Configured model and fallback models were unavailable, which matches backend_unavailable_signal, so the terminal classifier emits STRIX_PROVIDER_UNAVAILABLE and exits 1. The prior attempt's raw log is dropped from strix_run_log but retained in the attempt audit log. Fail-closed and intended.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Replacement for #1322
Adds a bounded, fail-closed retry loop to the trusted Strix gate for typed provider/backend outages only.
Local checks:
python -m pytest tests/test_strix_backend_unavailable_after_exempted_finding.py tests/test_required_workflow_queue_contract.py::test_nvidia_nim_defaults_preserve_existing_fallbacks_without_secret tests/test_strix_nvidia_nim_not_found_fallback.py tests/test_pr_review_autofix_nvidia_nim_contract.py -q(46 passed)git diff --checkThis replaces the stale #1322 branch after the requested close-and-reupload workflow.