feat: GitHub provider lifecycle and onboarding hardening (#351)
* feat: improve GitHub provider onboarding and lifecycle * fix: address copilot review in provider manager * fix: address follow-up copilot review comments * test: resolve rebase conflict in provider profiles suite * fix: clear stale github hydrated marker * fix: harden github onboarding auth precedence * fix: remove merge markers from provider tests * fix: resolve latest copilot onboarding comments --------- Co-authored-by: KRATOS <84986124+gnanam1990@users.noreply.github.com>
This commit is contained in:
Agent_J
GitHub
@@ -2,6 +2,7 @@ import type { Command } from '../../commands.js'
|
||||
|
||||
const onboardGithub: Command = {
|
||||
name: 'onboard-github',
|
||||
aliases: ['onboarding-github', 'onboardgithub', 'onboardinggithub'],
|
||||
description:
|
||||
'Interactive setup for GitHub Models: device login or PAT, saved to secure storage',
|
||||
type: 'local-jsx',
|
||||
|
||||
148
src/commands/onboard-github/onboard-github.test.ts
Normal file
148
src/commands/onboard-github/onboard-github.test.ts
Normal file
@@ -0,0 +1,148 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
|
||||
import {
|
||||
activateGithubOnboardingMode,
|
||||
applyGithubOnboardingProcessEnv,
|
||||
buildGithubOnboardingSettingsEnv,
|
||||
hasExistingGithubModelsLoginToken,
|
||||
shouldForceGithubRelogin,
|
||||
} from './onboard-github.js'
|
||||
|
||||
describe('shouldForceGithubRelogin', () => {
|
||||
test.each(['force', '--force', 'relogin', '--relogin', 'reauth', '--reauth'])(
|
||||
'treats %s as force re-login',
|
||||
arg => {
|
||||
expect(shouldForceGithubRelogin(arg)).toBe(true)
|
||||
},
|
||||
)
|
||||
|
||||
test('returns false for empty or unknown args', () => {
|
||||
expect(shouldForceGithubRelogin('')).toBe(false)
|
||||
expect(shouldForceGithubRelogin(undefined)).toBe(false)
|
||||
expect(shouldForceGithubRelogin('something-else')).toBe(false)
|
||||
})
|
||||
|
||||
test('treats force flags as present in multi-word args', () => {
|
||||
expect(shouldForceGithubRelogin('--force extra')).toBe(true)
|
||||
expect(shouldForceGithubRelogin('foo --relogin bar')).toBe(true)
|
||||
expect(shouldForceGithubRelogin('abc reauth xyz')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('hasExistingGithubModelsLoginToken', () => {
|
||||
test('returns true when GITHUB_TOKEN is present', () => {
|
||||
expect(
|
||||
hasExistingGithubModelsLoginToken({ GITHUB_TOKEN: 'token' }, ''),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
test('returns true when GH_TOKEN is present', () => {
|
||||
expect(
|
||||
hasExistingGithubModelsLoginToken({ GH_TOKEN: 'token' }, ''),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
test('returns true when stored token exists', () => {
|
||||
expect(hasExistingGithubModelsLoginToken({}, 'stored-token')).toBe(true)
|
||||
})
|
||||
|
||||
test('returns false when both env and stored token are missing', () => {
|
||||
expect(hasExistingGithubModelsLoginToken({}, '')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('onboarding auth precedence cleanup', () => {
|
||||
test('clears preexisting OpenAI auth when switching to GitHub', () => {
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
CLAUDE_CODE_USE_OPENAI: '1',
|
||||
OPENAI_MODEL: 'gpt-4o',
|
||||
OPENAI_API_KEY: 'sk-stale-openai-key',
|
||||
OPENAI_ORG: 'org-old',
|
||||
OPENAI_PROJECT: 'project-old',
|
||||
OPENAI_ORGANIZATION: 'org-legacy',
|
||||
OPENAI_BASE_URL: 'https://api.openai.com/v1',
|
||||
OPENAI_API_BASE: 'https://api.openai.com/v1',
|
||||
CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED: '1',
|
||||
CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED_ID: 'profile_old',
|
||||
}
|
||||
|
||||
applyGithubOnboardingProcessEnv('github:copilot', env)
|
||||
|
||||
expect(env.CLAUDE_CODE_USE_GITHUB).toBe('1')
|
||||
expect(env.OPENAI_MODEL).toBe('github:copilot')
|
||||
|
||||
expect(env.OPENAI_API_KEY).toBeUndefined()
|
||||
expect(env.OPENAI_ORG).toBeUndefined()
|
||||
expect(env.OPENAI_PROJECT).toBeUndefined()
|
||||
expect(env.OPENAI_ORGANIZATION).toBeUndefined()
|
||||
expect(env.OPENAI_BASE_URL).toBeUndefined()
|
||||
expect(env.OPENAI_API_BASE).toBeUndefined()
|
||||
|
||||
expect(env.CLAUDE_CODE_USE_OPENAI).toBeUndefined()
|
||||
expect(env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED).toBeUndefined()
|
||||
expect(env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED_ID).toBeUndefined()
|
||||
|
||||
const settingsEnv = buildGithubOnboardingSettingsEnv('github:copilot')
|
||||
expect(settingsEnv.CLAUDE_CODE_USE_GITHUB).toBe('1')
|
||||
expect(settingsEnv.OPENAI_MODEL).toBe('github:copilot')
|
||||
expect(settingsEnv.OPENAI_API_KEY).toBeUndefined()
|
||||
expect(settingsEnv.OPENAI_ORG).toBeUndefined()
|
||||
expect(settingsEnv.OPENAI_PROJECT).toBeUndefined()
|
||||
expect(settingsEnv.OPENAI_ORGANIZATION).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('activateGithubOnboardingMode', () => {
|
||||
test('activates settings/env/hydration in order when merge succeeds', () => {
|
||||
const calls: string[] = []
|
||||
|
||||
const result = activateGithubOnboardingMode(' github:copilot ', {
|
||||
mergeSettingsEnv: model => {
|
||||
calls.push(`merge:${model}`)
|
||||
return { ok: true }
|
||||
},
|
||||
applyProcessEnv: model => {
|
||||
calls.push(`apply:${model}`)
|
||||
},
|
||||
hydrateToken: () => {
|
||||
calls.push('hydrate')
|
||||
},
|
||||
onChangeAPIKey: () => {
|
||||
calls.push('onChangeAPIKey')
|
||||
},
|
||||
})
|
||||
|
||||
expect(result).toEqual({ ok: true })
|
||||
expect(calls).toEqual([
|
||||
'merge:github:copilot',
|
||||
'apply:github:copilot',
|
||||
'hydrate',
|
||||
'onChangeAPIKey',
|
||||
])
|
||||
})
|
||||
|
||||
test('stops activation when settings merge fails', () => {
|
||||
const calls: string[] = []
|
||||
|
||||
const result = activateGithubOnboardingMode(DEFAULT_MODEL_FOR_TESTS, {
|
||||
mergeSettingsEnv: () => {
|
||||
calls.push('merge')
|
||||
return { ok: false, detail: 'settings write failed' }
|
||||
},
|
||||
applyProcessEnv: () => {
|
||||
calls.push('apply')
|
||||
},
|
||||
hydrateToken: () => {
|
||||
calls.push('hydrate')
|
||||
},
|
||||
onChangeAPIKey: () => {
|
||||
calls.push('onChangeAPIKey')
|
||||
},
|
||||
})
|
||||
|
||||
expect(result).toEqual({ ok: false, detail: 'settings write failed' })
|
||||
expect(calls).toEqual(['merge'])
|
||||
})
|
||||
})
|
||||
|
||||
const DEFAULT_MODEL_FOR_TESTS = 'github:copilot'
|
||||
@@ -12,11 +12,20 @@ import {
|
||||
import type { LocalJSXCommandCall } from '../../types/command.js'
|
||||
import {
|
||||
hydrateGithubModelsTokenFromSecureStorage,
|
||||
readGithubModelsToken,
|
||||
saveGithubModelsToken,
|
||||
} from '../../utils/githubModelsCredentials.js'
|
||||
import { updateSettingsForSource } from '../../utils/settings/settings.js'
|
||||
|
||||
const DEFAULT_MODEL = 'github:copilot'
|
||||
const FORCE_RELOGIN_ARGS = new Set([
|
||||
'force',
|
||||
'--force',
|
||||
'relogin',
|
||||
'--relogin',
|
||||
'reauth',
|
||||
'--reauth',
|
||||
])
|
||||
|
||||
type Step =
|
||||
| 'menu'
|
||||
@@ -24,17 +33,72 @@ type Step =
|
||||
| 'pat'
|
||||
| 'error'
|
||||
|
||||
export function shouldForceGithubRelogin(args?: string): boolean {
|
||||
const normalized = (args ?? '').trim().toLowerCase()
|
||||
if (!normalized) {
|
||||
return false
|
||||
}
|
||||
return normalized.split(/\s+/).some(arg => FORCE_RELOGIN_ARGS.has(arg))
|
||||
}
|
||||
|
||||
export function hasExistingGithubModelsLoginToken(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
storedToken?: string,
|
||||
): boolean {
|
||||
const envToken = env.GITHUB_TOKEN?.trim() || env.GH_TOKEN?.trim()
|
||||
if (envToken) {
|
||||
return true
|
||||
}
|
||||
const persisted = (storedToken ?? readGithubModelsToken())?.trim()
|
||||
return Boolean(persisted)
|
||||
}
|
||||
|
||||
export function buildGithubOnboardingSettingsEnv(
|
||||
model: string,
|
||||
): Record<string, string | undefined> {
|
||||
return {
|
||||
CLAUDE_CODE_USE_GITHUB: '1',
|
||||
OPENAI_MODEL: model,
|
||||
OPENAI_API_KEY: undefined,
|
||||
OPENAI_ORG: undefined,
|
||||
OPENAI_PROJECT: undefined,
|
||||
OPENAI_ORGANIZATION: undefined,
|
||||
OPENAI_BASE_URL: undefined,
|
||||
OPENAI_API_BASE: undefined,
|
||||
CLAUDE_CODE_USE_OPENAI: undefined,
|
||||
CLAUDE_CODE_USE_GEMINI: undefined,
|
||||
CLAUDE_CODE_USE_BEDROCK: undefined,
|
||||
CLAUDE_CODE_USE_VERTEX: undefined,
|
||||
CLAUDE_CODE_USE_FOUNDRY: undefined,
|
||||
}
|
||||
}
|
||||
|
||||
export function applyGithubOnboardingProcessEnv(
|
||||
model: string,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): void {
|
||||
env.CLAUDE_CODE_USE_GITHUB = '1'
|
||||
env.OPENAI_MODEL = model
|
||||
|
||||
delete env.OPENAI_API_KEY
|
||||
delete env.OPENAI_ORG
|
||||
delete env.OPENAI_PROJECT
|
||||
delete env.OPENAI_ORGANIZATION
|
||||
delete env.OPENAI_BASE_URL
|
||||
delete env.OPENAI_API_BASE
|
||||
|
||||
delete env.CLAUDE_CODE_USE_OPENAI
|
||||
delete env.CLAUDE_CODE_USE_GEMINI
|
||||
delete env.CLAUDE_CODE_USE_BEDROCK
|
||||
delete env.CLAUDE_CODE_USE_VERTEX
|
||||
delete env.CLAUDE_CODE_USE_FOUNDRY
|
||||
delete env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED
|
||||
delete env.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED_ID
|
||||
}
|
||||
|
||||
function mergeUserSettingsEnv(model: string): { ok: boolean; detail?: string } {
|
||||
const { error } = updateSettingsForSource('userSettings', {
|
||||
env: {
|
||||
CLAUDE_CODE_USE_GITHUB: '1',
|
||||
OPENAI_MODEL: model,
|
||||
CLAUDE_CODE_USE_OPENAI: undefined as any,
|
||||
CLAUDE_CODE_USE_GEMINI: undefined as any,
|
||||
CLAUDE_CODE_USE_BEDROCK: undefined as any,
|
||||
CLAUDE_CODE_USE_VERTEX: undefined as any,
|
||||
CLAUDE_CODE_USE_FOUNDRY: undefined as any,
|
||||
},
|
||||
env: buildGithubOnboardingSettingsEnv(model) as any,
|
||||
})
|
||||
if (error) {
|
||||
return { ok: false, detail: error.message }
|
||||
@@ -42,6 +106,32 @@ function mergeUserSettingsEnv(model: string): { ok: boolean; detail?: string } {
|
||||
return { ok: true }
|
||||
}
|
||||
|
||||
export function activateGithubOnboardingMode(
|
||||
model: string = DEFAULT_MODEL,
|
||||
options?: {
|
||||
mergeSettingsEnv?: (model: string) => { ok: boolean; detail?: string }
|
||||
applyProcessEnv?: (model: string) => void
|
||||
hydrateToken?: () => void
|
||||
onChangeAPIKey?: () => void
|
||||
},
|
||||
): { ok: boolean; detail?: string } {
|
||||
const normalizedModel = model.trim() || DEFAULT_MODEL
|
||||
const mergeSettingsEnv = options?.mergeSettingsEnv ?? mergeUserSettingsEnv
|
||||
const applyProcessEnv = options?.applyProcessEnv ?? applyGithubOnboardingProcessEnv
|
||||
const hydrateToken =
|
||||
options?.hydrateToken ?? hydrateGithubModelsTokenFromSecureStorage
|
||||
|
||||
const merged = mergeSettingsEnv(normalizedModel)
|
||||
if (!merged.ok) {
|
||||
return merged
|
||||
}
|
||||
|
||||
applyProcessEnv(normalizedModel)
|
||||
hydrateToken()
|
||||
options?.onChangeAPIKey?.()
|
||||
return { ok: true }
|
||||
}
|
||||
|
||||
function OnboardGithub(props: {
|
||||
onDone: Parameters<LocalJSXCommandCall>[0]
|
||||
onChangeAPIKey: () => void
|
||||
@@ -64,19 +154,17 @@ function OnboardGithub(props: {
|
||||
setStep('error')
|
||||
return
|
||||
}
|
||||
const merged = mergeUserSettingsEnv(model.trim() || DEFAULT_MODEL)
|
||||
if (!merged.ok) {
|
||||
const activated = activateGithubOnboardingMode(model, {
|
||||
onChangeAPIKey,
|
||||
})
|
||||
if (!activated.ok) {
|
||||
setErrorMsg(
|
||||
`Token saved, but settings were not updated: ${merged.detail ?? 'unknown error'}. ` +
|
||||
`Token saved, but settings were not updated: ${activated.detail ?? 'unknown error'}. ` +
|
||||
`Add env CLAUDE_CODE_USE_GITHUB=1 and OPENAI_MODEL to ~/.claude/settings.json manually.`,
|
||||
)
|
||||
setStep('error')
|
||||
return
|
||||
}
|
||||
process.env.CLAUDE_CODE_USE_GITHUB = '1'
|
||||
process.env.OPENAI_MODEL = model.trim() || DEFAULT_MODEL
|
||||
hydrateGithubModelsTokenFromSecureStorage()
|
||||
onChangeAPIKey()
|
||||
onDone(
|
||||
'GitHub Models onboard complete. Token stored in secure storage; user settings updated. Restart if the model does not switch.',
|
||||
{ display: 'user' },
|
||||
@@ -147,11 +235,11 @@ function OnboardGithub(props: {
|
||||
{deviceHint.verification_uri}
|
||||
</Text>
|
||||
<Text dimColor>
|
||||
A browser window may have opened. Waiting for authorization…
|
||||
A browser window may have opened. Waiting for authorization...
|
||||
</Text>
|
||||
</>
|
||||
) : (
|
||||
<Text dimColor>Requesting device code from GitHub…</Text>
|
||||
<Text dimColor>Requesting device code from GitHub...</Text>
|
||||
)}
|
||||
<Spinner />
|
||||
</Box>
|
||||
@@ -206,7 +294,7 @@ function OnboardGithub(props: {
|
||||
<Text bold>GitHub Models setup</Text>
|
||||
<Text dimColor>
|
||||
Stores your token in the OS credential store (macOS Keychain when available)
|
||||
and enables CLAUDE_CODE_USE_GITHUB in your user settings — no export
|
||||
and enables CLAUDE_CODE_USE_GITHUB in your user settings - no export
|
||||
GITHUB_TOKEN needed for future runs.
|
||||
</Text>
|
||||
<Select
|
||||
@@ -227,7 +315,28 @@ function OnboardGithub(props: {
|
||||
)
|
||||
}
|
||||
|
||||
export const call: LocalJSXCommandCall = async (onDone, context) => {
|
||||
export const call: LocalJSXCommandCall = async (onDone, context, args) => {
|
||||
const forceRelogin = shouldForceGithubRelogin(args)
|
||||
if (hasExistingGithubModelsLoginToken() && !forceRelogin) {
|
||||
const activated = activateGithubOnboardingMode(DEFAULT_MODEL, {
|
||||
onChangeAPIKey: context.onChangeAPIKey,
|
||||
})
|
||||
if (!activated.ok) {
|
||||
onDone(
|
||||
`GitHub token detected, but settings activation failed: ${activated.detail ?? 'unknown error'}. ` +
|
||||
'Set CLAUDE_CODE_USE_GITHUB=1 and OPENAI_MODEL=github:copilot in user settings manually.',
|
||||
{ display: 'system' },
|
||||
)
|
||||
return null
|
||||
}
|
||||
|
||||
onDone(
|
||||
'GitHub Models already authorized. Activated GitHub Models mode using your existing token. Use /onboard-github --force to re-authenticate.',
|
||||
{ display: 'user' },
|
||||
)
|
||||
return null
|
||||
}
|
||||
|
||||
return (
|
||||
<OnboardGithub
|
||||
onDone={onDone}
|
||||
|
||||
@@ -275,6 +275,21 @@ test('buildCurrentProviderSummary does not relabel local gpt-5.4 providers as Co
|
||||
expect(summary.endpointLabel).toBe('http://127.0.0.1:8080/v1')
|
||||
})
|
||||
|
||||
test('buildCurrentProviderSummary recognizes GitHub Models mode', () => {
|
||||
const summary = buildCurrentProviderSummary({
|
||||
processEnv: {
|
||||
CLAUDE_CODE_USE_GITHUB: '1',
|
||||
OPENAI_MODEL: 'github:copilot',
|
||||
OPENAI_BASE_URL: 'https://models.github.ai/inference',
|
||||
},
|
||||
persisted: null,
|
||||
})
|
||||
|
||||
expect(summary.providerLabel).toBe('GitHub Models')
|
||||
expect(summary.modelLabel).toBe('github:copilot')
|
||||
expect(summary.endpointLabel).toBe('https://models.github.ai/inference')
|
||||
})
|
||||
|
||||
test('getProviderWizardDefaults ignores poisoned current provider values', () => {
|
||||
const defaults = getProviderWizardDefaults({
|
||||
OPENAI_API_KEY: 'sk-secret-12345678',
|
||||
|
||||
@@ -178,6 +178,23 @@ export function buildCurrentProviderSummary(options?: {
|
||||
}
|
||||
}
|
||||
|
||||
if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB)) {
|
||||
return {
|
||||
providerLabel: 'GitHub Models',
|
||||
modelLabel: getSafeDisplayValue(
|
||||
processEnv.OPENAI_MODEL ?? 'github:copilot',
|
||||
processEnv,
|
||||
),
|
||||
endpointLabel: getSafeDisplayValue(
|
||||
processEnv.OPENAI_BASE_URL ??
|
||||
processEnv.OPENAI_API_BASE ??
|
||||
'https://models.github.ai/inference',
|
||||
processEnv,
|
||||
),
|
||||
savedProfileLabel,
|
||||
}
|
||||
}
|
||||
|
||||
if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_OPENAI)) {
|
||||
const request = resolveProviderRequest({
|
||||
model: processEnv.OPENAI_MODEL,
|
||||
|
||||
Reference in New Issue
Block a user