Open
Conversation
📝 WalkthroughWalkthroughAdds model-aware same-request fallback: when a selected account reports a model unavailable, the account is marked unsupported for that model and the fetch logic retries the same request on another eligible account. Changes touch account selection, fetch retry flow, model availability cache, notifications, docs, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent/Client
participant Fetch as Copilot Fetch
participant Manager as Account Manager
participant Cache as Model Cache
participant Notifier as Usage Notifier
Agent->>Fetch: Request(modelId, host, initiator)
Fetch->>Manager: selectAccount(modelId, host)
Manager->>Cache: check models / isUnsupported(account, modelId)
Cache-->>Manager: eligibility result
Manager-->>Fetch: selected account
Fetch->>Fetch: prepare headers, refresh token if needed
Fetch->>CopilotAPI: send request
alt model unavailable (400/403/404 + body)
CopilotAPI-->>Fetch: model-unavailable response
Fetch->>Cache: markUnsupported(account, modelId)
Cache-->>Fetch: updated unsupportedModels
Fetch->>Manager: selectAccount(modelId, host, excludedAccountIds={account})
Manager->>Cache: check eligibility excluding previous
Cache-->>Manager: fallback account
Manager-->>Fetch: fallback account
Fetch->>CopilotAPI: retry request on fallback
CopilotAPI-->>Fetch: success 200
Fetch->>Notifier: accountSelected(fallback, reason, message)
else success
CopilotAPI-->>Fetch: success 200
Fetch->>Notifier: accountSelected(account, reason)
end
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/accounts/manager.ts`:
- Around line 121-123: The code currently mutates account.models by filtering
out unsupported model entries; instead, revert that mutation and ensure
unsupported-model removals are recorded only in the ModelAvailabilityCache used
by isAccountEligible(). Locate the block that checks
Array.isArray(account.models) and remove the line that assigns account.models =
account.models.filter(...); instead update or call the ModelAvailabilityCache
API (the same mechanism used to mark TTL-scoped unsupported models) to record
the unsupported model for that account so account.models remains unchanged and
the exclusion expires with the cache.
- Around line 140-144: The current check treats an empty models array as
"unknown" by returning true when models.length === 0; update the logic in the
method that reads this.availability.get(account) (the block assigning
cachedModels and models and returning models.includes(modelId)) so that
null/undefined still means "unknown" (keep returning true), but an explicit
empty array (models.length === 0) is treated as "supports nothing" and returns
false; ensure the final return uses models.includes(modelId) only when models is
a non-empty array.
In `@src/fetch/copilot-fetch.ts`:
- Around line 169-175: The function isModelUnavailableResponse currently checks
only for 400 and 404, but must also treat 403 as a model-unavailable case;
update isModelUnavailableResponse (the async function
isModelUnavailableResponse(response: Response, modelId: string)) to include
response.status === 403 in the initial status check so that 403 responses are
routed to isModelUnavailableBody(modelId) and ultimately call
markModelUnsupported for that model instead of triggering account-wide
auth-failure handling.
- Around line 226-231: The current updateHostLock function lets a non-agent
request overwrite the sticky agent accountId even when the agent was recently
active; change updateHostLock (which updates lockByHost for host) to preserve
previous.accountId if previous.lastAgentAt indicates the agent was recently
active and the current request is not an agent (i.e., if !isAgent and
previous?.lastAgentAt is recent/agentRecentlyActive), otherwise update accountId
as before; still update lastAgentAt only when isAgent (Date.now()) or carry
previous.lastAgentAt when not an agent.
- Line 150: Add explicit TypeScript return type annotations to the six new
helper functions to satisfy the repo rule: annotate isModelUnavailableBody with
: boolean; isModelUnavailableResponse with : Promise<boolean>; updateHostLock
with : void; prepareSelection with : Promise<NonNullable<ReturnType<typeof
manager.selectAccount>>>; buildFallbackMessage with : string; and selectFallback
with : { fallback: NonNullable<ReturnType<typeof manager.selectAccount>>;
message: string } | null. Locate the functions by name in copilot-fetch.ts
(isModelUnavailableBody, isModelUnavailableResponse, updateHostLock,
prepareSelection, buildFallbackMessage, selectFallback) and add the specified
return type annotations to their declarations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6787498-9e7f-43ae-b6df-fafcb3a467c1
📒 Files selected for processing (9)
README.mddocs/ARCHITECTURE.mdsrc/accounts/manager.tssrc/fetch/copilot-fetch.tssrc/models/availability.tssrc/observe/usage.tstest/accounts.test.tstest/availability.test.tstest/fetch.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/accounts/manager.ts`:
- Around line 125-130: The new public methods lack explicit return types: add an
explicit ": boolean" return type to isAccountEligible(account: CopilotAccount,
modelId: string, host: string, excludedAccountIds: Set<string> = new Set()) and
annotate selectAccount(...) with ": AccountSelection | null" (the widened
signature around selectAccount in the block roughly corresponding to lines
187-191) so the manager's public API is not inferred; update both function
signatures accordingly and ensure any helper/internal functions keep their
existing types.
In `@src/fetch/copilot-fetch.ts`:
- Around line 150-176: The current heuristic in isModelUnavailableBody is too
permissive; change it so we only classify a miss when the body either (A)
contains the requested modelId (modelId !== 'unknown') together with one of the
error phrases, or (B) contains a stronger model-specific phrase where "model"
appears adjacent to the phrase (e.g. "model not found", "model is not
available", "model unsupported") — implement this by replacing the simple
includes checks with: require modelId presence plus phrase, or test a regex that
matches /model.{0,10}(not found|does not exist|not available|unsupported|no
access)/ (case-insensitive). Keep isModelUnavailableResponse behavior but call
the tightened isModelUnavailableBody; this prevents markModelUnsupported() from
caching unrelated 400/403/404 errors that merely mention "model" or generic "not
found".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2876b2d5-64db-49fa-aee2-6f176e4fbe70
📒 Files selected for processing (4)
src/accounts/manager.tssrc/fetch/copilot-fetch.tstest/accounts.test.tstest/fetch.test.ts
Comment on lines
+125
to
+130
| isAccountEligible( | ||
| account: CopilotAccount, | ||
| modelId: string, | ||
| host: string, | ||
| excludedAccountIds: Set<string> = new Set() | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return types to the new public methods.
isAccountEligible() and the widened selectAccount() signature are now part of the manager’s public surface, but both still rely on inference. Please annotate them explicitly (: boolean and : AccountSelection | null) so future edits cannot silently widen the API.
♻️ 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
Verify each finding against the current code and only fix it if needed.
In `@src/accounts/manager.ts` around lines 125 - 130, The new public methods lack
explicit return types: add an explicit ": boolean" return type to
isAccountEligible(account: CopilotAccount, modelId: string, host: string,
excludedAccountIds: Set<string> = new Set()) and annotate selectAccount(...)
with ": AccountSelection | null" (the widened signature around selectAccount in
the block roughly corresponding to lines 187-191) so the manager's public API is
not inferred; update both function signatures accordingly and ensure any
helper/internal functions keep their existing types.
Comment on lines
+150
to
+176
| function isModelUnavailableBody(bodyText: string, modelId: string): boolean { | ||
| 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): Promise<boolean> { | ||
| if (response.status !== 400 && response.status !== 403 && response.status !== 404) return false; | ||
| const bodyText = await response | ||
| .clone() | ||
| .text() | ||
| .catch(() => ''); | ||
| return isModelUnavailableBody(bodyText, modelId); | ||
| } |
There was a problem hiding this comment.
Tighten the model-unavailable classifier before caching the miss.
Right now any 400/403/404 body that contains the word model and a generic phrase like not found or not available returns true. That will misclassify unrelated errors such as {"error":"not found","model":"gpt-5.4"} and then markModelUnsupported() will wrongly exclude that account for the TTL window. Please require either the requested model id or a stronger model-specific phrase before treating the response as a model miss.
💡 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
Verify each finding against the current code and only fix it if needed.
In `@src/fetch/copilot-fetch.ts` around lines 150 - 176, The current heuristic in
isModelUnavailableBody is too permissive; change it so we only classify a miss
when the body either (A) contains the requested modelId (modelId !== 'unknown')
together with one of the error phrases, or (B) contains a stronger
model-specific phrase where "model" appears adjacent to the phrase (e.g. "model
not found", "model is not available", "model unsupported") — implement this by
replacing the simple includes checks with: require modelId presence plus phrase,
or test a regex that matches /model.{0,10}(not found|does not exist|not
available|unsupported|no access)/ (case-insensitive). Keep
isModelUnavailableResponse behavior but call the tightened
isModelUnavailableBody; this prevents markModelUnsupported() from caching
unrelated 400/403/404 errors that merely mention "model" or generic "not found".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by CodeRabbit
New Features
Documentation
UI / Notifications
Tests