-
Notifications
You must be signed in to change notification settings - Fork 0
Fix model-aware fallback on first request #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,13 +118,32 @@ export class CopilotAccountManager { | |
| async markModelUnsupported(id: string, model: string) { | ||
| const account = this.accounts.find((item) => item.id === id); | ||
| if (!account) return; | ||
| account.models = Array.isArray(account.models) | ||
| ? account.models.filter((item) => item !== model) | ||
| : []; | ||
| if (Array.isArray(account.models)) { | ||
| account.models = account.models.filter((item) => item !== model); | ||
| } | ||
| this.availability.markUnsupported(account, model); | ||
| await this.persist(); | ||
| } | ||
|
|
||
| isAccountEligible( | ||
| account: CopilotAccount, | ||
| modelId: string, | ||
| host: string, | ||
| excludedAccountIds: Set<string> = new Set() | ||
| ) { | ||
|
Comment on lines
+125
to
+130
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add explicit return types to the new public methods.
♻️ Proposed fix isAccountEligible(
account: CopilotAccount,
modelId: string,
host: string,
excludedAccountIds: Set<string> = new Set()
- ) {
+ ): boolean { selectAccount(
modelId: string,
host: string,
excludedAccountIds: Set<string> = new Set()
- ): AccountSelection | null {
+ ): AccountSelection | null {As per coding guidelines, "Prefer explicit type annotations over inference". Also applies to: 187-191 🤖 Prompt for AI Agents |
||
| if (excludedAccountIds.has(account.id)) return false; | ||
| if (!account.enabled) return false; | ||
| if (account.host !== host) return false; | ||
| if (account.cooldownUntil && account.cooldownUntil > Date.now()) return false; | ||
| if (this.availability.isUnsupported(account, modelId)) return false; | ||
|
|
||
| const cachedModels = this.availability.get(account); | ||
| const models = cachedModels ?? account.models ?? null; | ||
| if (!models || models.length === 0) return true; | ||
|
|
||
| return models.includes(modelId); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| async markFailure(id: string, cooldownMs: number) { | ||
| const account = this.accounts.find((item) => item.id === id); | ||
| if (!account) return; | ||
|
|
@@ -167,15 +186,13 @@ export class CopilotAccountManager { | |
| }; | ||
| } | ||
|
|
||
| selectAccount(modelId: string, host: string): AccountSelection | null { | ||
| selectAccount( | ||
| modelId: string, | ||
| host: string, | ||
| excludedAccountIds: Set<string> = new Set() | ||
| ): AccountSelection | null { | ||
| const eligible = this.accounts.filter((account) => { | ||
| if (!account.enabled) return false; | ||
| if (account.host !== host) return false; | ||
| if (account.cooldownUntil && account.cooldownUntil > Date.now()) return false; | ||
| const cached = this.availability.get(account); | ||
| const models = cached ?? account.models; | ||
| if (!models || models.length === 0) return true; | ||
| return models.includes(modelId); | ||
| return this.isAccountEligible(account, modelId, host, excludedAccountIds); | ||
| }); | ||
|
|
||
| if (eligible.length === 0) return null; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,7 +105,8 @@ function sanitizeCopilotBody(body?: string): string | undefined { | |
|
|
||
| function getHeaderValue(headers: HeadersInit | undefined, key: string): string | undefined { | ||
| if (!headers) return undefined; | ||
| if (headers instanceof Headers) return headers.get(key) ?? headers.get(key.toLowerCase()) ?? undefined; | ||
| if (headers instanceof Headers) | ||
| return headers.get(key) ?? headers.get(key.toLowerCase()) ?? undefined; | ||
| if (Array.isArray(headers)) { | ||
| const found = headers.find(([name]) => name.toLowerCase() === key.toLowerCase()); | ||
| return found ? found[1] : undefined; | ||
|
|
@@ -121,20 +122,6 @@ function getInitiator(headers: HeadersInit | undefined): Initiator { | |
| return undefined; | ||
| } | ||
|
|
||
| function isAccountEligible( | ||
| account: { enabled: boolean; host: string; cooldownUntil?: number; models?: string[] }, | ||
| modelId: string, | ||
| host: string, | ||
| ) { | ||
| if (!account.enabled) return false; | ||
| if (account.host !== host) return false; | ||
| if (account.cooldownUntil && account.cooldownUntil > Date.now()) return false; | ||
| if (Array.isArray(account.models) && account.models.length > 0) { | ||
| return account.models.includes(modelId); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| function buildHeaders(base: HeadersInit | undefined, auth: string, parsed: ParsedRequest) { | ||
| const headers = new Headers(base); | ||
| headers.set('authorization', `Bearer ${auth}`); | ||
|
|
@@ -160,6 +147,34 @@ function getRetryAfter(response: Response, fallback: number) { | |
| return fallback; | ||
| } | ||
|
|
||
| function isModelUnavailableBody(bodyText: string, modelId: string) { | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const normalized = bodyText.toLowerCase(); | ||
| const mentionsModel = | ||
| normalized.includes('model') || | ||
| (modelId !== 'unknown' && normalized.includes(modelId.toLowerCase())); | ||
|
|
||
| if (!mentionsModel) return false; | ||
|
|
||
| return [ | ||
| 'not found', | ||
| 'does not exist', | ||
| 'not available', | ||
| 'not supported', | ||
| 'unsupported', | ||
| 'no access to model', | ||
| 'access to this model', | ||
| ].some((phrase) => normalized.includes(phrase)); | ||
| } | ||
|
|
||
| async function isModelUnavailableResponse(response: Response, modelId: string) { | ||
| if (response.status !== 400 && response.status !== 404) return false; | ||
| const bodyText = await response | ||
| .clone() | ||
| .text() | ||
| .catch(() => ''); | ||
| return isModelUnavailableBody(bodyText, modelId); | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
Comment on lines
+150
to
+176
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tighten the model-unavailable classifier before caching the miss. Right now any 400/403/404 body that contains the word 💡 Narrow the heuristic function isModelUnavailableBody(bodyText: string, modelId: string): boolean {
const normalized = bodyText.toLowerCase();
- const mentionsModel =
- normalized.includes('model') ||
- (modelId !== 'unknown' && normalized.includes(modelId.toLowerCase()));
+ const mentionsRequestedModel =
+ modelId !== 'unknown' && normalized.includes(modelId.toLowerCase());
+ const mentionsModel = normalized.includes('model') || mentionsRequestedModel;
- if (!mentionsModel) return false;
+ if (!mentionsModel) return false;
+
+ const strongModelSpecificMatch = [
+ 'not supported',
+ 'unsupported',
+ 'no access to model',
+ 'access to this model',
+ ].some((phrase) => normalized.includes(phrase));
+ if (strongModelSpecificMatch) return true;
return [
'not found',
'does not exist',
'not available',
- 'not supported',
- 'unsupported',
- 'no access to model',
- 'access to this model',
- ].some((phrase) => normalized.includes(phrase));
+ ].some((phrase) => mentionsRequestedModel && normalized.includes(phrase));
}🤖 Prompt for AI Agents |
||
|
|
||
| async function refreshToken(host: string, refresh: string) { | ||
| const domain = host === 'github.com' ? 'github.com' : host; | ||
| const response = await fetch(`https://${domain}/login/oauth/access_token`, { | ||
|
|
@@ -203,105 +218,150 @@ export function createCopilotFetch({ config, manager, notifier }: FetchDeps) { | |
| const now = Date.now(); | ||
| const lock = lockByHost.get(host); | ||
| const agentRecentlyActive = Boolean( | ||
| lock?.lastAgentAt && now - lock.lastAgentAt < AGENT_IDLE_TIMEOUT_MS, | ||
| lock?.lastAgentAt && now - lock.lastAgentAt < AGENT_IDLE_TIMEOUT_MS | ||
| ); | ||
| const attemptedAccountIds = new Set<string>(); | ||
| const resolvedParsed = { ...parsed, isAgent }; | ||
|
|
||
| const updateHostLock = (accountId: string) => { | ||
| const previous = lockByHost.get(host); | ||
| lockByHost.set(host, { | ||
| accountId, | ||
| lastAgentAt: isAgent ? Date.now() : (previous?.lastAgentAt ?? 0), | ||
| }); | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| const prepareSelection = async ( | ||
| selection: NonNullable<ReturnType<typeof manager.selectAccount>> | ||
| ) => { | ||
| updateHostLock(selection.account.id); | ||
|
|
||
| if (selection.account.expires > 0 && selection.account.expires < Date.now()) { | ||
| const refreshed = await refreshToken(host, selection.account.refresh); | ||
| if (refreshed) { | ||
| await manager.updateAccountTokens( | ||
| selection.account.id, | ||
| refreshed.access, | ||
| refreshed.refresh, | ||
| refreshed.expires | ||
| ); | ||
| selection.account.access = refreshed.access; | ||
| selection.account.refresh = refreshed.refresh; | ||
| selection.account.expires = refreshed.expires; | ||
| } | ||
| } | ||
|
|
||
| return selection; | ||
| }; | ||
|
|
||
| const buildFallbackMessage = ( | ||
| nextAccountLabel: string, | ||
| previousAccountLabel: string, | ||
| message: string | ||
| ) => { | ||
| return `Copilot: sticking to ${nextAccountLabel} for ${modelId}; ${previousAccountLabel} ${message}`; | ||
| }; | ||
|
|
||
| const selectFallback = ( | ||
| previousSelection: NonNullable<ReturnType<typeof manager.selectAccount>>, | ||
| message: string | ||
| ) => { | ||
| const fallback = manager.selectAccount(modelId, host, attemptedAccountIds); | ||
| if (!fallback) return null; | ||
| return { | ||
| fallback, | ||
| message: buildFallbackMessage( | ||
| fallback.account.label, | ||
| previousSelection.account.label, | ||
| message | ||
| ), | ||
| }; | ||
| }; | ||
|
|
||
| let selection = null; | ||
| if (lock && (isAgent || agentRecentlyActive)) { | ||
| const locked = manager | ||
| .listAccounts() | ||
| .find((account) => account.id === lock.accountId && isAccountEligible(account, modelId, host)); | ||
| const locked = manager.listAccounts().find((account) => { | ||
| return account.id === lock.accountId && manager.isAccountEligible(account, modelId, host); | ||
| }); | ||
| if (locked) { | ||
| selection = { account: locked, index: 0, reason: 'sticky' as const }; | ||
| } | ||
| } | ||
|
|
||
| if (!selection) { | ||
| selection = manager.selectAccount(modelId, host); | ||
| selection = manager.selectAccount(modelId, host, attemptedAccountIds); | ||
| } | ||
| if (!selection) { | ||
| throw new Error(`No eligible Copilot accounts available for ${modelId}`); | ||
| } | ||
|
|
||
| lockByHost.set(host, { | ||
| accountId: selection.account.id, | ||
| lastAgentAt: isAgent ? now : lock?.lastAgentAt ?? 0, | ||
| }); | ||
|
|
||
| if (selection.account.expires > 0 && selection.account.expires < Date.now()) { | ||
| const refreshed = await refreshToken(host, selection.account.refresh); | ||
| if (refreshed) { | ||
| await manager.updateAccountTokens( | ||
| selection.account.id, | ||
| refreshed.access, | ||
| refreshed.refresh, | ||
| refreshed.expires, | ||
| ); | ||
| selection.account.access = refreshed.access; | ||
| selection.account.refresh = refreshed.refresh; | ||
| selection.account.expires = refreshed.expires; | ||
| let notificationMessage: string | undefined; | ||
|
|
||
| for (;;) { | ||
| attemptedAccountIds.add(selection.account.id); | ||
| const preparedSelection = await prepareSelection(selection); | ||
| const headers = buildHeaders(init?.headers, preparedSelection.account.access, resolvedParsed); | ||
| const sanitizedBody = sanitizeCopilotBody(init?.body); | ||
| const response = await fetch(request, { | ||
| ...init, | ||
| body: sanitizedBody, | ||
| headers, | ||
| }); | ||
|
|
||
| if (await isModelUnavailableResponse(response, modelId)) { | ||
| await manager.markModelUnsupported(preparedSelection.account.id, modelId); | ||
| log.warn('model unavailable on account', { | ||
| account: preparedSelection.account.label, | ||
| modelId, | ||
| }); | ||
|
|
||
| const next = selectFallback(preparedSelection, 'does not support that model'); | ||
| if (!next) return response; | ||
| selection = next.fallback; | ||
| notificationMessage = next.message; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (isAgent) { | ||
| await manager.notifySelection(selection, modelId); | ||
| } | ||
| const resolvedParsed = { ...parsed, isAgent }; | ||
| const headers = buildHeaders(init?.headers, selection.account.access, resolvedParsed); | ||
|
|
||
| const sanitizedBody = sanitizeCopilotBody(init?.body); | ||
| const response = await fetch(request, { | ||
| ...init, | ||
| body: sanitizedBody, | ||
| headers, | ||
| }); | ||
|
|
||
| if (response.status === 404 || response.status === 400) { | ||
| const bodyText = await response | ||
| .clone() | ||
| .text() | ||
| .catch(() => ''); | ||
| if ( | ||
| bodyText.toLowerCase().includes('model') && | ||
| bodyText.toLowerCase().includes('not found') | ||
| ) { | ||
| await manager.markModelUnsupported(selection.account.id, modelId); | ||
| if (response.status === 401 || response.status === 403) { | ||
| await manager.markFailure(preparedSelection.account.id, config.rateLimit.defaultBackoffMs); | ||
| log.warn('auth failure detected', { account: preparedSelection.account.label, modelId }); | ||
|
|
||
| const next = selectFallback(preparedSelection, 'had an auth failure'); | ||
| if (!next) return response; | ||
| selection = next.fallback; | ||
| notificationMessage = next.message; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (response.status === 401 || response.status === 403) { | ||
| await manager.markFailure(selection.account.id, config.rateLimit.defaultBackoffMs); | ||
| log.warn('auth failure detected', { account: selection.account.label, modelId }); | ||
| const fallback = manager.selectAccount(modelId, host); | ||
| if (!fallback) return response; | ||
| await notifier.accountSelected(fallback.account, modelId, 'fallback'); | ||
| lockByHost.set(host, { | ||
| accountId: fallback.account.id, | ||
| lastAgentAt: isAgent ? Date.now() : lockByHost.get(host)?.lastAgentAt ?? 0, | ||
| }); | ||
| const retryHeaders = buildHeaders(init?.headers, fallback.account.access, resolvedParsed); | ||
| return fetch(request, { ...init, headers: retryHeaders }); | ||
| } | ||
| if (response.status === 429 || response.status === 503) { | ||
| const backoff = getRetryAfter(response, config.rateLimit.defaultBackoffMs); | ||
| await manager.markFailure( | ||
| preparedSelection.account.id, | ||
| Math.min(backoff, config.rateLimit.maxBackoffMs) | ||
| ); | ||
| log.warn('rate limit detected', { account: preparedSelection.account.label, modelId }); | ||
|
|
||
| if (response.status === 429 || response.status === 503) { | ||
| const backoff = getRetryAfter(response, config.rateLimit.defaultBackoffMs); | ||
| await manager.markFailure( | ||
| selection.account.id, | ||
| Math.min(backoff, config.rateLimit.maxBackoffMs), | ||
| ); | ||
| log.warn('rate limit detected', { account: selection.account.label, modelId }); | ||
| const fallback = manager.selectAccount(modelId, host); | ||
| if (!fallback) return response; | ||
| await notifier.accountSelected(fallback.account, modelId, 'fallback'); | ||
| lockByHost.set(host, { | ||
| accountId: fallback.account.id, | ||
| lastAgentAt: isAgent ? Date.now() : lockByHost.get(host)?.lastAgentAt ?? 0, | ||
| }); | ||
| const retryHeaders = buildHeaders(init?.headers, fallback.account.access, resolvedParsed); | ||
| return fetch(request, { ...init, headers: retryHeaders }); | ||
| } | ||
| const next = selectFallback(preparedSelection, 'hit a cooldown-worthy rate limit'); | ||
| if (!next) return response; | ||
| selection = next.fallback; | ||
| notificationMessage = next.message; | ||
| continue; | ||
| } | ||
|
|
||
| await manager.markSuccess(selection.account.id); | ||
| return response; | ||
| await manager.markSuccess(preparedSelection.account.id); | ||
| if (isAgent) { | ||
| if (notificationMessage) { | ||
| await notifier.accountSelected( | ||
| preparedSelection.account, | ||
| modelId, | ||
| 'fallback', | ||
| notificationMessage | ||
| ); | ||
| } else { | ||
| await manager.notifySelection(preparedSelection, modelId); | ||
| } | ||
| } | ||
| return response; | ||
| } | ||
| }; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.