fix(api-proxy): route GPT-5 family models to /responses regardless of auth path#3177
Conversation
… auth path - In api-proxy-service.ts, decouple COPILOT_PROVIDER_WIRE_API=responses from the copilotApiKey branch: it is now set based solely on the detected model via requiresResponsesWireApi() whenever any Copilot credential is configured (copilotGithubToken OR copilotApiKey). This fixes the 400 error reported when using gpt-5.4-mini with COPILOT_GITHUB_TOKEN in strict AWF mode. - In copilot.js, introduce resolveApiKey() which filters out the AWF placeholder token (ghu_aaa...) so the sidecar never treats it as a real BYOK credential. createCopilotAdapter now uses resolveApiKey() for the apiKey variable so that when AWF injects the placeholder as COPILOT_API_KEY, the sidecar falls back to COPILOT_GITHUB_TOKEN as the sole auth source and BYOK-specific code paths (e.g. model fetching from custom providers) are not triggered. - Add tests covering the GitHub token wire-API path (gpt-5, o3-mini, gpt-5.4-mini) and the placeholder-key skipping behavior in the sidecar adapter. Fixes: api-proxy: gpt-5.4-mini routed to /chat/completions instead of /responses
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Fixes Copilot API proxy wiring so GPT-5/O3-family models reliably use the /responses wire API even when the workflow is authenticated via COPILOT_GITHUB_TOKEN (OAuth path), and prevents the api-proxy sidecar from treating the AWF placeholder sentinel as a real BYOK key.
Changes:
- Set
COPILOT_PROVIDER_WIRE_API=responsesbased onCOPILOT_MODELwhenever any Copilot credential is configured (GitHub token or API key), not only in BYOK mode. - Add
resolveApiKey()in the Copilot provider adapter to treat the AWF placeholderCOPILOT_API_KEYvalue as “absent”, avoiding incorrect BYOK-only behavior. - Add/extend unit tests covering the above behaviors for GitHub-token mode and placeholder handling in the adapter.
Show a summary per file
| File | Description |
|---|---|
src/services/api-proxy-service.ts |
Moves responses-wire-api selection to the shared Copilot-credential path so GPT-5/O3 models route correctly regardless of auth mode. |
src/services/api-proxy-service.test.ts |
Adds test coverage ensuring COPILOT_PROVIDER_WIRE_API=responses is set (or not set) in GitHub token mode based on model. |
containers/api-proxy/server.auth.test.js |
Adds tests for placeholder-aware API key resolution and adapter enablement/auth-header behavior. |
containers/api-proxy/providers/copilot.js |
Introduces resolveApiKey() to ignore the placeholder sentinel and exports it for tests. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
containers/api-proxy/server.auth.test.js:271
- Test description refers to "no GITHUB_TOKEN", but the adapter enablement logic here depends on COPILOT_GITHUB_TOKEN. Updating the wording to COPILOT_GITHUB_TOKEN would make the test intent accurate and easier to understand.
it('is disabled when COPILOT_API_KEY is the AWF placeholder and no GITHUB_TOKEN is set', () => {
const adapter = createCopilotAdapter({ COPILOT_API_KEY: COPILOT_PLACEHOLDER_TOKEN });
expect(adapter.isEnabled()).toBe(false);
});
containers/api-proxy/server.auth.test.js:279
- This test case description mentions "GITHUB_TOKEN", but the input variable that enables the adapter in this scenario is COPILOT_GITHUB_TOKEN. Consider renaming the test to avoid ambiguity with the standard GitHub Actions GITHUB_TOKEN.
it('is enabled when COPILOT_API_KEY is the AWF placeholder but COPILOT_GITHUB_TOKEN is set', () => {
const adapter = createCopilotAdapter({
COPILOT_GITHUB_TOKEN: 'gho_real_token',
COPILOT_API_KEY: COPILOT_PLACEHOLDER_TOKEN,
});
expect(adapter.isEnabled()).toBe(true);
});
- Files reviewed: 4/4 changed files
- Comments generated: 2
| // AWF injects this sentinel value into the agent environment for credential isolation. | ||
| // The ghu_ prefix is intentional: it matches the GitHub token shape that Copilot CLI | ||
| // auth pre-checks expect, but the 36 repeated 'a' characters make it unambiguous as | ||
| // a non-real placeholder. It is defined in src/constants/placeholders.ts and must | ||
| // stay in sync. When this value appears as COPILOT_API_KEY in the sidecar environment |
| })).toBe('gho_abc123'); | ||
| }); | ||
|
|
||
| it('treats AWF placeholder COPILOT_API_KEY as absent when no GITHUB_TOKEN is set', () => { |
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.
|
@copilot address the review feedback |
This comment has been minimized.
This comment has been minimized.
…ix test descriptions - Add a test in placeholders.test.ts asserting that the hardcoded COPILOT_PLACEHOLDER_TOKEN in copilot.js matches the canonical TS constant, mirroring the existing health-check shell script consistency check. - Rename three test cases in server.auth.test.js from "no GITHUB_TOKEN" to "no COPILOT_GITHUB_TOKEN" to match the actual environment variable name.
Both issues addressed in the latest commit:
|
Smoke Test Results❌ GitHub API - Result: FAIL (2/3 tests passed — GitHub API auth issue)
|
Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: FAIL — GitHub MCP returned 401 and workflow template variables were not expanded, preventing full validation.
|
🔬 Smoke Test Results
Overall: FAIL The pre-computed step outputs ( /cc
|
Smoke Test CodexGitHub PR review ✅ Refactor option parser utilities into domain-focused modules with compatibility wrappers; Refactor API proxy request path into focused guard and transport modules 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.
|
Chroot Smoke Test Results
Result: FAILED — Python and Node.js versions differ between host and chroot.
|
Gemini Smoke Test Results - PASSWarning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results — FAIL
Overall: FAIL —
|
Bug Fix
What was the bug?
Workflows using
engine.id: copilotwith a GPT-5/O3-family model (e.g.gpt-5.4-mini) and standard GitHub OAuth auth (COPILOT_GITHUB_TOKEN) failed with400 model "gpt-5.4-mini" is not accessible via the /chat/completions endpoint. TheCOPILOT_PROVIDER_WIRE_API=responsesagent env var was gated entirely inside theif (config.copilotApiKey)branch, so the Copilot CLI defaulted to/chat/completionswhenever onlycopilotGithubTokenwas provided.A secondary issue: when
COPILOT_API_KEYin the sidecar environment is the AWF placeholder sentinel (ghu_aaa…), the adapter'sapiKeyvariable was truthy, incorrectly activating BYOK-specific code paths (e.g. model fetching from custom providers with the dummy key).How did you fix it?
src/services/api-proxy-service.ts— Moved therequiresResponsesWireApi()check out of thecopilotApiKey-only branch into the sharedcopilotGithubToken || copilotApiKeyblock. The wire API is now model-driven, not auth-path-driven:containers/api-proxy/providers/copilot.js— AddedresolveApiKey()which returnsundefinedwhenCOPILOT_API_KEYequals the known AWF placeholder constant, preventing the sidecar from treating it as a real BYOK credential.createCopilotAdapternow usesresolveApiKey()in place of the rawstripBearerPrefixcall forapiKey.Testing
api-proxy-service.test.tscoverCOPILOT_PROVIDER_WIRE_API=responsesbeing set forgpt-5,gpt-5.4-mini,o3-mini, etc. when onlycopilotGithubTokenis provided.server.auth.test.jscoverresolveApiKey()returningundefinedfor the placeholder and the adapter correctly falling back toCOPILOT_GITHUB_TOKENwhenCOPILOT_API_KEYis the sentinel.