Skip to content

refactor: unexport internal helpers in copilot-api-resolver#3394

Merged
lpcox merged 3 commits into
mainfrom
copilot/remove-unused-exports
May 19, 2026
Merged

refactor: unexport internal helpers in copilot-api-resolver#3394
lpcox merged 3 commits into
mainfrom
copilot/remove-unused-exports

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

deriveCopilotApiTargetFromProviderBaseUrl and deriveCopilotApiBasePathFromProviderBaseUrl were exported despite being private implementation details of resolveCopilotApiRouting, widening the public API surface of a security-critical module with no production consumers outside the defining file.

Changes

  • src/copilot-api-resolver.ts: Remove export from both helper functions; expose them via the established testHelpers pattern (@internal + ts-prune-ignore-next) for test access only
  • src/cli.test.ts: Update import to destructure from testHelpers instead of direct named imports — test assertions are unchanged
// before
export function deriveCopilotApiTargetFromProviderBaseUrl(...) { ... }
export function deriveCopilotApiBasePathFromProviderBaseUrl(...) { ... }

// after
function deriveCopilotApiTargetFromProviderBaseUrl(...) { ... }
function deriveCopilotApiBasePathFromProviderBaseUrl(...) { ... }

/** @internal Exposed only for unit tests — not part of the public API. */
// ts-prune-ignore-next
export const testHelpers = {
  deriveCopilotApiTargetFromProviderBaseUrl,
  deriveCopilotApiBasePathFromProviderBaseUrl,
};

Copilot AI changed the title [WIP] Remove dead exports from copilot-api-resolver.ts refactor: unexport internal helpers in copilot-api-resolver May 19, 2026
Copilot AI requested a review from lpcox May 19, 2026 05:09
Copilot finished work on behalf of lpcox May 19, 2026 05:09
@lpcox lpcox marked this pull request as ready for review May 19, 2026 14:20
Copilot AI review requested due to automatic review settings May 19, 2026 14:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.40% 95.47% 📈 +0.07%
Statements 95.22% 95.29% 📈 +0.07%
Functions 96.55% 96.55% ➡️ +0.00%
Branches 87.87% 87.91% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions

This comment has been minimized.

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

This PR reduces the exported API surface of the Copilot API routing resolver by making two URL-derivation helpers module-private and exposing them to unit tests via the repository’s established testHelpers export pattern.

Changes:

  • Make deriveCopilotApiTargetFromProviderBaseUrl and deriveCopilotApiBasePathFromProviderBaseUrl non-exported (module-private).
  • Add export const testHelpers = { ... } (annotated @internal + ts-prune-ignore-next) to preserve unit-test access without named exports.
  • Update src/cli.test.ts to import the helpers via testHelpers.
Show a summary per file
File Description
src/copilot-api-resolver.ts Unexports internal helper functions and re-exposes them under testHelpers for tests only.
src/cli.test.ts Updates test imports to use testHelpers instead of direct named imports.

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: 1

Comment thread src/cli.test.ts Outdated
Comment on lines 26 to 29

const { deriveCopilotApiTargetFromProviderBaseUrl, deriveCopilotApiBasePathFromProviderBaseUrl } =
copilotApiResolverTestHelpers;
import { redactSecrets } from './redact-secrets';
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 19, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 19, 2026

@copilot address review feedback

Done — moved the redactSecrets import above the testHelpers destructuring so all import declarations come first. Fixed in 9e5de8c.

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

✅ GitHub API: 2 PR entries confirmed
✅ Playwright: Title contains "GitHub"
✅ File verify: smoke-test file exists

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity ✅ PR data fetched successfully
GitHub.com HTTP connectivity ⚠️ Pre-step data unavailable (template not expanded)
File write/read ⚠️ Pre-step data unavailable (template not expanded)

PR: "refactor: unexport internal helpers in copilot-api-resolver"
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PARTIAL — MCP test passed; pre-computed test data was not injected (raw ${{ }} placeholders in task input).

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Codex: FAIL
PRs: Internalize DOMAIN_CHAR_PATTERN and decouple tests from non-API regex internals; Privatize ruleset internals and test rules parsing through public API
✅ GitHub PR review
❌ SafeInputs GH CLI (safeinputs-gh missing; gh fallback used)
✅ Playwright GitHub title
❌ Tavily search (server exposes 0 tools)
✅ File write/read
✅ Discussion comment #390
✅ npm ci && npm run build
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

Smoke Test: Copilot BYOK — PASS ✅

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ (pre-step confirmed)
File write/read ✅ (pre-step confirmed)
BYOK inference (api-proxy → api.githubcopilot.com)

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

PR by @Copilot · Assignees: @lpcox, @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@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: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Gemini Engine Validation\n\n- GitHub MCP Testing: ❌\n- GitHub.com Connectivity: ✅\n- File Writing Testing: ✅\n- Bash Tool Testing: ✅\n\nOverall Status: FAIL

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

Smoke Test Results

Check Result
Redis PING ❌ No response
PostgreSQL pg_isready ❌ No response
PostgreSQL SELECT 1 ❌ No response

Overall: FAILhost.docker.internal is not reachable from this runner environment. All three service connectivity checks timed out with no response.

🔌 Service connectivity validated by Smoke Services

@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 passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit 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 #3394 · ● 6.8M ·

Copilot finished work on behalf of lpcox May 19, 2026 15:02
@lpcox lpcox merged commit 067a3ad into main May 19, 2026
66 of 68 checks passed
@lpcox lpcox deleted the copilot/remove-unused-exports branch May 19, 2026 15:09
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.

[Export Audit] Dead exports in src/copilot-api-resolver.ts: two helper functions exported but only used internally and in tests

3 participants