fix(strix): route the openai-direct terminal fallback to OpenAI credentials - #1290
Conversation
STRIX_FALLBACK_MODELS' NVIDIA NIM entry ends in the hyphenated openai-direct/gpt-5.6-luna alias (the workflow's user-facing input spelling, also pinned verbatim by protected main's own trusted strix_required_workflow_smoke.sh, so that exact string cannot change). child_model_for_api_base() only recognized the underscored openai_direct/ form the primary-model case statement produces internally, so the fallback alias passed through unrewritten and reached LiteLLM as an unrecognized provider string. Observed three times in CI: NVIDIA NIM rate-limited the primary and first fallback model, the run advanced to the third fallback, and litellm.BadRequestError: LLM Provider NOT provided ended the scan instead of completing against direct OpenAI. Filed as a standalone fix against main rather than bundled into #1052 (whose branch already carries this same change): pull_request_target resolves job.workflow_sha to the base branch commit, so any .github PR's own "strix" check always fetches scripts/ci/strix_quick_gate.sh from protected main regardless of what the PR branch itself contains. A PR that touches this file cannot verify its own fix via that check; only a merge to main can.
…ntials Production evidence (LineageWeave#551 strix run 32678825167): with provider_mode=nvidia_nim the primary and first fallback both hit NIM 429, then the configured openai-direct/gpt-5.6-luna terminal fallback failed in 3s with litellm BadRequestError 'LLM Provider NOT provided ... You passed model=openai-direct/gpt-5.6-luna'. Two defects: - child_model_for_api_base only translated the underscore alias openai_direct/* while the workflow's fallback matrices spell the model openai-direct/*, so litellm received an unknown provider prefix. - even translated, the child would have called LLM_API_BASE_FILE (the primary provider endpoint) with the primary provider key. Fixes: - accept both spellings in child_model_for_api_base via a shared is_openai_direct_model helper. - mirror the GitHub Models cross-provider pattern: optional STRIX_OPENAI_FALLBACK_KEY_FILE / STRIX_OPENAI_FALLBACK_API_BASE_FILE switch key and endpoint for openai-direct candidates. - strix.yml writes those files from STRIX_OPENAI_API_KEY || OPENAI_API_KEY whenever nvidia_nim or openrouter is primary, and forwards them into the gate run env. test_strix_quick_gate.sh: PASS (full suite).
|
Warning Review limit reachedNext included review available in 51 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 (5)
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.
# Conflicts: # .github/workflows/strix.yml # scripts/ci/strix_quick_gate.sh
Exact-head fix and re-review requestCurrent exact head: 370250c The valid Strix finding is fixed: the direct-OpenAI fallback credential preparation step now runs for nvidia_nim, openrouter, and github_models provider modes. The remote concurrent commit 8f3ee4a also added the alias regression coverage; this head adds a workflow contract assertion so the github_models condition cannot regress. Verification on this exact head:
The unfiltered local shell suite was not accepted as green: unrelated existing pull-request-target fixture cases fail closed on unsafe backend/frontend paths. Hosted exact-head checks remain authoritative. Please review the complete changed diff and publish a substantive verdict for this exact head. The previous review was tied to bad3ebd and is stale. |
…-alias' into HEAD
Current-head re-review requestThe branch has since incorporated the concurrent main update and is now at exact head 50ad57a. The github_models direct-OpenAI fallback credential fix and its workflow contract assertion are present on this head. Please treat all earlier reviews and checks as stale, inspect the complete current diff, and publish a substantive exact-head verdict. Current hosted checks are still pending; no merge is requested until required checks and independent approval are current. |
Superseded PR auditThe current exact head of #1263 ( Disposition: superseded by #1263; continue validation and normal merge through #1263. |
|
Closed as superseded by #1263 after current-head review. |
There was a problem hiding this comment.
🟡 Call to deleted function prints error each scan
is_explicit_openai_model is still invoked at line 2571, but this PR deleted its definition, so every model attempt runs an undefined command and bash writes is_explicit_openai_model: command not found into the gate log. The branch it guarded never runs; the new is_openai_direct_model block above already sets the same key, so credentials are unchanged.
(Refers to this code)
Prompt for agents
The function is_explicit_openai_model() was removed in this PR (its former definition around old lines 406-415 was replaced by the STRIX_OPENAI_FALLBACK_API_BASE_FILE validation block), but a call to it remains at line 2571 in run_strix_once. This dangling reference makes bash emit 'is_explicit_openai_model: command not found' to stderr (captured into the gate console log) on every model attempt. The block is now redundant because the new is_openai_direct_model check at line 2561 already sets child_llm_api_key to STRIX_OPENAI_FALLBACK_KEY for the exact same set of models (openai-direct/* and openai_direct/*). Remove the entire stale if block (lines 2571-2576) so no removed function is referenced.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
📝 Info: Dead openai-direct branch left behind
The merged case openai_direct/* | openai-direct/* at line 2503 now handles both spellings, making the later standalone openai-direct/* case unreachable dead code. Harmless but worth removing with its stale comment.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Prepare direct-OpenAI fallback credentials | ||
| # The nvidia_nim, github_models, and openrouter fallback matrices all | ||
| # end with openai-direct/gpt-5.6-luna. Without OpenAI credentials those | ||
| # candidates previously ran against the primary provider endpoint with | ||
| # the wrong key and failed closed on every provider outage. | ||
| if: steps.gate.outputs.provider_mode == 'nvidia_nim' || steps.gate.outputs.provider_mode == 'openrouter' || steps.gate.outputs.provider_mode == 'github_models' | ||
| env: | ||
| OPENAI_FALLBACK_KEY: ${{ secrets.STRIX_OPENAI_API_KEY || secrets.OPENAI_API_KEY }} | ||
| run: | | ||
| umask 077 | ||
| sanitized="$(printf '%s' "$OPENAI_FALLBACK_KEY" | tr -d '\r\n')" | ||
| trimmed="$(printf '%s' "$sanitized" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | ||
| if [ -z "$trimmed" ]; then | ||
| echo '::notice::No direct-OpenAI key available; Strix scans run without the openai-direct terminal fallback.' | ||
| exit 0 | ||
| fi | ||
| openai_fallback_key_file="$RUNNER_TEMP/openai_fallback_key.txt" | ||
| printf '%s' "$trimmed" > "$openai_fallback_key_file" | ||
| echo "STRIX_OPENAI_FALLBACK_KEY_FILE=$openai_fallback_key_file" >> "$GITHUB_ENV" | ||
| openai_fallback_api_base_file="$RUNNER_TEMP/openai_fallback_api_base.txt" | ||
| printf '%s' 'https://api.openai.com/v1' > "$openai_fallback_api_base_file" | ||
| echo "STRIX_OPENAI_FALLBACK_API_BASE_FILE=$openai_fallback_api_base_file" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
📝 Info: Two steps write the same OpenAI fallback key
For nvidia_nim and openrouter modes both the existing GitHub Models fallback step and the new direct-OpenAI step write the same openai_fallback_key.txt and export STRIX_OPENAI_FALLBACK_KEY_FILE. Contents are identical, so this is harmless redundancy; only the new step adds the API-base file that constitutes the actual fix.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Problem
Production evidence: ContextualWisdomLab/LineageWeave#551 strix run 32678825167. With
provider_mode=nvidia_nim, the primary model and the first NIM fallback both hit NVIDIA 429 rate limits, then the configuredopenai-direct/gpt-5.6-lunaterminal fallback failed in 3 seconds with:Every open LineageWeave PR inherits this failure, so the required strix check blocks the whole queue whenever NVIDIA NIM throttles.
Root cause (two layers)
openai_direct/* | openai-direct/*inchild_model_for_api_base) - included here as its parent commit.LLM_API_BASE_FILE(the primary provider endpoint, e.g. integrate.api.nvidia.com) authenticated with the primary provider key. The gate only had cross-provider credential switching for github_models/* candidates.Fix
is_openai_direct_model()helper recognizing both spellings.STRIX_OPENAI_FALLBACK_KEY_FILEandSTRIX_OPENAI_FALLBACK_API_BASE_FILEswitch key and endpoint for openai-direct candidates insideresolved_llm_api_base_for_model()andrun_strix_once().STRIX_OPENAI_API_KEY || OPENAI_API_KEY, and forwards them into the gate run env.Verification
bash scripts/ci/test_strix_quick_gate.sh: PASS (full suite, 0 failures).