Pass custom-provider models through verbatim for OPENAI_BASE_URL/ANTHROPIC_BASE_URL endpoints - #54363
Conversation
…ic endpoints Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (95 additions detected, threshold is 100).
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
❌ Ponytail Reviewer failed. Please review the logs for details. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
Non-blocking: I didn't find a changed-line correctness or security issue strong enough to block this PR.
Highlights
- The new default is scoped behind explicit custom OpenAI/Anthropic base URLs and still yields to an explicit
sandbox.agent.model-fallbacksetting. - The added tests cover both provider families plus the override path.
- The docs now explain the behavior and the escape hatch, which matches the implementation.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 4.55 AIC · ⌖ 6.76 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
Clean implementation. The else if guard ensures explicit sandbox.agent.model-fallback always wins over the auto-disable logic. hasCustomLLMAPITarget is correctly scoped to OPENAI_BASE_URL/ANTHROPIC_BASE_URL. Three test cases cover Anthropic URL, OpenAI URL, and explicit override. Documentation is accurate and cross-linked. No blocking issues.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 15.7 AIC · ⌖ 8.78 AIC · ⊞ 5.7K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — one edge case gap identified; requesting a minor change.
📋 Key Themes & Highlights
Key Themes
- Expression-valued URL gap:
hasCustomLLMAPITargetcallsextractAPITargetHost, which returns""for${{ ... }}values. A workflow withANTHROPIC_BASE_URL: ${{ vars.LLM_URL }}will not get the auto-disable, silently leaving model rewriting enabled and likely reproducing the original HTTP 404.
Positive Highlights
- ✅ Clean, minimal
hasCustomLLMAPITargethelper — well-placed in the precedence chain - ✅ Three subtests cover the main positive cases and explicit-override precedence
- ✅ Documentation is accurate and cross-linked correctly; the OpenRouter example is immediately actionable
- ✅
sandbox.mdsection fills a real gap (previously the field was only reachable via the Azure BYOK guide) - ✅ Recompile of 286 workflows with no lock-file changes is a strong regression guard
The single inline comment covers the expression-URL gap. If the fix is to check key presence instead of extractAPITargetHost, a small follow-up test documents the new behaviour.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 34.6 AIC · ⌖ 10.1 AIC · ⊞ 7.8K
Comment /matt to run again
| } | ||
|
|
||
| jsonStr, err := BuildAWFConfigJSON(config) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
[/tdd] Missing edge case: expression-valued ANTHROPIC_BASE_URL/OPENAI_BASE_URL (e.g. ${{ vars.LLM_URL }}) will silently skip the auto-disable because extractAPITargetHost returns "" for expression values, leaving hasCustomLLMAPITarget returning false.
💡 Suggested test and fix
Add a test to document the gap:
t.Run("model-fallback is NOT auto-disabled for expression-valued custom API target", func(t *testing.T) {
config := AWFCommandConfig{
EngineName: "claude",
WorkflowData: &WorkflowData{
EngineConfig: &EngineConfig{
ID: "claude",
Env: map[string]string{
"ANTHROPIC_BASE_URL": "${{ vars.LLM_URL }}",
},
},
NetworkPermissions: &NetworkPermissions{Firewall: &FirewallConfig{Enabled: true}},
},
}
jsonStr, err := BuildAWFConfigJSON(config)
require.NoError(t, err)
assert.NotContains(t, jsonStr, `"modelFallback"`)
})If the intended behaviour for expression-valued URLs is also to disable fallback (which seems correct — the user is clearly targeting a custom provider), hasCustomLLMAPITarget should check for key presence rather than a non-empty extractAPITargetHost return:
func hasCustomLLMAPITarget(workflowData *WorkflowData) bool {
env := getEngineEnvOverrides(workflowData)
if env == nil {
return false
}
for _, envVar := range []string{"OPENAI_BASE_URL", "ANTHROPIC_BASE_URL"} {
if val, ok := env[envVar]; ok && val != "" {
return true
}
}
return false
}This would correctly handle ANTHROPIC_BASE_URL: ${{ vars.LLM_URL }}.
@copilot please address this.
There was a problem hiding this comment.
Fixed in 37ba5e4. hasCustomLLMAPITarget now treats non-empty OPENAI_BASE_URL/ANTHROPIC_BASE_URL env values (including expressions like ${{ vars.LLM_URL }}) as custom targets, and I added a regression test that verifies modelFallback auto-disables for expression-valued custom API targets.
There was a problem hiding this comment.
Pull request overview
Disables AWF model fallback automatically for custom OpenAI- and Anthropic-compatible endpoints, preserving provider-specific model identifiers.
Changes:
- Detects custom LLM API targets and emits
modelFallback.enabled: false. - Adds coverage for both providers and explicit overrides.
- Documents automatic behavior and manual controls.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/awf_config.go |
Implements custom-endpoint detection and fallback defaults. |
pkg/workflow/awf_config_test.go |
Tests endpoint detection and override precedence. |
docs/src/content/docs/reference/engines.md |
Documents custom-provider model handling. |
docs/src/content/docs/reference/sandbox.md |
Documents model fallback configuration. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Balanced
🔍 PR TriageCategory: Score: 56/100 (impact 25/50 + urgency 15/30 + quality 16/20) Recommended action: Focused fix with docs+tests, CI passing, addition-only diff. Automated triage — run 32432526976
|
|
@copilot Quick triage nudge for PR #54363.
Run: https://github.com/github/gh-aw/actions/runs/32434686968
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in 37ba5e4:
|
|
🎉 This pull request is included in a new release. Release: |
Routing an engine to a custom OpenAI/Anthropic-compatible provider (e.g.
claude+ OpenRouter viaengine.env.ANTHROPIC_BASE_URL) fails with HTTP 404model_not_found: the AWF API proxy resolves the requested model against its built-in catalog and rewrites it, and provider-specific slugs likeanthropic/claude-sonnet-5are not in that catalog. The existing workaround —sandbox.agent.model-fallback: false— was only documented for BYOK Azure OpenAI and had no reference section of its own.Changes
pkg/workflow/awf_config.go— newhasCustomLLMAPITargethelper detectsOPENAI_BASE_URL/ANTHROPIC_BASE_URLinengine.env. When present andsandbox.agent.model-fallbackis unset, the compiler emitsapiProxy.modelFallback: {"enabled": false}so the configured model reaches the provider unmodified. Explicit frontmatter (literal or expression) continues to take precedence.pkg/workflow/awf_config_test.go— subtests for the Anthropic target, the OpenAI target, and explicit-override precedence.docs/reference/engines.md— documents the pass-through default under "Custom API Endpoints via Environment Variables", adds the OpenRouter example, and points attoken-steering: falseas the secondary escape hatch.docs/reference/sandbox.md— adds the missingsandbox.agent.model-fallbacksection (previously referenced only from the Azure BYOK guide).This workflow now compiles to
"modelFallback":{"enabled":false}in the generatedawf-config:Recompiling the 286 workflows in this repository produced no lock-file changes, so no existing workflow is affected.