refactor: extract Copilot API resolver from cli.ts#2636
Conversation
Extract resolveCopilotApiKey, deriveCopilotApiTargetFromProviderBaseUrl, deriveCopilotApiBasePathFromProviderBaseUrl, and resolveCopilotApiRouting into src/copilot-api-resolver.ts (84 lines). cli.ts re-exports all symbols for backwards compatibility. All 32 cli tests pass. Build passes. Closes #2624 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
✨ New Files (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR refactors Copilot BYOK/Copilot API routing helper logic out of src/cli.ts into a dedicated src/copilot-api-resolver.ts module, keeping cli.ts as the public/backwards-compatible export surface for existing consumers.
Changes:
- Added
src/copilot-api-resolver.tscontaining the 4 Copilot API resolution/derivation helpers (key, target, base path, combined routing). - Updated
src/cli.tsto import the helpers for internal use and re-export them for backwards compatibility (e.g., existingcli.test.tsimports).
Show a summary per file
| File | Description |
|---|---|
| src/copilot-api-resolver.ts | New focused module for Copilot API key/target/base-path resolution helpers. |
| src/cli.ts | Removes inlined helper implementations; imports and re-exports helpers from the new module to preserve existing import paths. |
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: 0
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PARTIAL — MCP and BYOK inference pass; pre-step template vars were not substituted so HTTP/file tests are unverifiable. PR by @lpcox · Assignees: none listed
|
Smoke Test Results✅ GitHub MCP: refactor: eliminate duplicate log source resolution logic Status: PASS
|
🧪 Smoke Test Results
PR: "refactor: extract Copilot API resolver from cli.ts" by @lpcox
|
|
🔮 Codex smoke: FAIL 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 Runtime Version Comparison
Result: ❌ Not all runtimes match — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results — PR #2636
Overall: FAIL — service containers appear unreachable from this runner environment.
|
|
@copilot address the review feedback |
I checked the PR review details and there are currently no generated review comments to address on this PR (the reviewer summary shows 0 comments). Please point me to the specific feedback you want me to apply, and I’ll make the update right away. |
Summary
Extracts 4 Copilot API helper functions from
src/cli.ts(lines 131-214) into a focusedsrc/copilot-api-resolver.tsmodule (84 lines):resolveCopilotApiKey()— resolves BYOK key from env varsderiveCopilotApiTargetFromProviderBaseUrl()— extracts hostnamederiveCopilotApiBasePathFromProviderBaseUrl()— extracts base pathresolveCopilotApiRouting()— combined target/path resolution with precedenceBackwards Compatibility
cli.tsre-exports all symbols, so existing imports (e.g.,cli.test.ts) continue to work unchanged.Verification
npm run build)Closes #2624