Skip to content

Refactor COPILOT_MODEL resolution to remove duplicated env precedence logic#3325

Merged
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-function-logic
May 18, 2026
Merged

Refactor COPILOT_MODEL resolution to remove duplicated env precedence logic#3325
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-function-logic

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

src/services/api-proxy-service.ts had two near-identical helpers implementing the same env precedence (additionalEnv ?? envFile ?? process.env) for COPILOT_MODEL. This created avoidable drift risk for future precedence changes.

  • Consolidation

    • Removed getCopilotModel(config) (duplicate implementation).
    • Kept getConfigEnvValue(config, key) as the single source of truth for config/env resolution.
  • Call site update

    • Replaced the only getCopilotModel usage in buildApiProxyService with getConfigEnvValue(config, 'COPILOT_MODEL').
  • Behavioral intent

    • No functional behavior change: lookup order and normalization remain identical, now centralized in one helper.
// before
const copilotModel = getCopilotModel(config);

// after
const copilotModel = getConfigEnvValue(config, 'COPILOT_MODEL');

Copilot AI changed the title [WIP] Fix duplicate logic in getCopilotModel function Refactor COPILOT_MODEL resolution to remove duplicated env precedence logic May 18, 2026
Copilot AI requested a review from lpcox May 18, 2026 13:55
Copilot finished work on behalf of lpcox May 18, 2026 13:55
@lpcox lpcox marked this pull request as ready for review May 18, 2026 14:49
@lpcox lpcox requested a review from Mossaka as a code owner May 18, 2026 14:49
Copilot AI review requested due to automatic review settings May 18, 2026 14:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors src/services/api-proxy-service.ts to centralize COPILOT_MODEL environment resolution logic, reducing duplication and drift risk while preserving the existing precedence order and normalization.

Changes:

  • Removed the dedicated getCopilotModel(config) helper.
  • Reused the existing getConfigEnvValue(config, key) helper for COPILOT_MODEL resolution at the single call site.
Show a summary per file
File Description
src/services/api-proxy-service.ts Removes duplicated COPILOT_MODEL resolution helper and updates the call site to use the shared config/env lookup utility.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read

Overall: PASS@Copilot

Run: 26037938747

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smoke Test Results

Test Status Details
GitHub API (gh CLI) ❌ FAIL HTTP 401 authentication error
Playwright (browser) ✅ PASS Title contains "GitHub"
File verify ✅ PASS smoke-test-claude-26037938907.txt exists

Result: 2/3 PASS — GitHub API authentication failed (pre-auth may not be configured for this context)

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🧪 Smoke Test: Copilot BYOK Offline Mode

Test Result
GitHub MCP connectivity ❌ 401 (MCP auth unavailable in sandbox)
GitHub.com HTTP ✅ (pre-step passed)
File write/read (smoke-test-copilot-byok-26037938735.txt) ✅ Verified
BYOK inference (agent → api-proxy → api.githubcopilot.com) ✅ Responding now

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com.

Overall: PASS (GitHub MCP 401 is a sandbox constraint, not a BYOK failure)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

✅ GitHub PR review: fix: resolve test failures on macOS; fix: postprocess claude-token-optimizer lock file to use local awf build
❌ Safeinputs GH CLI: safeinputs-gh unavailable
✅ Playwright: GitHub title verified
❌ Tavily search: no Tavily tools exposed
✅ File+Bash: smoke file read back
✅ Build: npm ci && npm run build
⏭ Discussion: github-discussion-query unavailable
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.15.0 v20.20.2
Go go1.22.12 go1.22.12

Overall: FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

  • Redis PING: ❌ (timeout/no response)
  • PostgreSQL pg_isready: ❌ (no response on port 5432)
  • PostgreSQL SELECT 1: ❌ (skipped, host unreachable)

Overall: FAILhost.docker.internal is not reachable from this environment. Service containers may not be running or the host alias is not resolvable.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

Gemini Engine Smoke Test: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #3325 · ● 6.6M ·

@lpcox lpcox merged commit af4286d into main May 18, 2026
68 of 75 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-function-logic branch May 18, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Duplicate Code] getCopilotModel() duplicates getConfigEnvValue() logic in api-proxy-service.ts

3 participants