fix(api-proxy): suppress model fallback for Copilot, add excludeEngines#4015
Merged
Conversation
Addresses #3987 — modelFallback was silently rewriting retired/restricted Copilot model names to middle-power fallback models, obscuring the real upstream error. Changes: - Suppress middle-power fallback for standard Copilot (non-BYOK): the Copilot CLI is authoritative for its own model catalogue, so retired/restricted models should fail fast with a clear error - Suppress for BYOK Copilot pointing at GitHub catalog targets (same rationale — catalog is authoritative) - Add modelFallback.excludeEngines config option: array of engine names for which fallback is suppressed, allowing per-engine control - Improve error messaging: emit a model_unavailable diagnostic log when Copilot returns 400 'model not supported' after retries are exhausted, instead of only the misleading 'check API key' auth error - Suppress generic upstream_auth_error log for 400s that are actually model-not-supported errors (avoids conflating auth issues with model availability) Closes #3987 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Contributor
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines the api-proxy “model fallback” behavior to avoid silently rewriting Copilot model names (which can obscure upstream “model not supported” errors), and introduces a per-engine suppression mechanism via modelFallback.excludeEngines.
Changes:
- Suppress middle-power model fallback for Copilot in “standard” mode and for Copilot requests that still target GitHub Copilot catalog hosts.
- Add
apiProxy.modelFallback.excludeEnginesto config schema/spec and apply suppression policy per provider. - Improve diagnostics by emitting a
model_unavailablelog for Copilot “model not supported” errors and suppressing misleading auth warnings in that case; update tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| src/awf-config-schema.json | Adds modelFallback.excludeEngines to the source config schema. |
| docs/awf-config.schema.json | Mirrors the schema change in the published docs schema. |
| docs/awf-config-spec.md | Documents excludeEngines and Copilot-specific suppression conditions. |
| containers/api-proxy/server.js | Parses excludeEngines and applies per-provider fallback suppression policy. |
| containers/api-proxy/providers/copilot.js | Suppresses fallback for standard Copilot and GitHub-catalog targets. |
| containers/api-proxy/upstream-response.js | Adds model_unavailable diagnostic and suppresses misleading auth warnings for model errors. |
| containers/api-proxy/server.network.test.js | Adds/updates reflect endpoint tests for suppression policies. |
| containers/api-proxy/server.models.test.js | Updates model transform tests to use a non-Copilot provider where fallback remains active. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
containers/api-proxy/upstream-response.js:76
- The 400-path still logs
upstream_auth_erroreven whenresponseBodyis unavailable (e.g. when the 400 response isn't buffered). That can reintroduce the misleading "check that the API key is valid" warning for model-not-supported 400s. Consider skipping the 400 auth warning unless the buffered body is present and confirmed to be a non-model error.
function logUpstreamAuthError(statusCode, { requestId, provider, targetHost, req, responseBody }) {
if (statusCode === 401 || statusCode === 403) {
logRequest('warn', 'upstream_auth_error', {
request_id: requestId, provider, status: statusCode,
upstream_host: targetHost, path: sanitizeForLog(req.url),
message: `Upstream returned ${statusCode} — check that the API key is valid and correctly formatted`,
});
} else if (statusCode === 400) {
// Suppress generic auth-error message when the 400 is a model-not-supported
// error — that case is handled by the model_unavailable diagnostic.
if (responseBody && parseModelNotSupportedFromBody(responseBody)) return;
logRequest('warn', 'upstream_auth_error', {
request_id: requestId, provider, status: statusCode,
upstream_host: targetHost, path: sanitizeForLog(req.url),
message: `Upstream returned ${statusCode} — check that the API key is valid and correctly formatted`,
});
}
- Files reviewed: 8/8 changed files
- Comments generated: 3
Comment on lines
+104
to
+107
| const excludeEngines = Array.isArray(parsed.excludeEngines) | ||
| ? parsed.excludeEngines.filter(e => typeof e === 'string').map(e => e.toLowerCase()) | ||
| : []; | ||
| return { enabled, strategy, excludeEngines }; |
Comment on lines
+816
to
823
| - The provider is in the `excludeEngines` list | ||
| - Copilot engine in standard mode (no BYOK env vars): the Copilot CLI is | ||
| authoritative for its own model catalogue, so retired/restricted model names | ||
| should fail fast with a clear upstream error rather than being silently | ||
| rewritten to a middle-power fallback | ||
| - Copilot is configured for a BYOK non-`githubcopilot` target (for example Azure | ||
| OpenAI deployment endpoints), where deployment names are provider-local and | ||
| must not be rewritten to catalog model IDs |
Comment on lines
+446
to
+454
| it('should suppress fallback for standard Copilot (no BYOK hints)', () => { | ||
| // Default test environment has no BYOK env vars set — standard Copilot | ||
| const result = reflectEndpoints(); | ||
| expect(result.model_fallback_effective.copilot).toEqual({ | ||
| enabled: false, | ||
| strategy: 'middle_power', | ||
| suppressed: true, | ||
| suppression_reason: 'copilot_standard_authoritative', | ||
| }); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
Author
|
@copilot address review feedback |
Contributor
Addressed in commit e584303.
Validated with: |
Contributor
Smoke Test: Claude Engine
Result: PASS
|
Contributor
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios passed. OTEL integration is functional.
|
Contributor
🔬 Smoke Test Results
PR: fix(api-proxy): suppress model fallback for Copilot, add excludeEngines Overall: PASS ✅
|
Contributor
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PARTIAL — BYOK inference and MCP confirmed ✅; pre-step smoke data vars were not injected. Author: @lpcox | No assignees.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Gemini Engine Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Contributor
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Contributor
Smoke Test Results — FAIL ❌
|
Contributor
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
This was referenced May 29, 2026
Contributor
|
fix(api-proxy): suppress model fallback for Copilot, add excludeEngines Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses #3987 —
modelFallbackwas silently rewriting retired/restricted Copilot model names to middle-power fallback models, obscuring the real upstream error with generic auth-failure diagnostics.Changes
1. Suppress middle-power fallback for standard Copilot
When no BYOK env vars are set, the Copilot CLI is authoritative for its own model catalogue. Retired or restricted models should fail fast with a clear upstream error instead of being silently rewritten to a random middle-tier model.
Previously: standard Copilot → fallback enabled → model rewritten → confusing error or wrong model
Now: standard Copilot → fallback suppressed → request goes through with original model → clear
400 model not supportederror2. Suppress for BYOK Copilot targeting GitHub catalog
When BYOK hints are present but the target is still a GitHub Copilot catalog host (
api.githubcopilot.com, etc.), the same logic applies — catalog is authoritative.3. Add
modelFallback.excludeEnginesconfig optionNew config field: array of engine/provider names for which middle-power fallback is suppressed. This gives gh-aw authors explicit control to disable fallback per-engine without disabling it globally.
{ "apiProxy": { "modelFallback": { "enabled": true, "strategy": "middle_power", "excludeEngines": ["copilot", "anthropic"] } } }4. Improved error messaging
model_unavailablediagnostic log: Emitted when Copilot returns400 model not supportedafter retries are exhausted. Provides actionable guidance instead of generic auth-error message.upstream_auth_errorwarning ("check that the API key is valid") is no longer emitted for 400 responses that are actually model-not-supported errors.Files Changed
containers/api-proxy/providers/copilot.js— Suppress fallback for standard Copilot + BYOK catalog targetscontainers/api-proxy/server.js— ParseexcludeEngines, apply per-provider policycontainers/api-proxy/upstream-response.js— Addmodel_unavailablediagnostic, suppress auth-error for model errorssrc/awf-config-schema.json+docs/awf-config.schema.json— AddexcludeEnginesfield to schemadocs/awf-config-spec.md— Document new field and suppression conditionscontainers/api-proxy/server.network.test.js— Tests for suppression policiescontainers/api-proxy/server.models.test.js— Updated model transform tests to use non-copilot providerTesting
server.custom-auth-header.test.json main)excludeEnginesconfig and standard-Copilot suppression