-
-
Notifications
You must be signed in to change notification settings - Fork 171
fix: resolve auto model routing failures for GitHub Copilot Free/Student plans #1578
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
Changes from all 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 |
|---|---|---|
|
|
@@ -530,7 +530,7 @@ end | |
| ---@field get_headers nil|fun():table<string, string>,number? | ||
| ---@field get_info nil|fun(headers:table):string[] | ||
| ---@field get_models nil|fun(headers:table):table<CopilotChat.client.Model> | ||
| ---@field resolve_model nil|fun(headers:table, model: string):string | ||
| ---@field resolve_model nil|fun(headers:table, model: string):string, table<string, string>? | ||
| ---@field prepare_input nil|fun(inputs:CopilotChat.client.Message[], opts:CopilotChat.config.providers.Options):table,table? | ||
| ---@field prepare_output nil|fun(output:table, opts:CopilotChat.config.providers.Options):CopilotChat.config.providers.Output | ||
| ---@field get_url nil|fun(opts:CopilotChat.config.providers.Options):string | ||
|
|
@@ -626,6 +626,9 @@ M.copilot = { | |
| -- Build request headers without our internal routing header. | ||
| local request_headers = vim.tbl_extend('force', headers, { ['x-copilot-base-url'] = nil }) | ||
|
|
||
| local auth = headers.Authorization or headers.authorization or '' | ||
| local sku = auth:match('sku=([^;]+)') or 'free' | ||
|
IqbalHossainEmon marked this conversation as resolved.
|
||
|
|
||
| local response, err = curl.get(base_url .. '/models', { | ||
| json_response = true, | ||
| headers = request_headers, | ||
|
|
@@ -635,10 +638,24 @@ M.copilot = { | |
| error(err) | ||
| end | ||
|
|
||
| local function sku_matches_restricted_to(sku, restricted_to) | ||
| if not restricted_to or vim.tbl_isempty(restricted_to) then | ||
| return true | ||
| end | ||
|
|
||
| for _, tier in ipairs(restricted_to) do | ||
| if sku:find(tier, 1, true) then | ||
| return true | ||
| end | ||
| end | ||
|
|
||
| return false | ||
| end | ||
|
|
||
| local models = vim | ||
| .iter(response.body.data) | ||
| :filter(function(model) | ||
| return model.capabilities.type == 'chat' and model.model_picker_enabled | ||
| return model.capabilities.type == 'chat' and sku_matches_restricted_to(sku, model.restricted_to) | ||
|
Contributor
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.
Technical details### ⚠️ Removal of model_picker_enabled may surface non-selectable models
## Affected sites
- `lua/CopilotChat/config/providers.lua:659`
## Required outcome
- The model list contains exactly the models users should be able to select, and doesn't expose models the API explicitly marks as non-selectable.
## Suggested approach
- Verify whether `restricted_to` is authoritative for picker eligibility. If it is, keep the change but add a comment explaining why `model_picker_enabled` is no longer needed.
- If the two fields are independent, combine them: only include chat models that are both picker-enabled *or* explicitly available to the current SKU.
IqbalHossainEmon marked this conversation as resolved.
|
||
| end) | ||
| :map(function(model) | ||
| local supported_endpoints = model.supported_endpoints or {} | ||
|
|
@@ -714,7 +731,7 @@ M.copilot = { | |
| error(err) | ||
| end | ||
|
|
||
| return response.body.selected_model | ||
| return response.body.selected_model, { ['Copilot-Session-Token'] = response.body.session_token } | ||
| end, | ||
|
|
||
| prepare_input = function(inputs, opts) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -557,6 +557,7 @@ function M.ask(prompt, config) | |
| end | ||
|
|
||
| local response = ask_response.message | ||
| local effective_model = response.model or selected_model | ||
| local token_count = ask_response.token_count | ||
| local token_max_count = ask_response.token_max_count | ||
|
|
||
|
|
@@ -632,7 +633,7 @@ function M.ask(prompt, config) | |
| resources = resolved_resources, | ||
| tools = selected_tools, | ||
| system_prompt = system_prompt, | ||
| model = selected_model, | ||
| model = effective_model, | ||
|
Contributor
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.
Technical details### ⚠️ Session token is not reused across continuation turns
## Affected sites
- `lua/CopilotChat/init.lua:560`
- `lua/CopilotChat/init.lua:636`
- `lua/CopilotChat/init.lua:647`
- `lua/CopilotChat/client.lua:325-326`
- `lua/CopilotChat/client.lua:543-545`
## Required outcome
- The `Copilot-Session-Token` returned for `auto` is attached to every request in the same chat turn/conversation, not just the first request.
## Suggested approach
- Cache the session token at the chat level (e.g. alongside the chat state or config) and merge it into each `client:ask` for that conversation, rather than relying solely on `resolve_model` returning it every time.
- Alternatively, special-case `model == 'auto'` so `effective_model` continues to be `'auto'` while a separate resolved id is tracked for the request body.
## Open questions for the human
- Does the `/models/session` token need to be sent with every turn, or only with the first request?
IqbalHossainEmon marked this conversation as resolved.
|
||
| temperature = config.temperature, | ||
| on_progress = vim.schedule_wrap(function(message) | ||
| if not config.headless then | ||
|
|
@@ -643,6 +644,7 @@ function M.ask(prompt, config) | |
|
|
||
| if continue_response then | ||
| local continue_message = continue_response.message | ||
| effective_model = continue_message.model or effective_model | ||
| continue_message.content = vim.trim(continue_message.content) | ||
| if utils.empty(continue_message.content) then | ||
| continue_message.content = '' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sku=...from theAuthorizationheader string is fragile. The copilot provider builds the header as'Bearer ' .. response.body.token, which is not semicolon-delimited, so this match will almost always returnniland default the SKU to'free'. On Business/Enterprise/Pro accounts that silently filters out paid-tier models.Technical details