Improve GitHub Copilot provider: official OAuth onboarding, Copilot API routing, and test hardening and auto refresh token logic#288
Conversation
a04df6b to
bdba089
Compare
bdba089 to
90c56be
Compare
|
When will it be shipped in OpenClaude? currently it not working on v0.17 |
|
@Vasanthdev2004 @gnanam1990 can you check it ?? |
@igsoul now let's see how it goes. |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
I like the direction here, but I found two blocker-level regressions before I�d be comfortable merging it.
-
cli.tsxnow has a forked copy of provider validation logic that regresses the recently-merged Gemini non-key auth support.
Insrc/entrypoints/cli.tsx, the PR reintroduces localgetProviderValidationError()/validateProviderEnvOrExit()logic that only acceptsGEMINI_API_KEY/GOOGLE_API_KEY. Currentmainalready moved that logic intosrc/utils/providerValidation.ts, where Gemini access-token and ADC auth are supported. I reproduced this directly on the PR branch:CLAUDE_CODE_USE_GEMINI=1+GEMINI_ACCESS_TOKEN=dummy-tokenexits immediately withGEMINI_API_KEY is required when CLAUDE_CODE_USE_GEMINI=1., while the current shared providerValidation helper returnsnullfor the same env. -
/onboard-githubleaves stale OpenAI-compatible endpoint/key settings behind when switching a user from another OpenAI-compatible provider to GitHub Copilot.
mergeUserSettingsEnv()insrc/commands/onboard-github/onboard-github.tsxsetsCLAUDE_CODE_USE_GITHUB=1andOPENAI_MODEL, and it clears provider-selection flags, but it does not clearOPENAI_BASE_URLorOPENAI_API_KEY.updateSettingsForSource()is a merge, not a replace, so those stale values survive. I reproduced this with a syntheticsettings.jsoncontaining an old DeepSeek/OpenAI-compatible config: after the onboard merge, the resulting settings still hadOPENAI_BASE_URL=https://api.deepseek.com/v1andOPENAI_API_KEY=sk-oldalongsideCLAUDE_CODE_USE_GITHUB=1. That means GitHub Copilot onboarding can silently inherit a previous provider base URL/key and route requests to the wrong backend.
The build/smoke checks do pass, so this is not a �PR is broken to compile� issue. But these two behavior regressions are both real enough that I�d fix them before merge.
1. Gemini Non-Key Auth Regression (
|
|
@Meetpatel006 resolve conflicts |
|
I'm testing this OpenClaude PR with my GitHub Copilot subscription. A single message to resolve some lint errors cost me 16 premium requests using Claude Sonnet 4.6, and I didn't even let it finish the job. Apparently, each interaction with the OpenClaude agent is charged as a request, which I don't think is worthwhile. |
73b4469 to
a29f4cf
Compare
|
@matheusrodacki, let me check for another; it's working fine, I guess, and @Vasanthdev2004, can you check again?? |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
I like the direction here, but I still see two blocker-level regressions on the current head.
src/entrypoints/cli.tsxstill reintroduces a local copy of provider validation logic, and that copy regresses currentmain's Gemini non-key auth support.
Direct repro from this branch:
CLAUDE_CODE_USE_GEMINI=1GEMINI_ACCESS_TOKEN=dummy-token- no
GEMINI_API_KEY/GOOGLE_API_KEY
The shared src/utils/providerValidation.ts correctly returns null for that env, but the local getProviderValidationError() in cli.tsx returns GEMINI_API_KEY is required when CLAUDE_CODE_USE_GEMINI=1.
So this still rejects a flow that current main already supports.
src/commands/onboard-github/onboard-github.tsxstill leaves staleOPENAI_BASE_URL/OPENAI_API_KEYbehind when switching from a previous OpenAI-compatible provider setup.
mergeUserSettingsEnv() only clears provider-selection flags, but updateSettingsForSource() merges env maps instead of replacing them. I reproduced that directly with a synthetic prior settings.json containing:
OPENAI_BASE_URL=https://api.deepseek.com/v1OPENAI_API_KEY=deepseek-keyOPENAI_MODEL=deepseek-chat
After onboarding merge, the resulting env still contains:
OPENAI_BASE_URL=https://api.deepseek.com/v1OPENAI_API_KEY=deepseek-keyOPENAI_MODEL=github:copilotCLAUDE_CODE_USE_GITHUB=1
So the old OpenAI-compatible endpoint/key survive alongside the new Copilot mode.
I did rerun:
bun test ./src/services/github/deviceFlow.test.ts ./src/utils/githubModelsCredentials.refresh.test.ts ./src/services/api/providerConfig.github.test.ts ./src/services/api/withRetry.test.ts ./src/services/api/codexShim.test.tsbun run buildbun run smoke
Those pass, but these two regressions are still real enough that I wouldn't merge it yet.
|
@matheusrodacki Bro the issue is like , GitHub Billing Model (3rd-Party Coding Agents)
|
|
|
Rechecked the latest head. Good news first: the two blockers from my earlier review are no longer in the same state.
I still see one remaining runtime problem though:
It does not clear stale live-session values such as:
I reproduced that directly with a current-process env that started in a DeepSeek/OpenAI-compatible state. After the branch's current in-process update, So the saved profile/settings are fixed, but the current session can still stay polluted until restart. If the intended UX is strictly �restart required after onboarding�, this may be acceptable. But since the flow already hydrates the stored token and calls |
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub provider to use official Copilot OAuth device onboarding, exchanges OAuth tokens for Copilot API tokens, and improves Copilot API routing/model handling while hardening provider-related tests and startup credential validation/refresh.
Changes:
- Switched GitHub onboarding to Copilot’s OAuth device flow + OAuth→Copilot token exchange, and added startup auto-refresh using the stored OAuth token.
- Updated GitHub defaults/UX to target
api.githubcopilot.com, added Copilot-specific headers, and expanded model normalization + routing (including/responseswhere required). - Hardened tests by isolating environment variables and adding coverage for token exchange/refresh and GitHub routing.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/providerValidation.ts | Adds GitHub token expiry/format detection and more actionable onboarding error messages. |
| src/utils/providerFlag.test.ts | Improves env isolation to reduce cross-test leakage/flakes. |
| src/utils/model/modelOptions.ts | Adds GitHub provider model picker options sourced from a Copilot model registry. |
| src/utils/model/model.ts | Sets GitHub provider defaults to github:copilot across default-model helpers. |
| src/utils/model/copilotModels.ts | Introduces a hardcoded Copilot model registry used for model selection UI. |
| src/utils/model/configs.ts | Extends model configs to include github and codex provider mappings. |
| src/utils/githubModelsCredentials.ts | Stores OAuth + Copilot tokens, and adds startup auto-refresh via OAuth exchange. |
| src/utils/githubModelsCredentials.refresh.test.ts | Adds tests for GitHub token auto-refresh behavior. |
| src/services/github/deviceFlow.ts | Updates device-flow client/scope and adds OAuth→Copilot token exchange. |
| src/services/github/deviceFlow.test.ts | Adds tests for Copilot token exchange parsing/error handling. |
| src/services/api/withRetry.test.ts | Improves env cleanup/isolation in retry tests. |
| src/services/api/providerConfig.ts | Updates GitHub model normalization and routes certain GitHub models to responses transport. |
| src/services/api/providerConfig.github.test.ts | Expands tests for GitHub normalization and transport routing. |
| src/services/api/openaiShim.ts | Switches GitHub base URL/headers and adds GitHub-specific /responses fallback handling. |
| src/services/api/codexShim.test.ts | Restores env vars after tests to avoid provider cross-contamination. |
| src/entrypoints/cli.tsx | Runs GitHub token refresh on startup before hydration. |
| src/components/StartupScreen.ts | Updates provider display/base URL for “GitHub Copilot”. |
| src/commands/onboard-github/onboard-github.tsx | Removes PAT onboarding, implements OAuth→Copilot exchange, and cleans provider-specific env state. |
| scripts/system-check.ts | Updates GitHub base URL to api.githubcopilot.com. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const transport: ProviderTransport = | ||
| isCodexBaseUrl(rawBaseUrl) || (!rawBaseUrl && isCodexAlias(requestedModel)) | ||
| isCodexBaseUrl(rawBaseUrl) || | ||
| (!rawBaseUrl && isCodexAlias(requestedModel)) || | ||
| (isGithubMode && shouldUseGithubResponsesApi(githubResolvedModel)) | ||
| ? 'codex_responses' |
There was a problem hiding this comment.
resolveProviderRequest sets transport to codex_responses for GitHub models that need /responses, but the function’s default base URL for codex_responses is DEFAULT_CODEX_BASE_URL when OPENAI_BASE_URL isn’t set. That can route GitHub Copilot GPT‑5/Codex traffic to https://chatgpt.com/backend-api/codex instead of https://api.githubcopilot.com. Suggest adding a GitHub-specific default base URL (or a distinct transport) so GitHub mode uses the Copilot base regardless of transport when no explicit base URL is provided.
| if (isGithub && response.status === 400) { | ||
| const errorBody = await response.text().catch(() => '') | ||
| if (errorBody.includes('/chat/completions') || errorBody.includes('not accessible')) { | ||
| const responsesUrl = `${request.baseUrl}/responses` |
There was a problem hiding this comment.
In the GitHub 400-handling path, the response body is read via await response.text() to decide whether to retry via /responses, but if the fallback condition isn’t met the code later calls response.text() again when building the generic APIError. Since the body stream can only be consumed once, this can result in empty/incorrect error messages for GitHub 400s. Consider reading the body once (or using response.clone()) and reusing the same string for both the fallback check and the final error construction.
| type GithubTokenStatus = 'valid' | 'expired' | 'invalid_format' | ||
|
|
||
| function checkGithubTokenStatus(token: string): GithubTokenStatus { | ||
| const expMatch = token.match(/exp=(\d+)/) | ||
| if (expMatch) { |
There was a problem hiding this comment.
checkGithubTokenStatus is duplicated here and in src/utils/githubModelsCredentials.ts. Keeping two copies of token-parsing logic increases the chance they drift (e.g., if Copilot token formats change). Consider extracting this into a shared utility (e.g., src/utils/githubToken.ts) and reusing it in both validation and refresh logic.
| // @[MODEL LAUNCH]: Update the model picker lists below to include/reorder options for the new model. | ||
| // Each user tier (ant, Max/Team Premium, Pro/Team Standard/Enterprise, PAYG 1P, PAYG 3P) has its own list. | ||
|
|
||
| import { getAllCopilotModels } from './copilotModels.js' | ||
|
|
There was a problem hiding this comment.
There’s a new static import (getAllCopilotModels) added mid-file. While valid ESM, this file already has a large import block at the top (and a Biome ignore for import ordering), so leaving imports scattered makes the module harder to scan and can complicate tooling. Consider moving this import into the top import section with the others.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/services/api/openaiShim.ts:1106
- In the GitHub 400-error handling path, the response body is consumed via
await response.text()inside the/chat/completionsfallback block, and thenawait response.text()is called again later when building the thrownAPIError. Since a Response body can only be read once, the second read will likely be empty, losing the original error details when the fallback condition does not match.
Suggested fix: read the body once and reuse it for both the fallback check and the error construction (or clone the Response before reading).
// If GitHub Copilot returns error about /chat/completions,
// try the /responses endpoint (needed for GPT-5+ models)
if (isGithub && response.status === 400) {
const errorBody = await response.text().catch(() => '')
if (errorBody.includes('/chat/completions') || errorBody.includes('not accessible')) {
const responsesUrl = `${request.baseUrl}/responses`
const responsesBody: Record<string, unknown> = {
model: request.resolvedModel,
input: convertAnthropicMessagesToResponsesInput(
params.messages as Array<{
role?: string
message?: { role?: string; content?: unknown }
content?: unknown
}>,
),
stream: params.stream ?? false,
}
if (!Array.isArray(responsesBody.input) || responsesBody.input.length === 0) {
responsesBody.input = [
{
type: 'message',
role: 'user',
content: [{ type: 'input_text', text: '' }],
},
]
}
const systemText = convertSystemPrompt(params.system)
if (systemText) {
responsesBody.instructions = systemText
}
if (body.max_tokens !== undefined) {
responsesBody.max_output_tokens = body.max_tokens
}
if (params.tools && params.tools.length > 0) {
const convertedTools = convertToolsToResponsesTools(
params.tools as Array<{
name?: string
description?: string
input_schema?: Record<string, unknown>
}>,
)
if (convertedTools.length > 0) {
responsesBody.tools = convertedTools
}
}
const responsesResponse = await fetch(responsesUrl, {
method: 'POST',
headers,
body: JSON.stringify(responsesBody),
signal: options?.signal,
})
if (responsesResponse.ok) {
return responsesResponse
}
const responsesErrorBody = await responsesResponse.text().catch(() => 'unknown error')
let responsesErrorResponse: object | undefined
try { responsesErrorResponse = JSON.parse(responsesErrorBody) } catch { /* raw text */ }
throw APIError.generate(
responsesResponse.status,
responsesErrorResponse,
`OpenAI API error ${responsesResponse.status}: ${responsesErrorBody}`,
responsesResponse.headers as unknown as Record<string, string>,
)
}
}
const errorBody = await response.text().catch(() => 'unknown error')
const rateHint =
isGithub && response.status === 429 ? formatRetryAfterHint(response) : ''
let errorResponse: object | undefined
try { errorResponse = JSON.parse(errorBody) } catch { /* raw text */ }
throw APIError.generate(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const githubResolvedModel = isGithubMode | ||
| ? normalizeGithubModelsApiModel(requestedModel) | ||
| : requestedModel | ||
| // Use Codex transport only when: | ||
| // - the base URL is explicitly the Codex endpoint, OR | ||
| // - the model is a Codex alias AND no custom base URL has been set | ||
| // A custom OPENAI_BASE_URL (e.g. Azure, OpenRouter) always wins over | ||
| // model-name-based Codex detection to prevent auth failures (#200, #203). | ||
| const transport: ProviderTransport = | ||
| isCodexBaseUrl(rawBaseUrl) || (!rawBaseUrl && isCodexAlias(requestedModel)) | ||
| isCodexBaseUrl(rawBaseUrl) || | ||
| (!rawBaseUrl && isCodexAlias(requestedModel)) || | ||
| (isGithubMode && shouldUseGithubResponsesApi(githubResolvedModel)) | ||
| ? 'codex_responses' | ||
| : 'chat_completions' | ||
|
|
||
| const resolvedModel = | ||
| transport === 'chat_completions' && | ||
| isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB) | ||
| ? normalizeGithubModelsApiModel(requestedModel) | ||
| : descriptor.baseModel |
There was a problem hiding this comment.
In GitHub mode, resolvedModel is computed from normalizeGithubModelsApiModel(requestedModel) and ignores descriptor.baseModel. This breaks Codex aliases (e.g. codexplan), where parseModelDescriptor resolves the base model to gpt-5.4, but resolvedModel will remain codexplan and be sent to the Copilot API.
Suggested fix: base resolvedModel on descriptor.baseModel (and then apply GitHub normalization to that value), rather than on requestedModel unconditionally in GitHub mode. Adding a regression test for a Codex alias under CLAUDE_CODE_USE_GITHUB=1 would help prevent this from reoccurring.
| type GithubTokenStatus = 'valid' | 'expired' | 'invalid_format' | ||
|
|
||
| function checkGithubTokenStatus(token: string): GithubTokenStatus { | ||
| const expMatch = token.match(/exp=(\d+)/) | ||
| if (expMatch) { | ||
| const expSeconds = Number(expMatch[1]) | ||
| if (!Number.isNaN(expSeconds)) { | ||
| return Date.now() >= expSeconds * 1000 ? 'expired' : 'valid' | ||
| } | ||
| } | ||
|
|
||
| const parts = token.split('.') | ||
| const looksLikeJwt = | ||
| parts.length === 3 && parts.every(part => /^[A-Za-z0-9_-]+$/.test(part)) | ||
| if (looksLikeJwt) { | ||
| try { | ||
| const normalized = parts[1].replace(/-/g, '+').replace(/_/g, '/') | ||
| const padded = normalized + '='.repeat((4 - (normalized.length % 4)) % 4) | ||
| const json = Buffer.from(padded, 'base64').toString('utf8') | ||
| const parsed = JSON.parse(json) | ||
| if (parsed && typeof parsed === 'object' && parsed.exp) { | ||
| return Date.now() >= (parsed.exp as number) * 1000 ? 'expired' : 'valid' | ||
| } | ||
| } catch { | ||
| return 'invalid_format' | ||
| } | ||
| } | ||
|
|
||
| return 'invalid_format' | ||
| } |
There was a problem hiding this comment.
checkGithubTokenStatus() is duplicated here and in src/utils/githubModelsCredentials.ts. Since both functions gate authentication/refresh behavior, this duplication risks the two drifting over time (leading to inconsistent validation vs refresh decisions).
Suggested fix: extract this token-parsing/expiry logic into a shared utility and import it from both places.
| type GithubTokenStatus = 'valid' | 'expired' | 'invalid_format' | ||
|
|
||
| function checkGithubTokenStatus(token: string): GithubTokenStatus { | ||
| const expMatch = token.match(/exp=(\d+)/) | ||
| if (expMatch) { | ||
| const expSeconds = Number(expMatch[1]) | ||
| if (!Number.isNaN(expSeconds)) { | ||
| return Date.now() >= expSeconds * 1000 ? 'expired' : 'valid' | ||
| } | ||
| } | ||
|
|
||
| const parts = token.split('.') | ||
| const looksLikeJwt = | ||
| parts.length === 3 && parts.every(part => /^[A-Za-z0-9_-]+$/.test(part)) | ||
| if (looksLikeJwt) { | ||
| try { | ||
| const normalized = parts[1].replace(/-/g, '+').replace(/_/g, '/') | ||
| const padded = normalized + '='.repeat((4 - (normalized.length % 4)) % 4) | ||
| const json = Buffer.from(padded, 'base64').toString('utf8') | ||
| const parsed = JSON.parse(json) | ||
| if (parsed && typeof parsed === 'object' && parsed.exp) { | ||
| return Date.now() >= (parsed.exp as number) * 1000 ? 'expired' : 'valid' | ||
| } | ||
| } catch { | ||
| return 'invalid_format' | ||
| } | ||
| } | ||
|
|
||
| return 'invalid_format' |
There was a problem hiding this comment.
checkGithubTokenStatus() is duplicated here and in src/utils/providerValidation.ts. Because this logic determines whether the startup refresh runs, keeping two separate implementations increases the risk of inconsistent behavior.
Suggested fix: move this helper to a shared utility (e.g. under src/utils/github/ or similar) and reuse it in both modules.
Fix Applied (lines 99–103)
Test Fix
|
|
@Meetpatel006 checking now |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Rechecked the latest head 652e77e8e5f05d1877bc85842bfa28de15012d72.
Good news first:
- The earlier duplicated Gemini validation issue in
src/entrypoints/cli.tsxis fixed. - The onboarding path in
src/commands/onboard-github/onboard-github.tsxnow clears stale provider env in both saved settings and the live process env.
I still can't approve this head because I can reproduce three blocker-level problems:
-
GitHub mode still breaks Codex aliases.
Insrc/services/api/providerConfig.ts:340,resolvedModelis derived fromnormalizeGithubModelsApiModel(requestedModel)in GitHub mode, which skipsdescriptor.baseModel. On this head, an actualcreateOpenAIShimClient()repro withCLAUDE_CODE_USE_GITHUB=1sendsPOST https://api.githubcopilot.com/responseswithmodel:codexplaninstead of the resolvedgpt-5.4. That sends an alias the Copilot API does not understand. -
scripts/system-check.tsstill crashes in GitHub mode.
scripts/system-check.ts:160andscripts/system-check.ts:438still referenceGITHUB_MODELS_DEFAULT_BASE, but the file now definesGITHUB_COPILOT_BASE. Runningbun ./scripts/system-check.ts --jsonwithCLAUDE_CODE_USE_GITHUB=1throwsReferenceError: GITHUB_MODELS_DEFAULT_BASE is not defined. -
OpenAI Haiku defaults regressed in an unrelated code path.
src/utils/model/model.ts:184now returnsgpt-4oforgetDefaultHaikuModel()when the active provider is OpenAI. Currentmainstill returnsgpt-4o-mini. I reproduced that directly on this head by settingCLAUDE_CODE_USE_OPENAI=1and callinggetDefaultHaikuModel(), which now printsgpt-4o. That's a real cost/latency regression outside the GitHub feature.
Focused verification I reran on this head:
bun test ./src/services/api/providerConfig.github.test.ts ./src/services/api/codexShim.test.ts ./src/services/github/deviceFlow.test.ts ./src/utils/githubModelsCredentials.refresh.test.ts ./src/utils/providerFlag.test.ts-> 53 passbun run build-> success
So build/tests are green, but the current head still has real behavior regressions. I wouldn't merge until those are fixed.
| const resolvedModel = | ||
| transport === 'chat_completions' && | ||
| isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB) | ||
| ? normalizeGithubModelsApiModel(requestedModel) |
There was a problem hiding this comment.
This GitHub-specific branch bypasses the normal alias resolution path. On the current head, with CLAUDE_CODE_USE_GITHUB=1, an actual createOpenAIShimClient() repro posts model: \codexplan\ to /responses here instead of the expanded gpt-5.4. We still need GitHub normalization, but the request model needs to come from the parsed descriptor so Codex aliases are resolved before sending.
| } | ||
| // OpenAI provider | ||
| if (getAPIProvider() === 'openai') { | ||
| return process.env.OPENAI_MODEL || 'gpt-4o' |
There was a problem hiding this comment.
This changes the OpenAI Haiku fallback from gpt-4o-mini on main to gpt-4o. I reproduced that directly on this head by setting CLAUDE_CODE_USE_OPENAI=1 and calling getDefaultHaikuModel(), which now returns gpt-4o. That looks like an unrelated cost/latency regression from the GitHub work.
@Vasanthdev2004 yes check this , also i am in window's its works fine for me @igsoul |
|
@igsoul, bro, can you check for |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Rechecked the latest head 8220dab1d7111fecd8e0f17386d01aafbddaebc8 against current origin/main.
The earlier Copilot-routing and model-picker blockers are fixed on this head, but I still can't approve because the new endpoint split regresses the older GitHub Models API path.
src/services/api/providerConfig.ts:372-376 now only normalizes github:copilot when the base URL resolves to the Copilot host. With CLAUDE_CODE_USE_GITHUB=1 and OPENAI_BASE_URL=https://models.github.ai/inference, the same default model now stays as the literal string github:copilot.
Direct repro on this head:
{
"transport": "chat_completions",
"requestedModel": "github:copilot",
"resolvedModel": "github:copilot",
"baseUrl": "https://models.github.ai/inference"
}That is a regression for existing users who still point GitHub mode at models.github.ai/inference and rely on the default model setting. The Models API expects a real model ID like openai/gpt-4.1 / gpt-4o, not github:copilot, so those requests will start failing with 400s from /chat/completions.
The regression comes from the new branch that preserves descriptor.baseModel for non-Copilot GitHub endpoints instead of normalizing the default alias for the Models API path.
What I rechecked on this head:
- direct code-path review of
src/services/api/providerConfig.ts - direct runtime repro of
resolveProviderRequest()withCLAUDE_CODE_USE_GITHUB=1,OPENAI_BASE_URL=https://models.github.ai/inference, andmodel=github:copilot - source review of
src/utils/model/modelOptions.ts, which now does expose the full Copilot model picker for the GitHub provider
I also attempted extra local repro commands after bun install --frozen-lockfile, but this Windows worktree still hit Bun module-resolution failures before those other scripts executed, so I am not using them as review evidence here.
|
@Vasanthdev2004 ,Changes made:
All tests pass (provider, build, smoke). The GitHub Models API now receives properly normalized model IDs like gpt-4o instead of the literal github:copilot alias. |
|
@Meetpatel006 check build fail |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Rechecked the latest head 6c4e0434556347e50c66744163cd50bd5214254a against current origin/main.
The branch does fix the earlier github:copilot default on models.github.ai, but I still can't approve because three medium regressions remain.
-
src/entrypoints/cli.tsx:99-124
GitHub credential refresh/hydration still runs beforebuildStartupEnvFromProfile()applies a saved provider profile. If the user selected GitHub through the persisted profile rather than shell env vars,refreshGithubModelsTokenIfNeeded()andhydrateGithubModelsTokenFromSecureStorage()both no-op becauseCLAUDE_CODE_USE_GITHUBis still unset at that point.getProviderValidationError(startupEnv)then sees a GitHub profile with no token and rejects the saved profile.Real scenario: user saves GitHub as their default provider, restarts, and the saved profile is ignored unless they also export GitHub env vars manually.
-
src/commands/onboard-github/onboard-github.tsx:58-67andsrc/utils/providerValidation.ts:20-53
PAT-shaped tokens are still treated as healthy GitHub credentials, but this branch switched the runtime toapi.githubcopilot.com, which expects a Copilot token produced by the OAuth exchange, not an old PAT.hasExistingGithubModelsLoginToken()returns true for any stored token, andcheckGithubTokenStatus()in validation explicitly marksghp_,ghu_,github_pat_, etc. asvalid. That means existing PAT users can hit the "already authorized" fast path, skip the new OAuth flow, and only discover the breakage later when Copilot requests fail. -
src/services/api/providerConfig.ts:301-314,372-379
The latest patch fixes the default alias onmodels.github.ai, but it still strips provider-qualified model IDs for GitHub Models/custom endpoints.Direct repro on this head with
CLAUDE_CODE_USE_GITHUB=1andOPENAI_BASE_URL=https://models.github.ai/inference:{ "defaultAlias": { "resolvedModel": "gpt-4o" }, "qualified": { "requestedModel": "openai/gpt-4.1", "resolvedModel": "gpt-4.1" } }The default alias is now fine, but
openai/gpt-4.1is still rewritten togpt-4.1. Formodels.github.aiand similar GitHub-compatible proxies,openai/gpt-4.1is the valid model ID; stripping the provider prefix will make those requests fail.
What I rechecked on this head:
- direct code-path review of
src/entrypoints/cli.tsx,src/commands/onboard-github/onboard-github.tsx,src/utils/providerValidation.ts,src/utils/githubModelsCredentials.ts, andsrc/services/api/providerConfig.ts - direct runtime repro of
resolveProviderRequest()onhttps://models.github.ai/inferenceconfirming:github:copilotnow resolves togpt-4oopenai/gpt-4.1still resolves incorrectly togpt-4.1
bun install --frozen-lockfilebun run smoke-> success- attempted
bun run build, but this Windows worktree still hit the recurring Bun module-resolution failures, so I am not using that as PR-specific evidence here
|
@Vasanthdev2004 I don't know about this, because the test failed. In my case, it passed for me. In action the issue is like '1 test failed: ' |
|
@Meetpatel006 Regarding the failing CI test — that specific test failure is not caused by this PR's changes. The failing test is:
This test lives in \src/ink/termio/osc.test.ts, which PR #288 does not touch at all. The test sets \process.platform\ to \win32\ via \Object.defineProperty\ and then verifies that \setClipboard\ calls \powershell. On CI (Linux runners), the mock timing of the async IIFE inside \copyNative\ can race, causing the \powershell\ call not to be observed before the assertion checks. This is a pre-existing flakiness issue in the test's async mock expectations when run on Linux CI — it's not introduced by any change in this branch. That said, the failing test is still a real CI gate issue and should be addressed (likely by the maintainers adding proper async flushing/await in that test), but it shouldn't block merging this PR. Separately, I still have three open review blockers on this PR's latest head (\6c4e043) that need to be addressed before I can approve:
|
… model handling for Copilot and GitHub Models
Summary of FixesThis PR resolves multiple issues related to GitHub provider integration, API routing, and configuration handling. Key Fixes1. Stale OpenAI Config Cleanup
File:
2. GitHub Credential Hydration Timing
File:
3. API Routing: Copilot vs GitHub Models
File:
4. API Key Override Handling
File:
5. GitHub Models API Headers
File:
6. PAT Token Handling
Files:
7. Model Display Names
File:
8. Model Picker Updates
File:
9. Test Updates
Result
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
src/utils/providerFlag.test.ts:35
- This test file now has two beforeEach() blocks that both delete the same env keys (ENV_KEYS vs RESET_KEYS, which are identical). This redundancy makes the setup harder to reason about and the RESET_KEYS constant appears unnecessary. Consider consolidating to a single list + single beforeEach to snapshot/delete, then restore in afterEach.
const RESET_KEYS = [
'CLAUDE_CODE_USE_OPENAI',
'CLAUDE_CODE_USE_GEMINI',
'CLAUDE_CODE_USE_GITHUB',
'CLAUDE_CODE_USE_BEDROCK',
'CLAUDE_CODE_USE_VERTEX',
scripts/system-check.ts:164
- checkGithubEnv() still reports "GitHub Models provider enabled" and claims the default is "github:copilot → openai/gpt-4.1", but this PR switches the default endpoint/model to GitHub Copilot + gpt-4o. Please update these user-facing diagnostics to match the new provider behavior so system-check output remains accurate.
const baseUrl = process.env.OPENAI_BASE_URL ?? GITHUB_COPILOT_BASE
results.push(pass('Provider mode', 'GitHub Models provider enabled.'))
const token = process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN
if (!token?.trim()) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ? GITHUB_COPILOT_BASE_URL | ||
| : (isGithubMode | ||
| ? GITHUB_COPILOT_BASE_URL | ||
| : DEFAULT_OPENAI_BASE_URL)) |
There was a problem hiding this comment.
resolveProviderRequest() no longer defaults codex_responses requests to DEFAULT_CODEX_BASE_URL when no base URL is provided. As written, Codex alias models (e.g. codexplan) will resolve to transport=codex_responses but baseUrl will fall back to the OpenAI base URL, which will break Codex backend requests unless the user explicitly sets OPENAI_BASE_URL. Please restore the DEFAULT_CODEX_BASE_URL fallback when transport is codex_responses and GitHub mode is not active (and add/adjust a test to assert baseUrl for codexplan).
| : DEFAULT_OPENAI_BASE_URL)) | |
| : (transport === 'codex_responses' | |
| ? DEFAULT_CODEX_BASE_URL | |
| : DEFAULT_OPENAI_BASE_URL))) |
| const githubResolvedModel = isGithubMode | ||
| ? normalizeGithubModelsApiModel(requestedModel) | ||
| : requestedModel | ||
|
|
||
| const transport: ProviderTransport = | ||
| shouldUseCodexTransport(requestedModel, rawBaseUrl) | ||
| shouldUseCodexTransport(requestedModel, rawBaseUrl) || | ||
| (isGithubCopilot && shouldUseGithubResponsesApi(githubResolvedModel)) | ||
| ? 'codex_responses' |
There was a problem hiding this comment.
For GitHub Copilot routing, shouldUseGithubResponsesApi() is evaluated against githubResolvedModel derived from normalizeGithubModelsApiModel(), which intentionally preserves provider-qualified prefixes (e.g. "openai/gpt-5.4"). That means models like "github:openai/gpt-5.4" won’t match the GPT-5 regex and will incorrectly stay on chat_completions even though normalizeGithubCopilotModel() later resolves them to a GPT-5 model. Consider using the Copilot-normalized model ID (prefix stripped) when deciding whether to route to /responses for Copilot endpoints.
| // Apply settings.env from user settings (includes GitHub provider settings from /onboard-github) | ||
| { | ||
| const { applySafeConfigEnvironmentVariables } = await import('../utils/managedEnv.js') | ||
| applySafeConfigEnvironmentVariables() | ||
| const { hydrateGeminiAccessTokenFromSecureStorage } = await import('../utils/geminiCredentials.js') | ||
| hydrateGeminiAccessTokenFromSecureStorage() | ||
| const { hydrateGithubModelsTokenFromSecureStorage } = await import('../utils/githubModelsCredentials.js') | ||
| hydrateGithubModelsTokenFromSecureStorage() | ||
| } |
There was a problem hiding this comment.
Gemini access-token hydration from secure storage was removed from the CLI startup path, but provider validation runs before createOpenAIShimClient() (which hydrates Gemini). This can cause validateProviderEnvOrExit() to fail for Gemini users who rely on a stored GEMINI_ACCESS_TOKEN. Suggest re-introducing hydrateGeminiAccessTokenFromSecureStorage() after settings/profile env is applied and before validateProviderEnvOrExit().
| // PATs are no longer supported - Copilot API requires OAuth-derived tokens | ||
| if (GITHUB_PAT_PREFIXES.some(prefix => token.startsWith(prefix))) { | ||
| return 'expired' |
There was a problem hiding this comment.
PATs are treated as token status "expired", which leads to the user-facing message "token has expired". Since PATs are no longer supported (not actually expired), this is misleading. Consider introducing a distinct status (e.g. "pat_not_supported") or returning invalid_format and adjusting the error text to explicitly say PATs aren’t supported and /onboard-github is required.
| // PATs are no longer supported - Copilot API requires OAuth-derived tokens | |
| if (GITHUB_PAT_PREFIXES.some(prefix => token.startsWith(prefix))) { | |
| return 'expired' | |
| // PATs are no longer supported - Copilot API requires OAuth-derived tokens, | |
| // so classify them as invalid rather than expired. | |
| if (GITHUB_PAT_PREFIXES.some(prefix => token.startsWith(prefix))) { | |
| return 'invalid_format' |
| // GitHub provider (legacy duplicate check - already handled above) | ||
| if (getAPIProvider() === 'github') { | ||
| return process.env.OPENAI_MODEL || 'github:copilot' | ||
| const settings = getSettings_DEPRECATED() || {} | ||
| return settings.model || process.env.OPENAI_MODEL || 'github:copilot' | ||
| } |
There was a problem hiding this comment.
getDefaultMainLoopModelSetting() contains a duplicate getAPIProvider()==='github' branch later in the function that is now unreachable because GitHub is handled at the top. Please remove the duplicate block to avoid confusion and reduce the chance of future changes diverging between the two branches.
| } | ||
| process.env.CLAUDE_CODE_USE_GITHUB = '1' | ||
| process.env.OPENAI_MODEL = model.trim() || DEFAULT_MODEL |
There was a problem hiding this comment.
finalize() clears various provider env vars but does not clear or overwrite an existing GITHUB_TOKEN/GH_TOKEN in the current session before calling hydrateGithubModelsTokenFromSecureStorage(). If the user is re-authing (or had a stale env token set), hydrate() will early-return and the session can keep using the old token until restart. Consider explicitly setting process.env.GITHUB_TOKEN to the newly issued Copilot token (or deleting any existing GH_TOKEN/GITHUB_TOKEN before hydrating) so onboarding takes effect immediately.
| } | |
| process.env.CLAUDE_CODE_USE_GITHUB = '1' | |
| process.env.OPENAI_MODEL = model.trim() || DEFAULT_MODEL | |
| } | |
| // Ensure any stale GitHub token env vars from the current session do | |
| // not prevent the newly saved token from taking effect immediately. | |
| delete process.env.GH_TOKEN | |
| delete process.env.GITHUB_TOKEN | |
| process.env.CLAUDE_CODE_USE_GITHUB = '1' | |
| process.env.OPENAI_MODEL = model.trim() || DEFAULT_MODEL | |
| process.env.GITHUB_TOKEN = token |
|
@Meetpatel006 This PR has been through many review rounds. Instead of another CHANGES_REQUESTED, here are the specific code issues that need fixing to merge, with concrete suggestions. 1. Dead code: duplicate GitHub branch in \getDefaultMainLoopModelSetting\\\ s The second \if (getAPIProvider() === 'github')\ block on line 263 is dead code — the first block on line 250 already returns for the \github\ provider. Delete lines 263-267. 2. PAT tokens treated as \expired\ breaks existing users\src/utils/providerValidation.ts:24: Any user with a Classic PAT (\ghp_), Fine-grained PAT (\github_pat_), or OAuth App token (\gho_, \ghu_, \ghs_, \ghr_) gets told their token is expired and directed to /onboard-github. But:
This is a breaking change for existing users using PATs with \models.github.ai. The fix depends on intent:
Suggested fix: And wire \endpoint\ from \getGithubEndpointType()\ at the call site in \getProviderValidationError. 3. Startup credential ordering in \cli.tsx\The sequence in \src/entrypoints/cli.tsx\ is:
If the user's saved profile uses GitHub Models with a custom endpoint, step 1 may set \GITHUB_TOKEN\ from the stored OAuth token, and then step 2 may overwrite \OPENAI_BASE_URL\ but not clear the stale OAuth token. On restart, the stale OAuth token from step 1 can take precedence over the profile's intended configuration. Suggested fix: call \�uildStartupEnvFromProfile()\ before the hydration steps, or clear \GITHUB_TOKEN/\GH_TOKEN\ inside \�uildStartupEnvFromProfile\ when the profile changes the provider away from GitHub. 4. Redundant .toLowerCase()\ in \getGithubEndpointType\\src/services/api/providerConfig.ts:341: 'api.githubcopilot.com'.toLowerCase()\ is identical to 'api.githubcopilot.com'\ since it's already lowercase. Remove the redundant second condition. 5. CI test failure: Windows clipboard fallbackThe failing test (\src/ink/termio/osc.test.ts) is not touched by this PR. It's a pre-existing flaky test that races async IIFE timing on Linux CI. This shouldn't block the PR. Consider adding a skip condition for non-Windows platforms: Or convert the test to use proper async awaiting. |
…ve endpoint type handling
Key Fixes1. Dead Code Removal
2. PAT Token Handling for GitHub Models
3. Redundant Logic Cleanup
4. Credential Initialization Order
Result
|
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head 6a9f89d against current origin/main.
All three blockers from the previous review are now fixed:
- Dead code in getDefaultMainLoopModelSetting — removed ✅
- PAT tokens now endpoint-aware (expired for copilot, valid for models/custom) ✅
- Startup credential ordering — hydrateGithubModelsTokenFromSecureStorage and refreshGithubModelsTokenIfNeeded now run after buildStartupEnvFromProfile ✅
- Redundant .toLowerCase() removed ✅
One minor note for follow-up (not blocking):
DEFAULT_GITHUB_MODELS_API_MODEL changed from openai/gpt-4.1 to gpt-4o. When github:copilot is used with models.github.ai, this resolves to gpt-4o without a provider prefix. The GitHub Models API does accept unqualified IDs and routes to the default provider, so this works — but if the intent is to maximize compatibility with all GitHub Models features, openai/gpt-4o would be more explicit. Low priority since the Copilot endpoint (the primary target of this PR) uses unqualified IDs.
|
@kevincodex1 one more eyes |
|
I fix this patch my EOD and give final call, this |
|
Quick status pass on the latest head (
I also ran a focused local pass on the current branch around the GitHub-specific regression areas ( At this point the branch looks mainly blocked on stale review state rather than failing checks. If anyone still sees an unresolved issue on the current head, please point to the specific file/behavior on |
|
This PR saved my day! Works wonderful! |
…PI routing, and test hardening and auto refresh token logic (Gitlawb#288) * update gitHub copilot API with offical client id and update model configurations * test: add unit tests for exchangeForCopilotToken and enhance GitHub model normalization * remove PAT token feature * test(api): harden provider tests against env leakage * Added back trimmed github auth token * added auto refresh logic for auto token along with test * fix: remove forked provider validation in cli.tsx and clear stale provider env vars in /onboard-github * refactor: streamline environment variable handling in mergeUserSettingsEnv * fix: clear stale provider env vars to ensure correct GH routing * Remove internal-only tooling from the external build (Gitlawb#352) * Remove internal-only tooling without changing external runtime contracts This trims the lowest-risk internal-only surfaces first: deleted internal modules are replaced by build-time no-op stubs, the bundled stuck skill is removed, and the insights S3 upload path now stays local-only. The privacy verifier is expanded and the remaining bundled internal Slack/Artifactory strings are neutralized without broad repo-wide renames. Constraint: Keep the first PR deletion-heavy and avoid mass rewrites of USER_TYPE, tengu, or claude_code identifiers Rejected: One-shot DMCA cleanup branch | too much semantic risk for a first PR Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Treat full-repo typecheck as a baseline issue on this upstream snapshot; do not claim this commit introduced the existing non-Phase-A errors without isolating them first Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Not-tested: Full repo typecheck (currently fails on widespread pre-existing upstream errors outside this change set) * Keep minimal source shims so CI can import Phase A cleanup paths The first PR removed internal-only source files entirely, but CI provider and context tests import those modules directly from source rather than through the build-time no-telemetry stubs. This restores tiny no-op source shims so tests and local source imports resolve while preserving the same external runtime behavior. Constraint: GitHub Actions runs source-level tests in addition to bundled build/privacy checks Rejected: Revert the entire deletion pass | unnecessary once the import contract is satisfied by small shims Confidence: high Scope-risk: narrow Reversibility: clean Directive: For later cleanup phases, treat build-time stubs and source-test imports as separate compatibility surfaces Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (still noisy on this upstream snapshot) --------- Co-authored-by: anandh8x <test@example.com> * Reduce internal-only labeling noise in source comments (Gitlawb#355) This pass rewrites comment-only ANT-ONLY markers to neutral internal-only language across the source tree without changing runtime strings, flags, commands, or protocol identifiers. The goal is to lower obvious internal prose leakage while keeping the diff mechanically safe and easy to review. Constraint: Phase B is limited to comments/prose only; runtime strings and user-facing labels remain deferred Rejected: Broad search-and-replace across strings and command descriptions | too risky for a prose-only pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Remaining ANT-ONLY hits are mostly runtime/user-facing strings and should be handled separately from comment cleanup Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Neutralize internal Anthropic prose in explanatory comments (Gitlawb#357) This is a small prose-only follow-up that rewrites clearly internal or explanatory Anthropic comment language to neutral wording in a handful of high-confidence files. It avoids runtime strings, flags, command labels, protocol identifiers, and provider-facing references. Constraint: Keep this pass narrowly scoped to comments/documentation only Rejected: Broader Anthropic comment sweep across functional API/protocol references | too ambiguous for a safe prose-only PR Confidence: high Scope-risk: narrow Reversibility: clean Directive: Leave functional Anthropic references (API behavior, SDKs, URLs, provider labels, protocol docs) for separate reviewed passes Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Neutralize remaining internal-only diagnostic labels (Gitlawb#359) This pass rewrites a small set of ant-only diagnostic and UI labels to neutral internal wording while leaving command definitions, flags, and runtime logic untouched. It focuses on internal debug output, dead UI branches, and noninteractive headings rather than broader product text. Constraint: Label cleanup only; do not change command semantics or ant-only logic gates Rejected: Renaming ant-only command descriptions in main.tsx | broader UX surface better handled in a separate reviewed pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Remaining ANT-ONLY hits are mostly command descriptions and intentionally deferred user-facing strings Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Finish eliminating remaining ANT-ONLY source labels (Gitlawb#360) This extends the label-only cleanup to the remaining internal-only command, debug, and heading strings so the source tree no longer contains ANT-ONLY markers. The pass still avoids logic changes and only renames labels shown in internal or gated surfaces. Constraint: Update the existing label-cleanup PR without widening scope into behavior changes Rejected: Leave the last ANT-ONLY strings for a later pass | low-cost cleanup while the branch is already focused on labels Confidence: high Scope-risk: narrow Reversibility: clean Directive: The next phase should move off label cleanup and onto a separately scoped logic or rebrand slice Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Stub internal-only recording and model capability helpers (Gitlawb#377) This follow-up Phase C-lite slice replaces purely internal helper modules with stable external no-op surfaces and collapses internal elevated error logging to a no-op. The change removes additional USER_TYPE-gated helper behavior without touching product-facing runtime flows. Constraint: Keep this PR limited to isolated helper modules that are already external no-ops in practice Rejected: Pulling in broader speculation or logging sink changes | less isolated and easier to debate during review Confidence: high Scope-risk: narrow Reversibility: clean Directive: Continue Phase C with similarly isolated helpers before moving into mixed behavior files Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Remove internal-only bundled skills and mock helpers (Gitlawb#376) * Remove internal-only bundled skills and mock rate-limit behavior This takes the next planned Phase C-lite slice by deleting bundled skills that only ever registered for internal users and replacing the internal mock rate-limit helper with a stable no-op external stub. The external build keeps the same behavior while removing a concentrated block of USER_TYPE-gated dead code. Constraint: Limit this PR to isolated internal-only helpers and avoid bridge, oauth, or rebrand behavior Rejected: Broad USER_TYPE cleanup across mixed runtime surfaces | too risky for the next medium-sized PR Confidence: high Scope-risk: moderate Reversibility: clean Directive: The next cleanup pass should continue with similarly isolated USER_TYPE helpers before touching main.tsx or protocol-heavy code Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) * Align internal-only helper removal with remaining user guidance This follow-up fixes the mock billing stub to be a true no-op and removes stale user-facing references to /verify and /skillify from the same PR. It also leaves a clearer paper trail for review: the deleted verify skill was explicitly ant-gated before removal, and the remaining mock helper callers still resolve to safe no-op returns in the external build. Constraint: Keep the PR focused on consistency fixes and reviewer-requested evidence, not new cleanup scope Rejected: Leave stale guidance for a later PR | would make this branch internally inconsistent after skill removal Confidence: high Scope-risk: narrow Reversibility: clean Directive: When deleting gated features, always sweep user guidance and coordinator prompts in the same pass Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy; changed-file scan still shows only pre-existing tipRegistry errors outside edited lines) * Clarify generic workflow wording after skill removal This removes the last generic verification-skill wording that could still be read as pointing at a deleted bundled command. The guidance now talks about project workflows rather than a specific bundled verify skill. Constraint: Keep the follow-up limited to reviewer-facing wording cleanup on the same PR Rejected: Leave generic wording as-is | still too easy to misread after the explicit /verify references were removed Confidence: high Scope-risk: narrow Reversibility: clean Directive: When removing bundled commands, scrub both explicit and generic references in the same branch Tested: bun run build Tested: bun run smoke Not-tested: Additional checks unchanged by wording-only follow-up --------- Co-authored-by: anandh8x <test@example.com> * test(api): add GEMINI_AUTH_MODE to environment setup in tests * test: isolate GitHub/Gemini credential tests with fresh module imports and explicit non-bare env setup to prevent cross-test mock/cache leaks * fix: update GitHub Copilot base URL and model defaults for improved compatibility * fix: enhance error handling in OpenAI API response processing * fix: improve error handling for GitHub Copilot API responses and streamline error body consumption * fix: enhance response handling in OpenAI API shim for better error reporting and support for streaming responses * feat: enhance GitHub device flow with fresh module import and token validation improvements * fix: separate Copilot API routing from GitHub Models, clear stale env vars, honor providerOverride.apiKey * fix: route GitHub GPT-5/Codex to Copilot API, show all Copilot models in picker, clear stale env vars * fix GitHub Models API regression * feat: update GitHub authentication to require OAuth tokens, normalize model handling for Copilot and GitHub Models * fix: update GitHub token validation to support OAuth tokens and improve endpoint type handling --------- Co-authored-by: Anandan <anandan.8x@gmail.com> Co-authored-by: anandh8x <test@example.com>
…PI routing, and test hardening and auto refresh token logic (Gitlawb#288) * update gitHub copilot API with offical client id and update model configurations * test: add unit tests for exchangeForCopilotToken and enhance GitHub model normalization * remove PAT token feature * test(api): harden provider tests against env leakage * Added back trimmed github auth token * added auto refresh logic for auto token along with test * fix: remove forked provider validation in cli.tsx and clear stale provider env vars in /onboard-github * refactor: streamline environment variable handling in mergeUserSettingsEnv * fix: clear stale provider env vars to ensure correct GH routing * Remove internal-only tooling from the external build (Gitlawb#352) * Remove internal-only tooling without changing external runtime contracts This trims the lowest-risk internal-only surfaces first: deleted internal modules are replaced by build-time no-op stubs, the bundled stuck skill is removed, and the insights S3 upload path now stays local-only. The privacy verifier is expanded and the remaining bundled internal Slack/Artifactory strings are neutralized without broad repo-wide renames. Constraint: Keep the first PR deletion-heavy and avoid mass rewrites of USER_TYPE, tengu, or claude_code identifiers Rejected: One-shot DMCA cleanup branch | too much semantic risk for a first PR Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Treat full-repo typecheck as a baseline issue on this upstream snapshot; do not claim this commit introduced the existing non-Phase-A errors without isolating them first Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Not-tested: Full repo typecheck (currently fails on widespread pre-existing upstream errors outside this change set) * Keep minimal source shims so CI can import Phase A cleanup paths The first PR removed internal-only source files entirely, but CI provider and context tests import those modules directly from source rather than through the build-time no-telemetry stubs. This restores tiny no-op source shims so tests and local source imports resolve while preserving the same external runtime behavior. Constraint: GitHub Actions runs source-level tests in addition to bundled build/privacy checks Rejected: Revert the entire deletion pass | unnecessary once the import contract is satisfied by small shims Confidence: high Scope-risk: narrow Reversibility: clean Directive: For later cleanup phases, treat build-time stubs and source-test imports as separate compatibility surfaces Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (still noisy on this upstream snapshot) --------- Co-authored-by: anandh8x <test@example.com> * Reduce internal-only labeling noise in source comments (Gitlawb#355) This pass rewrites comment-only ANT-ONLY markers to neutral internal-only language across the source tree without changing runtime strings, flags, commands, or protocol identifiers. The goal is to lower obvious internal prose leakage while keeping the diff mechanically safe and easy to review. Constraint: Phase B is limited to comments/prose only; runtime strings and user-facing labels remain deferred Rejected: Broad search-and-replace across strings and command descriptions | too risky for a prose-only pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Remaining ANT-ONLY hits are mostly runtime/user-facing strings and should be handled separately from comment cleanup Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Neutralize internal Anthropic prose in explanatory comments (Gitlawb#357) This is a small prose-only follow-up that rewrites clearly internal or explanatory Anthropic comment language to neutral wording in a handful of high-confidence files. It avoids runtime strings, flags, command labels, protocol identifiers, and provider-facing references. Constraint: Keep this pass narrowly scoped to comments/documentation only Rejected: Broader Anthropic comment sweep across functional API/protocol references | too ambiguous for a safe prose-only PR Confidence: high Scope-risk: narrow Reversibility: clean Directive: Leave functional Anthropic references (API behavior, SDKs, URLs, provider labels, protocol docs) for separate reviewed passes Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Neutralize remaining internal-only diagnostic labels (Gitlawb#359) This pass rewrites a small set of ant-only diagnostic and UI labels to neutral internal wording while leaving command definitions, flags, and runtime logic untouched. It focuses on internal debug output, dead UI branches, and noninteractive headings rather than broader product text. Constraint: Label cleanup only; do not change command semantics or ant-only logic gates Rejected: Renaming ant-only command descriptions in main.tsx | broader UX surface better handled in a separate reviewed pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Remaining ANT-ONLY hits are mostly command descriptions and intentionally deferred user-facing strings Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Finish eliminating remaining ANT-ONLY source labels (Gitlawb#360) This extends the label-only cleanup to the remaining internal-only command, debug, and heading strings so the source tree no longer contains ANT-ONLY markers. The pass still avoids logic changes and only renames labels shown in internal or gated surfaces. Constraint: Update the existing label-cleanup PR without widening scope into behavior changes Rejected: Leave the last ANT-ONLY strings for a later pass | low-cost cleanup while the branch is already focused on labels Confidence: high Scope-risk: narrow Reversibility: clean Directive: The next phase should move off label cleanup and onto a separately scoped logic or rebrand slice Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Stub internal-only recording and model capability helpers (Gitlawb#377) This follow-up Phase C-lite slice replaces purely internal helper modules with stable external no-op surfaces and collapses internal elevated error logging to a no-op. The change removes additional USER_TYPE-gated helper behavior without touching product-facing runtime flows. Constraint: Keep this PR limited to isolated helper modules that are already external no-ops in practice Rejected: Pulling in broader speculation or logging sink changes | less isolated and easier to debate during review Confidence: high Scope-risk: narrow Reversibility: clean Directive: Continue Phase C with similarly isolated helpers before moving into mixed behavior files Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Remove internal-only bundled skills and mock helpers (Gitlawb#376) * Remove internal-only bundled skills and mock rate-limit behavior This takes the next planned Phase C-lite slice by deleting bundled skills that only ever registered for internal users and replacing the internal mock rate-limit helper with a stable no-op external stub. The external build keeps the same behavior while removing a concentrated block of USER_TYPE-gated dead code. Constraint: Limit this PR to isolated internal-only helpers and avoid bridge, oauth, or rebrand behavior Rejected: Broad USER_TYPE cleanup across mixed runtime surfaces | too risky for the next medium-sized PR Confidence: high Scope-risk: moderate Reversibility: clean Directive: The next cleanup pass should continue with similarly isolated USER_TYPE helpers before touching main.tsx or protocol-heavy code Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) * Align internal-only helper removal with remaining user guidance This follow-up fixes the mock billing stub to be a true no-op and removes stale user-facing references to /verify and /skillify from the same PR. It also leaves a clearer paper trail for review: the deleted verify skill was explicitly ant-gated before removal, and the remaining mock helper callers still resolve to safe no-op returns in the external build. Constraint: Keep the PR focused on consistency fixes and reviewer-requested evidence, not new cleanup scope Rejected: Leave stale guidance for a later PR | would make this branch internally inconsistent after skill removal Confidence: high Scope-risk: narrow Reversibility: clean Directive: When deleting gated features, always sweep user guidance and coordinator prompts in the same pass Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy; changed-file scan still shows only pre-existing tipRegistry errors outside edited lines) * Clarify generic workflow wording after skill removal This removes the last generic verification-skill wording that could still be read as pointing at a deleted bundled command. The guidance now talks about project workflows rather than a specific bundled verify skill. Constraint: Keep the follow-up limited to reviewer-facing wording cleanup on the same PR Rejected: Leave generic wording as-is | still too easy to misread after the explicit /verify references were removed Confidence: high Scope-risk: narrow Reversibility: clean Directive: When removing bundled commands, scrub both explicit and generic references in the same branch Tested: bun run build Tested: bun run smoke Not-tested: Additional checks unchanged by wording-only follow-up --------- Co-authored-by: anandh8x <test@example.com> * test(api): add GEMINI_AUTH_MODE to environment setup in tests * test: isolate GitHub/Gemini credential tests with fresh module imports and explicit non-bare env setup to prevent cross-test mock/cache leaks * fix: update GitHub Copilot base URL and model defaults for improved compatibility * fix: enhance error handling in OpenAI API response processing * fix: improve error handling for GitHub Copilot API responses and streamline error body consumption * fix: enhance response handling in OpenAI API shim for better error reporting and support for streaming responses * feat: enhance GitHub device flow with fresh module import and token validation improvements * fix: separate Copilot API routing from GitHub Models, clear stale env vars, honor providerOverride.apiKey * fix: route GitHub GPT-5/Codex to Copilot API, show all Copilot models in picker, clear stale env vars * fix GitHub Models API regression * feat: update GitHub authentication to require OAuth tokens, normalize model handling for Copilot and GitHub Models * fix: update GitHub token validation to support OAuth tokens and improve endpoint type handling --------- Co-authored-by: Anandan <anandan.8x@gmail.com> Co-authored-by: anandh8x <test@example.com>
|
When will it be released? I'm using v0.18 and it doesn't work. |
|
Connect to owner, it will be on next version |
…PI routing, and test hardening and auto refresh token logic (Gitlawb#288) * update gitHub copilot API with offical client id and update model configurations * test: add unit tests for exchangeForCopilotToken and enhance GitHub model normalization * remove PAT token feature * test(api): harden provider tests against env leakage * Added back trimmed github auth token * added auto refresh logic for auto token along with test * fix: remove forked provider validation in cli.tsx and clear stale provider env vars in /onboard-github * refactor: streamline environment variable handling in mergeUserSettingsEnv * fix: clear stale provider env vars to ensure correct GH routing * Remove internal-only tooling from the external build (Gitlawb#352) * Remove internal-only tooling without changing external runtime contracts This trims the lowest-risk internal-only surfaces first: deleted internal modules are replaced by build-time no-op stubs, the bundled stuck skill is removed, and the insights S3 upload path now stays local-only. The privacy verifier is expanded and the remaining bundled internal Slack/Artifactory strings are neutralized without broad repo-wide renames. Constraint: Keep the first PR deletion-heavy and avoid mass rewrites of USER_TYPE, tengu, or claude_code identifiers Rejected: One-shot DMCA cleanup branch | too much semantic risk for a first PR Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Treat full-repo typecheck as a baseline issue on this upstream snapshot; do not claim this commit introduced the existing non-Phase-A errors without isolating them first Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Not-tested: Full repo typecheck (currently fails on widespread pre-existing upstream errors outside this change set) * Keep minimal source shims so CI can import Phase A cleanup paths The first PR removed internal-only source files entirely, but CI provider and context tests import those modules directly from source rather than through the build-time no-telemetry stubs. This restores tiny no-op source shims so tests and local source imports resolve while preserving the same external runtime behavior. Constraint: GitHub Actions runs source-level tests in addition to bundled build/privacy checks Rejected: Revert the entire deletion pass | unnecessary once the import contract is satisfied by small shims Confidence: high Scope-risk: narrow Reversibility: clean Directive: For later cleanup phases, treat build-time stubs and source-test imports as separate compatibility surfaces Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (still noisy on this upstream snapshot) --------- Co-authored-by: anandh8x <test@example.com> * Reduce internal-only labeling noise in source comments (Gitlawb#355) This pass rewrites comment-only ANT-ONLY markers to neutral internal-only language across the source tree without changing runtime strings, flags, commands, or protocol identifiers. The goal is to lower obvious internal prose leakage while keeping the diff mechanically safe and easy to review. Constraint: Phase B is limited to comments/prose only; runtime strings and user-facing labels remain deferred Rejected: Broad search-and-replace across strings and command descriptions | too risky for a prose-only pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Remaining ANT-ONLY hits are mostly runtime/user-facing strings and should be handled separately from comment cleanup Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Neutralize internal Anthropic prose in explanatory comments (Gitlawb#357) This is a small prose-only follow-up that rewrites clearly internal or explanatory Anthropic comment language to neutral wording in a handful of high-confidence files. It avoids runtime strings, flags, command labels, protocol identifiers, and provider-facing references. Constraint: Keep this pass narrowly scoped to comments/documentation only Rejected: Broader Anthropic comment sweep across functional API/protocol references | too ambiguous for a safe prose-only PR Confidence: high Scope-risk: narrow Reversibility: clean Directive: Leave functional Anthropic references (API behavior, SDKs, URLs, provider labels, protocol docs) for separate reviewed passes Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Neutralize remaining internal-only diagnostic labels (Gitlawb#359) This pass rewrites a small set of ant-only diagnostic and UI labels to neutral internal wording while leaving command definitions, flags, and runtime logic untouched. It focuses on internal debug output, dead UI branches, and noninteractive headings rather than broader product text. Constraint: Label cleanup only; do not change command semantics or ant-only logic gates Rejected: Renaming ant-only command descriptions in main.tsx | broader UX surface better handled in a separate reviewed pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Remaining ANT-ONLY hits are mostly command descriptions and intentionally deferred user-facing strings Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Finish eliminating remaining ANT-ONLY source labels (Gitlawb#360) This extends the label-only cleanup to the remaining internal-only command, debug, and heading strings so the source tree no longer contains ANT-ONLY markers. The pass still avoids logic changes and only renames labels shown in internal or gated surfaces. Constraint: Update the existing label-cleanup PR without widening scope into behavior changes Rejected: Leave the last ANT-ONLY strings for a later pass | low-cost cleanup while the branch is already focused on labels Confidence: high Scope-risk: narrow Reversibility: clean Directive: The next phase should move off label cleanup and onto a separately scoped logic or rebrand slice Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Stub internal-only recording and model capability helpers (Gitlawb#377) This follow-up Phase C-lite slice replaces purely internal helper modules with stable external no-op surfaces and collapses internal elevated error logging to a no-op. The change removes additional USER_TYPE-gated helper behavior without touching product-facing runtime flows. Constraint: Keep this PR limited to isolated helper modules that are already external no-ops in practice Rejected: Pulling in broader speculation or logging sink changes | less isolated and easier to debate during review Confidence: high Scope-risk: narrow Reversibility: clean Directive: Continue Phase C with similarly isolated helpers before moving into mixed behavior files Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) Co-authored-by: anandh8x <test@example.com> * Remove internal-only bundled skills and mock helpers (Gitlawb#376) * Remove internal-only bundled skills and mock rate-limit behavior This takes the next planned Phase C-lite slice by deleting bundled skills that only ever registered for internal users and replacing the internal mock rate-limit helper with a stable no-op external stub. The external build keeps the same behavior while removing a concentrated block of USER_TYPE-gated dead code. Constraint: Limit this PR to isolated internal-only helpers and avoid bridge, oauth, or rebrand behavior Rejected: Broad USER_TYPE cleanup across mixed runtime surfaces | too risky for the next medium-sized PR Confidence: high Scope-risk: moderate Reversibility: clean Directive: The next cleanup pass should continue with similarly isolated USER_TYPE helpers before touching main.tsx or protocol-heavy code Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy) * Align internal-only helper removal with remaining user guidance This follow-up fixes the mock billing stub to be a true no-op and removes stale user-facing references to /verify and /skillify from the same PR. It also leaves a clearer paper trail for review: the deleted verify skill was explicitly ant-gated before removal, and the remaining mock helper callers still resolve to safe no-op returns in the external build. Constraint: Keep the PR focused on consistency fixes and reviewer-requested evidence, not new cleanup scope Rejected: Leave stale guidance for a later PR | would make this branch internally inconsistent after skill removal Confidence: high Scope-risk: narrow Reversibility: clean Directive: When deleting gated features, always sweep user guidance and coordinator prompts in the same pass Tested: bun run build Tested: bun run smoke Tested: bun run verify:privacy Tested: bun run test:provider Tested: bun run test:provider-recommendation Not-tested: Full repo typecheck (upstream baseline remains noisy; changed-file scan still shows only pre-existing tipRegistry errors outside edited lines) * Clarify generic workflow wording after skill removal This removes the last generic verification-skill wording that could still be read as pointing at a deleted bundled command. The guidance now talks about project workflows rather than a specific bundled verify skill. Constraint: Keep the follow-up limited to reviewer-facing wording cleanup on the same PR Rejected: Leave generic wording as-is | still too easy to misread after the explicit /verify references were removed Confidence: high Scope-risk: narrow Reversibility: clean Directive: When removing bundled commands, scrub both explicit and generic references in the same branch Tested: bun run build Tested: bun run smoke Not-tested: Additional checks unchanged by wording-only follow-up --------- Co-authored-by: anandh8x <test@example.com> * test(api): add GEMINI_AUTH_MODE to environment setup in tests * test: isolate GitHub/Gemini credential tests with fresh module imports and explicit non-bare env setup to prevent cross-test mock/cache leaks * fix: update GitHub Copilot base URL and model defaults for improved compatibility * fix: enhance error handling in OpenAI API response processing * fix: improve error handling for GitHub Copilot API responses and streamline error body consumption * fix: enhance response handling in OpenAI API shim for better error reporting and support for streaming responses * feat: enhance GitHub device flow with fresh module import and token validation improvements * fix: separate Copilot API routing from GitHub Models, clear stale env vars, honor providerOverride.apiKey * fix: route GitHub GPT-5/Codex to Copilot API, show all Copilot models in picker, clear stale env vars * fix GitHub Models API regression * feat: update GitHub authentication to require OAuth tokens, normalize model handling for Copilot and GitHub Models * fix: update GitHub token validation to support OAuth tokens and improve endpoint type handling --------- Co-authored-by: Anandan <anandan.8x@gmail.com> Co-authored-by: anandh8x <test@example.com>
Summary
models.github.aitoapi.githubcopilot.comand aligned request headers with Copilot client expectations.responsestransport when required.chat/completionstoresponsesfor copilot newer model.Why it changed
Impact
Testing
bun run buildbun run smokebun run test:providerScreenshots
Before:

After:

Known limitations
/onboard-githubOAuth flow it can access all the model.