fix(strix): retry target-repository visibility lookup on transient failure - #1214
fix(strix): retry target-repository visibility lookup on transient failure#1214seonghobae wants to merge 4 commits into
Conversation
…ilure The "Resolve target repository visibility" step ran a single unretried `gh api repos/<target>` call. When the shared GitHub App installation token hits its hourly rate limit (observed on fast-mlsirm#1192, exit `API rate limit exceeded for installation ID 141441800`), that single call fails and the entire required Strix check fails closed with no security finding, immediately blocking review even though every other check and thread on the PR is clean. With ~190 open PRs across just fast-mlsirm and TEPP each running hourly review schedulers, this installation-token contention is a recurring, non-code-defect cause of stalled required checks org-wide. Retry the lookup up to 6 times with linear backoff (5s x attempt) before failing, mirroring the existing PR-head-fetch retry loop later in this same workflow. Still fails closed if visibility genuinely cannot be resolved after retries; does not change Strix's scan semantics, model routing, or bypass posture. Verified: actionlint (incl. embedded shellcheck) clean; scripts/ci/test_strix_quick_gate.sh passes unchanged.
|
Warning Review limit reached
Next review available in: 33 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 (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}
| for target_visibility_attempt in 1 2 3 4 5 6; do | ||
| if is_private="$(gh api "repos/${TARGET_REPOSITORY}" --jq '.private')"; then | ||
| break | ||
| fi | ||
| is_private="" | ||
| if [ "$target_visibility_attempt" -lt 6 ]; then | ||
| echo "Repository visibility lookup failed (attempt ${target_visibility_attempt}/6), possibly a transient GitHub API rate limit; retrying after backoff." >&2 | ||
| sleep "$(( target_visibility_attempt * 5 ))" | ||
| fi | ||
| done |
There was a problem hiding this comment.
📝 Info: Retry loop delays permanent failures too
The loop retries any gh api failure six times, not only transient rate limits. A permanent failure (404, denied auth) now waits the full 75s backoff before failing closed instead of failing immediately.
Was this helpful? React with 👍 or 👎 to provide feedback.
The sandbox container strix-agent launches runs a chown -R before starting caido-cli, and enforces a fixed 10-attempt loginAsGuest budget. A slow CI runner can exceed that budget before the local intercepting proxy is reachable, even though the penetration test itself never started and no vulnerability evidence was produced or lost: Error during penetration test: loginAsGuest failed after 10 attempts: curl exit 7: curl: (7) Failed to connect to 127.0.0.1 port 48080 after 0 ms: Could not connect to server Confirmed via `gh api search/issues` on usestrix/strix as a live, open upstream bug (not our misconfiguration): #1037 "Strix 1.5.2 fails in GitHub Actions: Caido bootstrap (loginAsGuest) cannot reach 127.0.0.1:48080", #1036 "loginAsGuest gives up after ~68s: fixed 10-attempt retry too short for slow sandbox boot", #1056 "fix: bound Caido readiness probe by wall-clock deadline, not fixed attempts" (all open). strix-agent==1.5.3 is the current PyPI release; our strix.yml invocation skips no setup step and the sandbox image was already pulled and ready before the failure. strix_quick_gate.sh already retries the same model up to STRIX_TRANSIENT_RETRY_PER_MODEL times (2, set in strix.yml) for rate-limit / LLM-connection / service-unavailable / midstream-fallback errors, but had no classifier for this signature, so it fell straight through to a hard `gate exit 1` on first occurrence. Add is_caido_bootstrap_timing_error() and wire it into is_transient_same_model_retry_error (retry — the Docker image is already cached, so a retry is cheap and usually clears a one-off boot race) and has_detected_infrastructure_error (safety net if retries exhaust, so a suspiciously-clean result is never silently trusted). Deliberately not wired into is_model_retryable_error (cross-model fallback): switching LLM models cannot change local sandbox boot timing. Verified: new tests/test_strix_caido_bootstrap_timing_retry.py (6 tests) pass; existing tests/test_strix_nvidia_nim_not_found_fallback.py (11 tests) unaffected; shellcheck clean; full scripts/ci/test_strix_quick_gate.sh suite passes (test_strix_quick_gate: PASS, exit 0).
Follow-up: same-model retry for the Caido sandbox bootstrap timing raceStacked a second, related Strix CI-resilience fix onto this PR (commit While watching this fix propagate through the org's PR queue, Confirmed via
Verified: new |
There was a problem hiding this comment.
🟡 Retry docstring undercounts error families
The docstring for is_transient_same_model_retry_error states "Four error families qualify" and lists four, but the function now also retries on is_caido_bootstrap_timing_error. The comment no longer matches the code.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| is_private="" | ||
| for target_visibility_attempt in 1 2 3 4 5 6; do | ||
| if is_private="$(gh api "repos/${TARGET_REPOSITORY}" --jq '.private')"; then | ||
| break | ||
| fi | ||
| is_private="" | ||
| if [ "$target_visibility_attempt" -lt 6 ]; then | ||
| echo "Repository visibility lookup failed (attempt ${target_visibility_attempt}/6), possibly a transient GitHub API rate limit; retrying after backoff." >&2 | ||
| sleep "$(( target_visibility_attempt * 5 ))" | ||
| fi | ||
| done | ||
| case "$is_private" in | ||
| true | false) ;; | ||
| *) | ||
| echo "::error::Target repository visibility did not resolve to true or false." | ||
| echo "::error::Target repository visibility did not resolve to true or false after retries." | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
📝 Info: Visibility retry loop fails closed correctly
The retry assigns inside an if, so set -euo pipefail does not abort on a failed lookup. On exhaustion is_private is reset to empty and the case exits 1. The emitted is_private output is unchanged, so contract tests reading it still pass.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if is_midstream_fallback_error; then | ||
| return 0 | ||
| fi | ||
| if is_caido_bootstrap_timing_error; then | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
📝 Info: Caido classifier sits behind timeout precedence
is_transient_same_model_retry_error returns non-retryable if is_timeout_error matches first. The observed Caido log has no timeout string, so it reaches the Caido branch. If a Caido run also emitted Strix run timed out after, timeout precedence would suppress the retry. Unlikely, and consistent with existing policy.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
…lookup-rate-limit-retry # Conflicts: # CHANGELOG.md
…lookup-rate-limit-retry # Conflicts: # CHANGELOG.md
| is_caido_bootstrap_timing_error() { | ||
| if grep -Fq 'loginAsGuest failed after' "$STRIX_LOG" && | ||
| grep -Eq 'Failed to connect to 127\.0\.0\.1 port [0-9]+' "$STRIX_LOG"; then | ||
| return 0 | ||
| fi | ||
|
|
||
| return 1 | ||
| } |
There was a problem hiding this comment.
📝 Info: Caido classifier stays same-model-retry only
is_caido_bootstrap_timing_error feeds is_transient_same_model_retry_error and has_detected_infrastructure_error but not is_model_retryable_error, so an exhausted same-model retry budget fails closed instead of switching models. The earlier is_timeout_error guard does not intercept it, since the observed log carries a connect failure rather than a timeout string.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Root cause
The Strix required check's "Resolve target repository visibility" step ran a
single unretried
gh api repos/<target>call. Observed failure onfast-mlsirm#1192(Strix Security Scan run32526888686, job96910702255):That is the shared GitHub App installation token used by CI across the
organization, not a per-repository credential. With roughly 190 open pull
requests across just
fast-mlsirmandTEPPtoday, each running its ownhourly review-repair scheduler plus push-triggered Strix/OpenCode/Noema
dispatches, this installation token's hourly quota is a real, recurring
point of contention. When it is briefly exhausted, this single-shot lookup
fails the entire required Strix check immediately — with no vulnerability
report and no code defect involved — blocking otherwise fully reviewed,
fully green pull requests fleet-wide.
Fix
Retry the visibility lookup up to 6 times with linear backoff (5s × attempt)
before failing, mirroring the existing PR-head-fetch retry loop later in
this same workflow (
for pr_head_fetch_attempt in 1 2 3 4 5 6 ... sleep 10).Still fails closed with
::error::if visibility genuinely cannot beresolved after all retries. No change to Strix's scan semantics, model
routing, provider selection, or bypass posture — this only hardens a
transport-layer lookup against a transient 403.
Scope boundary
Workflow-only change to one step. Does not touch scan execution, findings
handling, merge gating, or credentials.
Verification
actionlint .github/workflows/strix.yml(embedded shellcheck): clean.scripts/ci/test_strix_quick_gate.sh:test_strix_quick_gate: PASS(exit 0), unchanged pass/fail set — the retry loop does not alter any of
the file-content assertions this suite checks.