fix: skip --model CLI startup arg when BYOK provider is configured (#305)#306
Merged
Merged
Conversation
) PR #263 added --model <defaultModelID> to copilot.ClientOptions.CLIArgs so the embedded Copilot CLI honors the eval-configured model. However, the CLI validates that startup --model against the GitHub Copilot catalog BEFORE the per-session ProviderConfig from #240 is applied. For BYOK / custom provider model IDs that are not in the Copilot catalog (e.g. minimax-m2.7), startup fails with 'Model "..." is not available'. Suppress the startup --model arg whenever providerFromEnv() reports an enabled custom provider. SessionConfig.Model + SessionConfig.Provider are still passed per session via Execute, so the SDK selects the right model against the custom provider without pre-validating against the GitHub catalog. Normal GitHub Copilot models retain the #263 behavior. Adds a regression test covering the BYOK + non-empty defaultModelID case, and hardens the existing CLIArgs tests against environments that already have COPILOT_BASE_URL set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where BYOK/custom provider model IDs (not present in the GitHub Copilot catalog) cause the embedded Copilot CLI to fail at startup due to an early --model validation step, even though the same model works when applied per-session via SessionConfig + ProviderConfig.
Changes:
- Compute
providerFromEnv()once during engine construction and reuse it for bothengine.providerand startup CLI args. - Extend
modelCLIArgsto skip startup--modelwhenever a custom/BYOK provider is enabled, avoiding pre-session catalog validation failures. - Add a regression test ensuring
CLIArgsis empty when a custom provider is configured, and harden existing CLIArgs tests by clearing provider-related env vars.
Show a summary per file
| File | Description |
|---|---|
| internal/execution/copilot.go | Skips startup --model when a custom provider is enabled; reuses a single providerFromEnv() result. |
| internal/execution/copilot_engine_test.go | Adds regression coverage for BYOK startup behavior and makes CLIArgs tests deterministic wrt env vars. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
…derEnv
Replace t.Setenv(name, "") with os.Unsetenv + t.Cleanup so the helper
actually removes variables from the environment rather than setting them
to empty strings. This matches the comment ("unsets") and avoids any
potential confusion for future readers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…iderEnv Satisfies errcheck in golangci-lint v2.10.1; these calls don't realistically fail in tests and t.Cleanup ensures restoration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jun 1, 2026
Merged
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
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.
Closes #305
Problem
PR #263 added
--model <defaultModelID>tocopilot.ClientOptions.CLIArgsso the embedded Copilot CLI honors the eval-configured model. However, the
Copilot CLI validates that startup
--modelagainst the GitHub Copilotcatalog before the per-session
ProviderConfig(from PR #240) isapplied. For BYOK / custom provider model IDs that are not in the Copilot
catalog (e.g.
minimax-m2.7), startup fails with:even though the same model works fine when supplied as
SessionConfig.ModelwithSessionConfig.Provider.Fix
Suppress the startup
--modelarg wheneverproviderFromEnv()reportsan enabled custom provider.
SessionConfig.Model+SessionConfig.Providerare still passed per session via
Execute, so the SDK selects the rightmodel against the custom provider without pre-validating against the
GitHub catalog. Normal GitHub Copilot models retain the #263 startup
override behavior.
defaultModelIDCLIArgs""[]"claude-..."["--model","claude-..."]"minimax-..."[]✨""[]Changes
internal/execution/copilot.go— computeproviderFromEnv()oncebefore
cliArgs, extendmodelCLIArgsto accept acustomProvider booland skip
--modelwhen true. Reuse the same provider forengine.provider(avoids reading env vars twice).internal/execution/copilot_engine_test.go— addsTestCopilotEngineBuilder_CLIArgsEmptyWhenCustomProvideras theregression test for BYOK provider models fail after startup --model is passed to Copilot CLI #305. Hardens the existing two
CLIArgstests byexplicitly clearing all
COPILOT_*BYOK env vars so they staydeterministic regardless of the surrounding shell.
Validation
go test ./... -count=1— all packages passgo vet ./...— cleanmake lint— golangci-lint not installed locally; CI will run it