Address Copilot review feedback for Kimi/Opencode providers
- Allow APIBase-only config for opencode provider selection (like VLLM) - Keep moonshot provider on moonshot.cn/v1 default, only use kimi.com/coding/v1 for kimi/kimi-code - Use url.Parse hostname match for Kimi User-Agent check instead of strings.Contains - Add opencode to DefaultAPIBase test cases in factory_provider_test.go - Add opencode migration tests (full config + APIBase-only) in migration_test.go - Update AllProviders test count to include opencode (18 -> 19) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
ec540312da
commit
9c91d66427
4 changed files with 76 additions and 7 deletions
|
|
@ -132,14 +132,15 @@ func TestConvertProvidersToModelList_AllProviders(t *testing.T) {
|
||||||
Antigravity: ProviderConfig{AuthMethod: "oauth"},
|
Antigravity: ProviderConfig{AuthMethod: "oauth"},
|
||||||
Qwen: ProviderConfig{APIKey: "key17"},
|
Qwen: ProviderConfig{APIKey: "key17"},
|
||||||
Mistral: ProviderConfig{APIKey: "key18"},
|
Mistral: ProviderConfig{APIKey: "key18"},
|
||||||
|
Opencode: ProviderConfig{APIKey: "key19"},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
result := ConvertProvidersToModelList(cfg)
|
result := ConvertProvidersToModelList(cfg)
|
||||||
|
|
||||||
// All 18 providers should be converted
|
// All 19 providers should be converted
|
||||||
if len(result) != 18 {
|
if len(result) != 19 {
|
||||||
t.Errorf("len(result) = %d, want 18", len(result))
|
t.Errorf("len(result) = %d, want 19", len(result))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -551,6 +552,65 @@ func TestBuildModelWithProtocol_DifferentPrefix(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestConvertProvidersToModelList_Opencode(t *testing.T) {
|
||||||
|
cfg := &Config{
|
||||||
|
Providers: ProvidersConfig{
|
||||||
|
Opencode: ProviderConfig{
|
||||||
|
APIKey: "oc-test-key",
|
||||||
|
APIBase: "https://custom.opencode.ai/v1",
|
||||||
|
Proxy: "http://proxy:9090",
|
||||||
|
RequestTimeout: 60,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result := ConvertProvidersToModelList(cfg)
|
||||||
|
|
||||||
|
if len(result) != 1 {
|
||||||
|
t.Fatalf("len(result) = %d, want 1", len(result))
|
||||||
|
}
|
||||||
|
|
||||||
|
mc := result[0]
|
||||||
|
if mc.ModelName != "opencode" {
|
||||||
|
t.Errorf("ModelName = %q, want %q", mc.ModelName, "opencode")
|
||||||
|
}
|
||||||
|
if mc.Model != "opencode/auto" {
|
||||||
|
t.Errorf("Model = %q, want %q", mc.Model, "opencode/auto")
|
||||||
|
}
|
||||||
|
if mc.APIKey != "oc-test-key" {
|
||||||
|
t.Errorf("APIKey = %q, want %q", mc.APIKey, "oc-test-key")
|
||||||
|
}
|
||||||
|
if mc.APIBase != "https://custom.opencode.ai/v1" {
|
||||||
|
t.Errorf("APIBase = %q, want %q", mc.APIBase, "https://custom.opencode.ai/v1")
|
||||||
|
}
|
||||||
|
if mc.Proxy != "http://proxy:9090" {
|
||||||
|
t.Errorf("Proxy = %q, want %q", mc.Proxy, "http://proxy:9090")
|
||||||
|
}
|
||||||
|
if mc.RequestTimeout != 60 {
|
||||||
|
t.Errorf("RequestTimeout = %d, want %d", mc.RequestTimeout, 60)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestConvertProvidersToModelList_Opencode_APIBaseOnly(t *testing.T) {
|
||||||
|
cfg := &Config{
|
||||||
|
Providers: ProvidersConfig{
|
||||||
|
Opencode: ProviderConfig{
|
||||||
|
APIBase: "https://custom.opencode.ai/v1",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result := ConvertProvidersToModelList(cfg)
|
||||||
|
|
||||||
|
if len(result) != 1 {
|
||||||
|
t.Fatalf("len(result) = %d, want 1 (APIBase-only should create entry)", len(result))
|
||||||
|
}
|
||||||
|
|
||||||
|
if result[0].ModelName != "opencode" {
|
||||||
|
t.Errorf("ModelName = %q, want %q", result[0].ModelName, "opencode")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Test for legacy config with protocol prefix in model name
|
// Test for legacy config with protocol prefix in model name
|
||||||
func TestConvertProvidersToModelList_LegacyModelWithProtocolPrefix(t *testing.T) {
|
func TestConvertProvidersToModelList_LegacyModelWithProtocolPrefix(t *testing.T) {
|
||||||
cfg := &Config{
|
cfg := &Config{
|
||||||
|
|
|
||||||
|
|
@ -182,7 +182,7 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
case "opencode":
|
case "opencode":
|
||||||
if cfg.Providers.Opencode.APIKey != "" {
|
if cfg.Providers.Opencode.APIKey != "" || cfg.Providers.Opencode.APIBase != "" {
|
||||||
sel.apiKey = cfg.Providers.Opencode.APIKey
|
sel.apiKey = cfg.Providers.Opencode.APIKey
|
||||||
sel.apiBase = cfg.Providers.Opencode.APIBase
|
sel.apiBase = cfg.Providers.Opencode.APIBase
|
||||||
sel.proxy = cfg.Providers.Opencode.Proxy
|
sel.proxy = cfg.Providers.Opencode.Proxy
|
||||||
|
|
@ -196,7 +196,11 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) {
|
||||||
sel.apiBase = cfg.Providers.Moonshot.APIBase
|
sel.apiBase = cfg.Providers.Moonshot.APIBase
|
||||||
sel.proxy = cfg.Providers.Moonshot.Proxy
|
sel.proxy = cfg.Providers.Moonshot.Proxy
|
||||||
if sel.apiBase == "" {
|
if sel.apiBase == "" {
|
||||||
sel.apiBase = "https://api.kimi.com/coding/v1"
|
if providerName == "moonshot" {
|
||||||
|
sel.apiBase = "https://api.moonshot.cn/v1"
|
||||||
|
} else {
|
||||||
|
sel.apiBase = "https://api.kimi.com/coding/v1"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
case "github_copilot", "copilot":
|
case "github_copilot", "copilot":
|
||||||
|
|
@ -219,7 +223,11 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) {
|
||||||
sel.apiBase = cfg.Providers.Moonshot.APIBase
|
sel.apiBase = cfg.Providers.Moonshot.APIBase
|
||||||
sel.proxy = cfg.Providers.Moonshot.Proxy
|
sel.proxy = cfg.Providers.Moonshot.Proxy
|
||||||
if sel.apiBase == "" {
|
if sel.apiBase == "" {
|
||||||
sel.apiBase = "https://api.kimi.com/coding/v1"
|
if strings.Contains(lowerModel, "moonshot") || strings.HasPrefix(model, "moonshot/") {
|
||||||
|
sel.apiBase = "https://api.moonshot.cn/v1"
|
||||||
|
} else {
|
||||||
|
sel.apiBase = "https://api.kimi.com/coding/v1"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
case strings.HasPrefix(model, "openrouter/") ||
|
case strings.HasPrefix(model, "openrouter/") ||
|
||||||
strings.HasPrefix(model, "anthropic/") ||
|
strings.HasPrefix(model, "anthropic/") ||
|
||||||
|
|
|
||||||
|
|
@ -112,6 +112,7 @@ func TestCreateProviderFromConfig_DefaultAPIBase(t *testing.T) {
|
||||||
{"vllm", "vllm"},
|
{"vllm", "vllm"},
|
||||||
{"deepseek", "deepseek"},
|
{"deepseek", "deepseek"},
|
||||||
{"ollama", "ollama"},
|
{"ollama", "ollama"},
|
||||||
|
{"opencode", "opencode"},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|
|
||||||
|
|
@ -177,7 +177,7 @@ func (p *Provider) Chat(
|
||||||
req.Header.Set("Authorization", "Bearer "+p.apiKey)
|
req.Header.Set("Authorization", "Bearer "+p.apiKey)
|
||||||
}
|
}
|
||||||
// Kimi Code API requires a coding agent User-Agent
|
// Kimi Code API requires a coding agent User-Agent
|
||||||
if strings.Contains(p.apiBase, "api.kimi.com") {
|
if parsedURL, parseErr := url.Parse(p.apiBase); parseErr == nil && parsedURL.Hostname() == "api.kimi.com" {
|
||||||
req.Header.Set("User-Agent", "KimiCLI/0.77")
|
req.Header.Set("User-Agent", "KimiCLI/0.77")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue