fix(api-proxy): fetch models from BYOK custom providers and fix models_url in reflect#2699
fix(api-proxy): fetch models from BYOK custom providers and fix models_url in reflect#2699
Conversation
✅ 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.
There was a problem hiding this comment.
Pull request overview
Fixes Copilot BYOK + custom provider startup model discovery in the api-proxy sidecar, and corrects the reflected models_url to include COPILOT_API_BASE_PATH so downstream tooling can fetch models from the right endpoint.
Changes:
- Updated Copilot provider adapter to fetch
/modelsat startup for custom (BYOK) targets and to reflect a base-path-awaremodels_url. - Added Jest coverage for Copilot adapter model-fetch configuration, base-path-aware reflection, and adapter-driven
fetchStartupModelscaching behavior.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/providers/copilot.js | Adds base-path-aware models_url and enables startup model fetch for BYOK custom targets via adapter config. |
| containers/api-proxy/server.test.js | Adds tests for adapter-based startup model fetching and Copilot adapter getModelsFetchConfig() / getReflectionInfo() behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 4
| // Pre-computed models path used by getModelsFetchConfig and getReflectionInfo. | ||
| // For BYOK/custom providers the base path prefix is included (e.g. /api/v1/models | ||
| // for COPILOT_PROVIDER_BASE_URL=https://openrouter.ai/api/v1). | ||
| const modelsPath = basePath ? `${basePath}/models` : '/models'; |
| getModelsFetchConfig() { | ||
| // Only COPILOT_GITHUB_TOKEN is accepted by the /models endpoint | ||
| if (!githubToken) return null; | ||
| if (!authToken) return null; | ||
|
|
||
| // Standard Copilot API (api.githubcopilot.com): | ||
| // The /models endpoint only accepts GitHub OAuth tokens (COPILOT_GITHUB_TOKEN). | ||
| // Skip startup model fetch when only a BYOK API key is configured. | ||
| if (rawTarget === 'api.githubcopilot.com') { | ||
| if (!githubToken) return null; | ||
| return { | ||
| url: `https://${rawTarget}/models`, | ||
| opts: { | ||
| method: 'GET', | ||
| headers: { | ||
| 'Authorization': `Bearer ${githubToken}`, | ||
| 'Copilot-Integration-Id': integrationId, | ||
| }, | ||
| }, | ||
| cacheKey: 'copilot', | ||
| }; | ||
| } | ||
|
|
||
| // BYOK / custom provider (e.g. OpenRouter): | ||
| // Fetch models using the BYOK auth token so that arbitrary model names | ||
| // (e.g. "minimax/minimax-m2.5:free") are cached and visible in the reflect | ||
| // response. The pre-computed modelsPath already includes the base path prefix. | ||
| return { | ||
| url: `https://${rawTarget}/models`, | ||
| url: `https://${rawTarget}${modelsPath}`, | ||
| opts: { | ||
| method: 'GET', | ||
| headers: { | ||
| 'Authorization': `Bearer ${githubToken}`, | ||
| 'Copilot-Integration-Id': integrationId, | ||
| 'Authorization': `Bearer ${authToken}`, | ||
| }, | ||
| }, |
| expect(config.opts.headers['Authorization']).toBe('Bearer sk-or-key'); | ||
| expect(config.cacheKey).toBe('copilot'); | ||
| }); | ||
|
|
| it('getModelsFetchConfig uses /models directly when basePath is not configured', () => { | ||
| // When no basePath is set, /models is used directly (no prefix) | ||
| const adapter = createCopilotAdapter({ | ||
| COPILOT_API_KEY: 'sk-custom-key', | ||
| COPILOT_API_TARGET: 'custom.llm.example.com', | ||
| }); | ||
| const config = adapter.getModelsFetchConfig(); | ||
| expect(config).not.toBeNull(); | ||
| expect(config.url).toBe('https://custom.llm.example.com/models'); | ||
| }); | ||
|
|
||
| it('getReflectionInfo includes /models for standard Copilot API (no base path)', () => { | ||
| const adapter = createCopilotAdapter({ COPILOT_GITHUB_TOKEN: 'ghu_token' }); | ||
| const info = adapter.getReflectionInfo(); | ||
| expect(info.models_url).toBe('http://api-proxy:10002/models'); | ||
| }); | ||
|
|
||
| it('getReflectionInfo includes base path in models_url for BYOK providers', () => { | ||
| const adapter = createCopilotAdapter({ | ||
| COPILOT_API_KEY: 'sk-or-key', | ||
| COPILOT_API_TARGET: 'openrouter.ai', | ||
| COPILOT_API_BASE_PATH: '/api/v1', | ||
| }); | ||
| const info = adapter.getReflectionInfo(); | ||
| expect(info.models_url).toBe('http://api-proxy:10002/api/v1/models'); | ||
| }); |
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 |
Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/27e1c51d-5ef2-419d-9c13-909ce9832e9f Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
All three review issues addressed in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
🔬 Smoke Test Results
PR: fix(api-proxy): fetch models from BYOK custom providers and fix models_url in reflect Overall: PASS (core connectivity verified)
|
🔥 Smoke Test: Copilot BYOK
Running in BYOK offline mode ( PR by Overall: PASS (core BYOK path verified ✅)
|
|
Smoke Test Results ✅ GitHub MCP: Last 2 merged PRs retrieved
✅ Playwright: Navigated to github.com, title verified Overall: PASS
|
Smoke Test✅ GitHub PR review: 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.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Chroot Version Comparison
Overall: FAILED — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results
Overall: FAIL —
|
In BYOK mode with a custom provider (e.g. OpenRouter),
cachedModels.copilotwas never populated andmodels_urlin the reflect response always pointed to/models(ignoringCOPILOT_API_BASE_PATH). This causedawf-reflect: models fetch returned 400errors from the gh-aw framework and made arbitrary model names likeminimax/minimax-m2.5:freeinvisible to the model resolver.Changes
containers/api-proxy/providers/copilot.jsgetModelsFetchConfig()— For standardapi.githubcopilot.com, behavior is unchanged (onlyCOPILOT_GITHUB_TOKENaccepted, returnsnullfor BYOK-only). For custom targets, now returns a fetch config using the BYOK auth token athttps://${rawTarget}${basePath}/models, populating the model cache at startup.getReflectionInfo()—models_urlnow includes the base path prefix. For OpenRouter (COPILOT_API_BASE_PATH=/api/v1), the reflected URL becomeshttp://api-proxy:10002/api/v1/modelsinstead ofhttp://api-proxy:10002/models.Extracted a
modelsPathclosure variable shared by both methods to avoid duplication.Example: with
COPILOT_PROVIDER_BASE_URL=https://openrouter.ai/api/v1andCOPILOT_API_KEY=<key>:containers/api-proxy/server.test.jsAdded tests covering: adapter-based
fetchStartupModelswith BYOK + custom target,getModelsFetchConfig()for all auth/target combinations, andgetReflectionInfo()base-path-awaremodels_url.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
api.example.com/opt/hostedtoolcache/node/20.20.2/x64/bin/node node /home/REDACTED/work/gh-aw-firewall/gh-aw-firewall/containers/api-proxy/node_modules/.bin/jest server.test.js(dns block)/opt/hostedtoolcache/node/20.20.2/x64/bin/node /opt/hostedtoolcache/node/20.20.2/x64/bin/node /home/REDACTED/work/gh-aw-firewall/gh-aw-firewall/containers/api-proxy/node_modules/jest-worker/build/processChild.js(dns block)If you need me to access, download, or install something from one of these locations, you can either: