fix: avoid sync github credential reads in provider manager (#428)
* fix: avoid sync github credential reads in provider manager * test: stabilize provider manager async credential test * fix: avoid first-frame github provider false negative --------- Co-authored-by: KRATOS <84986124+gnanam1990@users.noreply.github.com>
This commit is contained in:
Agent_J
GitHub
305
src/components/ProviderManager.test.tsx
Normal file
305
src/components/ProviderManager.test.tsx
Normal file
@@ -0,0 +1,305 @@
|
||||
import { PassThrough } from 'node:stream'
|
||||
|
||||
import { afterEach, expect, mock, test } from 'bun:test'
|
||||
import React from 'react'
|
||||
import stripAnsi from 'strip-ansi'
|
||||
|
||||
import { createRoot } from '../ink.js'
|
||||
import { AppStateProvider } from '../state/AppState.js'
|
||||
|
||||
const SYNC_START = '\x1B[?2026h'
|
||||
const SYNC_END = '\x1B[?2026l'
|
||||
|
||||
const ORIGINAL_ENV = {
|
||||
CLAUDE_CODE_USE_GITHUB: process.env.CLAUDE_CODE_USE_GITHUB,
|
||||
GITHUB_TOKEN: process.env.GITHUB_TOKEN,
|
||||
GH_TOKEN: process.env.GH_TOKEN,
|
||||
}
|
||||
|
||||
function extractLastFrame(output: string): string {
|
||||
let lastFrame: string | null = null
|
||||
let cursor = 0
|
||||
|
||||
while (cursor < output.length) {
|
||||
const start = output.indexOf(SYNC_START, cursor)
|
||||
if (start === -1) {
|
||||
break
|
||||
}
|
||||
|
||||
const contentStart = start + SYNC_START.length
|
||||
const end = output.indexOf(SYNC_END, contentStart)
|
||||
if (end === -1) {
|
||||
break
|
||||
}
|
||||
|
||||
const frame = output.slice(contentStart, end)
|
||||
if (frame.trim().length > 0) {
|
||||
lastFrame = frame
|
||||
}
|
||||
cursor = end + SYNC_END.length
|
||||
}
|
||||
|
||||
return lastFrame ?? output
|
||||
}
|
||||
|
||||
function createTestStreams(): {
|
||||
stdout: PassThrough
|
||||
stdin: PassThrough & {
|
||||
isTTY: boolean
|
||||
setRawMode: (mode: boolean) => void
|
||||
ref: () => void
|
||||
unref: () => void
|
||||
}
|
||||
getOutput: () => string
|
||||
} {
|
||||
let output = ''
|
||||
const stdout = new PassThrough()
|
||||
const stdin = new PassThrough() as PassThrough & {
|
||||
isTTY: boolean
|
||||
setRawMode: (mode: boolean) => void
|
||||
ref: () => void
|
||||
unref: () => void
|
||||
}
|
||||
|
||||
stdin.isTTY = true
|
||||
stdin.setRawMode = () => {}
|
||||
stdin.ref = () => {}
|
||||
stdin.unref = () => {}
|
||||
;(stdout as unknown as { columns: number }).columns = 120
|
||||
stdout.on('data', chunk => {
|
||||
output += chunk.toString()
|
||||
})
|
||||
|
||||
return {
|
||||
stdout,
|
||||
stdin,
|
||||
getOutput: () => output,
|
||||
}
|
||||
}
|
||||
|
||||
async function waitForCondition(
|
||||
predicate: () => boolean,
|
||||
options?: { timeoutMs?: number; intervalMs?: number },
|
||||
): Promise<void> {
|
||||
const timeoutMs = options?.timeoutMs ?? 2000
|
||||
const intervalMs = options?.intervalMs ?? 10
|
||||
const startedAt = Date.now()
|
||||
|
||||
while (Date.now() - startedAt < timeoutMs) {
|
||||
if (predicate()) {
|
||||
return
|
||||
}
|
||||
await Bun.sleep(intervalMs)
|
||||
}
|
||||
|
||||
throw new Error('Timed out waiting for ProviderManager test condition')
|
||||
}
|
||||
|
||||
function createDeferred<T>(): {
|
||||
promise: Promise<T>
|
||||
resolve: (value: T) => void
|
||||
} {
|
||||
let resolve!: (value: T) => void
|
||||
const promise = new Promise<T>(r => {
|
||||
resolve = r
|
||||
})
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
function mockProviderProfilesModule(): void {
|
||||
mock.module('../utils/providerProfiles.js', () => ({
|
||||
addProviderProfile: () => null,
|
||||
applyActiveProviderProfileFromConfig: () => {},
|
||||
deleteProviderProfile: () => ({ removed: false, activeProfileId: null }),
|
||||
getActiveProviderProfile: () => null,
|
||||
getProviderPresetDefaults: () => ({
|
||||
provider: 'openai',
|
||||
name: 'Mock provider',
|
||||
baseUrl: 'http://localhost:11434/v1',
|
||||
model: 'mock-model',
|
||||
apiKey: '',
|
||||
}),
|
||||
getProviderProfiles: () => [],
|
||||
setActiveProviderProfile: () => null,
|
||||
updateProviderProfile: () => null,
|
||||
}))
|
||||
}
|
||||
|
||||
function mockProviderManagerDependencies(
|
||||
syncRead: () => string | undefined,
|
||||
asyncRead: () => Promise<string | undefined>,
|
||||
): void {
|
||||
mockProviderProfilesModule()
|
||||
|
||||
mock.module('../utils/githubModelsCredentials.js', () => ({
|
||||
clearGithubModelsToken: () => ({ success: true }),
|
||||
GITHUB_MODELS_HYDRATED_ENV_MARKER: 'CLAUDE_CODE_GITHUB_TOKEN_HYDRATED',
|
||||
hydrateGithubModelsTokenFromSecureStorage: () => {},
|
||||
readGithubModelsToken: syncRead,
|
||||
readGithubModelsTokenAsync: asyncRead,
|
||||
}))
|
||||
|
||||
mock.module('../utils/settings/settings.js', () => ({
|
||||
updateSettingsForSource: () => ({ error: null }),
|
||||
}))
|
||||
}
|
||||
|
||||
async function waitForFrameOutput(
|
||||
getOutput: () => string,
|
||||
predicate: (output: string) => boolean,
|
||||
timeoutMs = 2500,
|
||||
): Promise<string> {
|
||||
let output = ''
|
||||
|
||||
await waitForCondition(() => {
|
||||
output = stripAnsi(extractLastFrame(getOutput()))
|
||||
return predicate(output)
|
||||
}, { timeoutMs })
|
||||
|
||||
return output
|
||||
}
|
||||
|
||||
async function mountProviderManager(
|
||||
ProviderManager: React.ComponentType<{
|
||||
mode: 'first-run' | 'manage'
|
||||
onDone: () => void
|
||||
}>,
|
||||
): Promise<{
|
||||
getOutput: () => string
|
||||
dispose: () => Promise<void>
|
||||
}> {
|
||||
const { stdout, stdin, getOutput } = createTestStreams()
|
||||
const root = await createRoot({
|
||||
stdout: stdout as unknown as NodeJS.WriteStream,
|
||||
stdin: stdin as unknown as NodeJS.ReadStream,
|
||||
patchConsole: false,
|
||||
})
|
||||
|
||||
root.render(
|
||||
<AppStateProvider>
|
||||
<ProviderManager
|
||||
mode="manage"
|
||||
onDone={() => {}}
|
||||
/>
|
||||
</AppStateProvider>,
|
||||
)
|
||||
|
||||
return {
|
||||
getOutput,
|
||||
dispose: async () => {
|
||||
root.unmount()
|
||||
stdin.end()
|
||||
stdout.end()
|
||||
await Bun.sleep(0)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
async function renderProviderManagerFrame(
|
||||
ProviderManager: React.ComponentType<{
|
||||
mode: 'first-run' | 'manage'
|
||||
onDone: () => void
|
||||
}>,
|
||||
options?: {
|
||||
waitForOutput?: (output: string) => boolean
|
||||
timeoutMs?: number
|
||||
},
|
||||
): Promise<string> {
|
||||
const mounted = await mountProviderManager(ProviderManager)
|
||||
const output = await waitForFrameOutput(
|
||||
mounted.getOutput,
|
||||
frame => {
|
||||
if (!options?.waitForOutput) {
|
||||
return frame.includes('Provider manager')
|
||||
}
|
||||
return options.waitForOutput(frame)
|
||||
},
|
||||
options?.timeoutMs ?? 2500,
|
||||
)
|
||||
|
||||
await mounted.dispose()
|
||||
return output
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
mock.restore()
|
||||
|
||||
for (const [key, value] of Object.entries(ORIGINAL_ENV)) {
|
||||
if (value === undefined) {
|
||||
delete process.env[key as keyof typeof ORIGINAL_ENV]
|
||||
} else {
|
||||
process.env[key as keyof typeof ORIGINAL_ENV] = value
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
test('ProviderManager resolves GitHub virtual provider from async storage without sync reads in render flow', async () => {
|
||||
delete process.env.CLAUDE_CODE_USE_GITHUB
|
||||
delete process.env.GITHUB_TOKEN
|
||||
delete process.env.GH_TOKEN
|
||||
|
||||
const syncRead = mock(() => {
|
||||
throw new Error('sync credential read should not run in ProviderManager render flow')
|
||||
})
|
||||
const asyncRead = mock(async () => 'stored-token')
|
||||
|
||||
mockProviderManagerDependencies(syncRead, asyncRead)
|
||||
|
||||
const nonce = `${Date.now()}-${Math.random()}`
|
||||
const { ProviderManager } = await import(`./ProviderManager.js?ts=${nonce}`)
|
||||
const output = await renderProviderManagerFrame(ProviderManager, {
|
||||
waitForOutput: frame =>
|
||||
frame.includes('Provider manager') &&
|
||||
frame.includes('GitHub Models') &&
|
||||
frame.includes('token stored'),
|
||||
})
|
||||
|
||||
expect(output).toContain('Provider manager')
|
||||
expect(output).toContain('GitHub Models')
|
||||
expect(output).toContain('token stored')
|
||||
expect(output).not.toContain('No provider profiles configured yet.')
|
||||
|
||||
expect(syncRead).not.toHaveBeenCalled()
|
||||
expect(asyncRead).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('ProviderManager avoids first-frame false negative while stored-token lookup is pending', async () => {
|
||||
delete process.env.CLAUDE_CODE_USE_GITHUB
|
||||
delete process.env.GITHUB_TOKEN
|
||||
delete process.env.GH_TOKEN
|
||||
|
||||
const syncRead = mock(() => {
|
||||
throw new Error('sync credential read should not run in ProviderManager render flow')
|
||||
})
|
||||
const deferredStoredToken = createDeferred<string | undefined>()
|
||||
const asyncRead = mock(async () => deferredStoredToken.promise)
|
||||
|
||||
mockProviderManagerDependencies(syncRead, asyncRead)
|
||||
|
||||
const nonce = `${Date.now()}-${Math.random()}`
|
||||
const { ProviderManager } = await import(`./ProviderManager.js?ts=${nonce}`)
|
||||
const mounted = await mountProviderManager(ProviderManager)
|
||||
|
||||
const firstFrame = await waitForFrameOutput(
|
||||
mounted.getOutput,
|
||||
frame => frame.includes('Provider manager'),
|
||||
)
|
||||
|
||||
expect(firstFrame).toContain('Checking GitHub Models credentials...')
|
||||
expect(firstFrame).not.toContain('No provider profiles configured yet.')
|
||||
|
||||
deferredStoredToken.resolve('stored-token')
|
||||
|
||||
const resolvedFrame = await waitForFrameOutput(
|
||||
mounted.getOutput,
|
||||
frame => frame.includes('GitHub Models') && frame.includes('token stored'),
|
||||
)
|
||||
|
||||
expect(resolvedFrame).toContain('GitHub Models')
|
||||
expect(resolvedFrame).toContain('token stored')
|
||||
|
||||
await mounted.dispose()
|
||||
|
||||
expect(syncRead).not.toHaveBeenCalled()
|
||||
expect(asyncRead).toHaveBeenCalled()
|
||||
})
|
||||
Reference in New Issue
Block a user