Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor: clean up enterprise URL handling and simplify code patterns
- Remove unused looksLikeHost() function and tests
- Remove unnecessary type casts in url.ts
- Centralize enterprise URL reading from state
- Replace typeof checks with consistent function calls
- Update GITHUB_BASE_URL and GITHUB_API_BASE_URL to read from state.enterpriseUrl
- Remove enterpriseUrl parameters from service functions (getDeviceCode, pollAccessToken)
- Update integration tests to set state.enterpriseUrl instead of passing parameters
- Improve code consistency and maintainability

All 64 tests passing. TypeScript compiles successfully.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
  • Loading branch information
jkorsvik and claude committed Oct 13, 2025
commit ee4670ab95eef8a63e922bc48bb1e92238faf20e
11 changes: 5 additions & 6 deletions src/lib/api-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"

import type { State } from "./state"

import { state } from "./state"
import { githubBaseUrl, githubApiBaseUrl } from "./url"

export const standardHeaders = () => ({
Expand All @@ -22,8 +23,8 @@ export const copilotBaseUrl = (state: State) => {
}

// Otherwise use standard GitHub Copilot endpoints based on account type
return state.accountType === "individual"
? "https://api.githubcopilot.com"
return state.accountType === "individual" ?
"https://api.githubcopilot.com"
: `https://api.${state.accountType}.githubcopilot.com`
}
Comment on lines +19 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Enterprise URL not normalized in copilotBaseUrl.

The copilotBaseUrl function directly interpolates state.enterpriseUrl without normalization. If enterpriseUrl contains a protocol prefix (e.g., https://ghe.example.com), the resulting URL will be malformed: https://copilot-api.https://ghe.example.com.

Other functions use githubBaseUrl and githubApiBaseUrl from ./url.ts which call normalizeDomain. Consider using the same normalization here.

+import { normalizeDomain, githubBaseUrl, githubApiBaseUrl } from "./url"
+
 export const copilotBaseUrl = (state: State) => {
   // If enterprise URL is configured, use enterprise Copilot API endpoint
   if (state.enterpriseUrl) {
-    return `https://copilot-api.${state.enterpriseUrl}`
+    const domain = normalizeDomain(state.enterpriseUrl)
+    return `https://copilot-api.${domain}`
   }

   // Otherwise use standard GitHub Copilot endpoints based on account type
   return state.accountType === "individual" ?
       "https://api.githubcopilot.com"
     : `https://api.${state.accountType}.githubcopilot.com`
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const copilotBaseUrl = (state: State) => {
// If enterprise URL is configured, use enterprise Copilot API endpoint
if (state.enterpriseUrl) {
return `https://copilot-api.${state.enterpriseUrl}`
}
// Otherwise use standard GitHub Copilot endpoints based on account type
return state.accountType === "individual" ?
"https://api.githubcopilot.com"
: `https://api.${state.accountType}.githubcopilot.com`
}
import { normalizeDomain, githubBaseUrl, githubApiBaseUrl } from "./url"
export const copilotBaseUrl = (state: State) => {
// If enterprise URL is configured, use enterprise Copilot API endpoint
if (state.enterpriseUrl) {
const domain = normalizeDomain(state.enterpriseUrl)
return `https://copilot-api.${domain}`
}
// Otherwise use standard GitHub Copilot endpoints based on account type
return state.accountType === "individual" ?
"https://api.githubcopilot.com"
: `https://api.${state.accountType}.githubcopilot.com`
}
🤖 Prompt for AI Agents
In src/lib/api-config.ts around lines 19 to 29, the function interpolates
state.enterpriseUrl directly which can include a protocol and produce malformed
URLs; update the function to import and call normalizeDomain (from ./url.ts) on
state.enterpriseUrl (or otherwise strip any protocol/trailing slashes) and use
the normalized domain when building the copilot API URL (e.g., const domain =
normalizeDomain(state.enterpriseUrl); return `https://copilot-api.${domain}`) so
the returned URL is always well-formed.

export const copilotHeaders = (state: State, vision: boolean = false) => {
Expand All @@ -45,8 +46,7 @@ export const copilotHeaders = (state: State, vision: boolean = false) => {
return headers
}

export const GITHUB_API_BASE_URL = (enterprise?: string) =>
githubApiBaseUrl(enterprise)
export const GITHUB_API_BASE_URL = () => githubApiBaseUrl(state.enterpriseUrl)
export const githubHeaders = (state: State) => ({
...standardHeaders(),
authorization: `token ${state.githubToken}`,
Expand All @@ -57,7 +57,6 @@ export const githubHeaders = (state: State) => ({
"x-vscode-user-agent-library-version": "electron-fetch",
})

export const GITHUB_BASE_URL = (enterprise?: string) =>
githubBaseUrl(enterprise)
export const GITHUB_BASE_URL = () => githubBaseUrl(state.enterpriseUrl)
export const GITHUB_CLIENT_ID = "Iv1.b507a08c87ecfe98"
export const GITHUB_APP_SCOPES = ["read:user"].join(" ")
4 changes: 2 additions & 2 deletions src/lib/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ export async function setupGitHubToken(
const enterpriseFromOptions = options?.enterpriseUrl
if (enterpriseFromOptions) state.enterpriseUrl = enterpriseFromOptions

const response = await getDeviceCode(state.enterpriseUrl)
const response = await getDeviceCode()
consola.debug("Device code response:", response)

consola.info(
`Please enter the code "${response.user_code}" in ${response.verification_uri}`,
)

const token = await pollAccessToken(response, state.enterpriseUrl)
const token = await pollAccessToken(response)
await writeGithubToken(token)
// persist enterprise url if set
await writeEnterpriseUrl(state.enterpriseUrl)
Expand Down
12 changes: 3 additions & 9 deletions src/lib/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,12 @@ export function normalizeDomain(url: string | undefined): string | undefined {

export function githubBaseUrl(enterprise?: string): string {
if (!enterprise) return "https://github.com"
return `https://${normalizeDomain(enterprise)}`
const domain = normalizeDomain(enterprise)
return `https://${domain}`
}

export function githubApiBaseUrl(enterprise?: string): string {
if (!enterprise) return "https://api.github.com"
const domain = normalizeDomain(enterprise) as string
const domain = normalizeDomain(enterprise)
return `https://api.${domain}`
}

// small helper to validate a normalized host (basic check)
export function looksLikeHost(s: string | undefined): boolean {
if (!s) return false
// allow example.com or sub.example.com and digits/hyphen
return /^[a-z0-9.-]+$/i.test(s)
}
14 changes: 6 additions & 8 deletions src/services/github/get-copilot-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ import { HTTPError } from "~/lib/error"
import { state } from "~/lib/state"

export const getCopilotToken = async () => {
// default path for GitHub.com
const base =
typeof GITHUB_API_BASE_URL === "function" ?
GITHUB_API_BASE_URL(state.enterpriseUrl)
: GITHUB_API_BASE_URL
const response = await fetch(`${base}/copilot_internal/v2/token`, {
headers: githubHeaders(state),
})
const response = await fetch(
`${GITHUB_API_BASE_URL()}/copilot_internal/v2/token`,
{
headers: githubHeaders(state),
},
)

if (!response.ok) throw new HTTPError("Failed to get Copilot token", response)

Expand Down
13 changes: 6 additions & 7 deletions src/services/github/get-copilot-usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@ import { HTTPError } from "~/lib/error"
import { state } from "~/lib/state"

export const getCopilotUsage = async (): Promise<CopilotUsageResponse> => {
const base =
typeof GITHUB_API_BASE_URL === "function" ?
GITHUB_API_BASE_URL(state.enterpriseUrl)
: GITHUB_API_BASE_URL
const response = await fetch(`${base}/copilot_internal/user`, {
headers: githubHeaders(state),
})
const response = await fetch(
`${GITHUB_API_BASE_URL()}/copilot_internal/user`,
{
headers: githubHeaders(state),
},
)

if (!response.ok) {
throw new HTTPError("Failed to get Copilot usage", response)
Expand Down
10 changes: 2 additions & 8 deletions src/services/github/get-device-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,8 @@ import {
} from "~/lib/api-config"
import { HTTPError } from "~/lib/error"

export async function getDeviceCode(
enterpriseUrl?: string,
): Promise<DeviceCodeResponse> {
const base =
typeof GITHUB_BASE_URL === "function" ?
GITHUB_BASE_URL(enterpriseUrl)
: GITHUB_BASE_URL
const response = await fetch(`${base}/login/device/code`, {
export async function getDeviceCode(): Promise<DeviceCodeResponse> {
const response = await fetch(`${GITHUB_BASE_URL()}/login/device/code`, {
method: "POST",
headers: standardHeaders(),
body: JSON.stringify({
Expand Down
6 changes: 1 addition & 5 deletions src/services/github/get-user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ import { HTTPError } from "~/lib/error"
import { state } from "~/lib/state"

export async function getGitHubUser() {
const base =
typeof GITHUB_API_BASE_URL === "function" ?
GITHUB_API_BASE_URL(state.enterpriseUrl)
: GITHUB_API_BASE_URL
const response = await fetch(`${base}/user`, {
const response = await fetch(`${GITHUB_API_BASE_URL()}/user`, {
headers: {
authorization: `token ${state.githubToken}`,
...standardHeaders(),
Expand Down
26 changes: 12 additions & 14 deletions src/services/github/poll-access-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,25 @@ import type { DeviceCodeResponse } from "./get-device-code"

export async function pollAccessToken(
deviceCode: DeviceCodeResponse,
enterpriseUrl?: string,
): Promise<string> {
// Interval is in seconds, we need to multiply by 1000 to get milliseconds
// I'm also adding another second, just to be safe
const sleepDuration = (deviceCode.interval + 1) * 1000
consola.debug(`Polling access token with interval of ${sleepDuration}ms`)

while (true) {
const base =
typeof GITHUB_BASE_URL === "function" ?
GITHUB_BASE_URL(enterpriseUrl)
: GITHUB_BASE_URL
const response = await fetch(`${base}/login/oauth/access_token`, {
method: "POST",
headers: standardHeaders(),
body: JSON.stringify({
client_id: GITHUB_CLIENT_ID,
device_code: deviceCode.device_code,
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
}),
})
const response = await fetch(
`${GITHUB_BASE_URL()}/login/oauth/access_token`,
{
method: "POST",
headers: standardHeaders(),
body: JSON.stringify({
client_id: GITHUB_CLIENT_ID,
device_code: deviceCode.device_code,
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
}),
},
)

if (!response.ok) {
await sleep(sleepDuration)
Expand Down
4 changes: 3 additions & 1 deletion tests/copilot-base-url.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { describe, it, expect } from "bun:test"
import { copilotBaseUrl } from "../src/lib/api-config"

import type { State } from "../src/lib/state"

import { copilotBaseUrl } from "../src/lib/api-config"

describe("copilotBaseUrl", () => {
it("should return api.githubcopilot.com for individual account without enterprise", () => {
const state: State = {
Expand Down
45 changes: 26 additions & 19 deletions tests/enterprise-integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { describe, it, expect, beforeEach, afterEach, mock } from "bun:test"
import { getDeviceCode } from "../src/services/github/get-device-code"
import { pollAccessToken } from "../src/services/github/poll-access-token"

import { state } from "../src/lib/state"
import { getCopilotToken } from "../src/services/github/get-copilot-token"
import { getCopilotUsage } from "../src/services/github/get-copilot-usage"
import { getDeviceCode } from "../src/services/github/get-device-code"
import { getGitHubUser } from "../src/services/github/get-user"
import { state } from "../src/lib/state"
import { pollAccessToken } from "../src/services/github/poll-access-token"

describe("Enterprise OAuth Integration", () => {
const originalFetch = global.fetch
const originalFetch = globalThis.fetch
let fetchCalls: Array<{ url: string; options?: any }> = []

beforeEach(() => {
Expand All @@ -18,12 +19,12 @@ describe("Enterprise OAuth Integration", () => {
})

afterEach(() => {
global.fetch = originalFetch
globalThis.fetch = originalFetch
})

describe("getDeviceCode", () => {
it("should use github.com when no enterprise URL", async () => {
global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -45,7 +46,9 @@ describe("Enterprise OAuth Integration", () => {
})

it("should use enterprise URL when provided", async () => {
global.fetch = mock((url: string, options?: any) => {
state.enterpriseUrl = "ghe.example.com"

globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -60,7 +63,7 @@ describe("Enterprise OAuth Integration", () => {
})
}) as any

await getDeviceCode("ghe.example.com")
await getDeviceCode()

expect(fetchCalls.length).toBe(1)
expect(fetchCalls[0].url).toBe(
Expand All @@ -69,7 +72,9 @@ describe("Enterprise OAuth Integration", () => {
})

it("should normalize enterprise URL with https prefix", async () => {
global.fetch = mock((url: string, options?: any) => {
state.enterpriseUrl = "https://ghe.example.com/"

globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -84,7 +89,7 @@ describe("Enterprise OAuth Integration", () => {
})
}) as any

await getDeviceCode("https://ghe.example.com/")
await getDeviceCode()

expect(fetchCalls.length).toBe(1)
expect(fetchCalls[0].url).toBe(
Expand All @@ -103,7 +108,7 @@ describe("Enterprise OAuth Integration", () => {
}

it("should use github.com when no enterprise URL", async () => {
global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -125,7 +130,9 @@ describe("Enterprise OAuth Integration", () => {
})

it("should use enterprise URL when provided", async () => {
global.fetch = mock((url: string, options?: any) => {
state.enterpriseUrl = "ghe.example.com"

globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -138,7 +145,7 @@ describe("Enterprise OAuth Integration", () => {
})
}) as any

await pollAccessToken(deviceCodeResponse, "ghe.example.com")
await pollAccessToken(deviceCodeResponse)

expect(fetchCalls.length).toBe(1)
expect(fetchCalls[0].url).toBe(
Expand All @@ -151,7 +158,7 @@ describe("Enterprise OAuth Integration", () => {
it("should use api.github.com when no enterprise URL", async () => {
state.enterpriseUrl = undefined

global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -175,7 +182,7 @@ describe("Enterprise OAuth Integration", () => {
it("should use enterprise API URL when configured", async () => {
state.enterpriseUrl = "ghe.example.com"

global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -201,7 +208,7 @@ describe("Enterprise OAuth Integration", () => {
it("should use api.github.com when no enterprise URL", async () => {
state.enterpriseUrl = undefined

global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -227,7 +234,7 @@ describe("Enterprise OAuth Integration", () => {
it("should use enterprise API URL when configured", async () => {
state.enterpriseUrl = "ghe.example.com"

global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand Down Expand Up @@ -255,7 +262,7 @@ describe("Enterprise OAuth Integration", () => {
it("should use api.github.com when no enterprise URL", async () => {
state.enterpriseUrl = undefined

global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand All @@ -275,7 +282,7 @@ describe("Enterprise OAuth Integration", () => {
it("should use enterprise API URL when configured", async () => {
state.enterpriseUrl = "ghe.example.com"

global.fetch = mock((url: string, options?: any) => {
globalThis.fetch = mock((url: string, options?: any) => {
fetchCalls.push({ url, options })
return Promise.resolve({
ok: true,
Expand Down
2 changes: 1 addition & 1 deletion tests/enterprise-persistence.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, it, expect, beforeEach, afterEach } from "bun:test"
import fs from "node:fs/promises"
import path from "node:path"
import os from "node:os"
import path from "node:path"

describe("Enterprise URL Persistence", () => {
const TEST_APP_DIR = path.join(os.tmpdir(), "copilot-api-test")
Expand Down
21 changes: 0 additions & 21 deletions tests/url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
normalizeDomain,
githubBaseUrl,
githubApiBaseUrl,
looksLikeHost,
} from "../src/lib/url"

describe("URL helpers", () => {
Expand Down Expand Up @@ -64,24 +63,4 @@ describe("URL helpers", () => {
)
})
})

describe("looksLikeHost", () => {
it("returns false for empty/undefined input", () => {
expect(looksLikeHost(undefined)).toBe(false)
expect(looksLikeHost("")).toBe(false)
})

it("returns true for valid hostnames", () => {
expect(looksLikeHost("ghe.example.com")).toBe(true)
expect(looksLikeHost("sub.ghe.example.com")).toBe(true)
expect(looksLikeHost("localhost")).toBe(true)
expect(looksLikeHost("github-enterprise.company.com")).toBe(true)
})

it("returns false for invalid hostnames", () => {
expect(looksLikeHost("invalid host with spaces")).toBe(false)
expect(looksLikeHost("http://ghe.example.com")).toBe(false)
expect(looksLikeHost("ghe.example.com/path")).toBe(false)
})
})
})