Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions internal/cli/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,23 +241,10 @@ func parseProfilePhaseFlag(raw string) (name, phase string, assignment model.Mod
// parseModelSpec parses a "provider/model" or "provider:model" string into a
// ModelAssignment. Returns an error if the spec is empty or has no separator.
func parseModelSpec(spec string) (model.ModelAssignment, error) {
// Try slash separator first (common CLI format: anthropic/claude-haiku-3-5),
// then colon (opencode internal format: anthropic:claude-haiku-3-5).
sep := -1
for i, c := range spec {
if c == '/' || c == ':' {
sep = i
break
}
}
if sep <= 0 {
providerID, modelID, ok := model.SplitProviderModel(spec)
if !ok {
return model.ModelAssignment{}, fmt.Errorf("invalid model spec %q: expected provider/model or provider:model", spec)
}
providerID := spec[:sep]
modelID := spec[sep+1:]
if providerID == "" || modelID == "" {
return model.ModelAssignment{}, fmt.Errorf("invalid model spec %q: provider and model must both be non-empty", spec)
}
return model.ModelAssignment{ProviderID: providerID, ModelID: modelID}, nil
}

Expand Down
14 changes: 2 additions & 12 deletions internal/components/sdd/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,18 +205,8 @@ func extractModelFromAgent(agentMap map[string]any) model.ModelAssignment {
if modelStr == "" {
return model.ModelAssignment{}
}

// Try colon separator first (standard: "anthropic:claude-sonnet-4"), then slash.
idx := strings.Index(modelStr, ":")
if idx <= 0 {
idx = strings.Index(modelStr, "/")
}
if idx <= 0 {
return model.ModelAssignment{}
}
providerID := modelStr[:idx]
modelID := modelStr[idx+1:]
if modelID == "" {
providerID, modelID, ok := model.SplitProviderModel(modelStr)
if !ok {
return model.ModelAssignment{}
}
return model.ModelAssignment{ProviderID: providerID, ModelID: modelID}
Expand Down
69 changes: 69 additions & 0 deletions internal/components/sdd/profiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,75 @@ func TestRemoveProfileAgents_CannotRemoveDefaultByName(t *testing.T) {
}
}

// TestExtractModelFromAgent covers the model-spec parsing path used by
// DetectProfiles. Includes regression coverage for issue #260: OpenRouter
// free models such as "openrouter/qwen/qwen3.6-plus:free" must split at the
// first separator (the leading '/'), not at ':'.
func TestExtractModelFromAgent(t *testing.T) {
tests := []struct {
name string
agentMap map[string]any
want model.ModelAssignment
}{
{
name: "nil map",
agentMap: nil,
want: model.ModelAssignment{},
},
{
name: "missing model field",
agentMap: map[string]any{"prompt": "hello"},
want: model.ModelAssignment{},
},
{
name: "model field not a string",
agentMap: map[string]any{"model": 42},
want: model.ModelAssignment{},
},
{
name: "empty model string",
agentMap: map[string]any{"model": ""},
want: model.ModelAssignment{},
},
{
name: "model with no separator",
agentMap: map[string]any{"model": "no-separator-here"},
want: model.ModelAssignment{},
},
{
name: "colon separator",
agentMap: map[string]any{"model": "anthropic:claude-sonnet-4-20250514"},
want: model.ModelAssignment{ProviderID: "anthropic", ModelID: "claude-sonnet-4-20250514"},
},
{
name: "slash separator",
agentMap: map[string]any{"model": "zai-coding-plan/glm-5-turbo"},
want: model.ModelAssignment{ProviderID: "zai-coding-plan", ModelID: "glm-5-turbo"},
},
// Regression #260: OpenRouter free models contain both '/' and ':'.
// Must split at the first '/' so the colon stays inside the modelID.
{
name: "openrouter free suffix",
agentMap: map[string]any{"model": "openrouter/qwen/qwen3.6-plus:free"},
want: model.ModelAssignment{ProviderID: "openrouter", ModelID: "qwen/qwen3.6-plus:free"},
},
{
name: "openrouter paid with multiple slashes",
agentMap: map[string]any{"model": "openrouter/anthropic/claude-sonnet-4"},
want: model.ModelAssignment{ProviderID: "openrouter", ModelID: "anthropic/claude-sonnet-4"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := extractModelFromAgent(tt.agentMap)
if got != tt.want {
t.Errorf("extractModelFromAgent() = %+v, want %+v", got, tt.want)
}
})
}
}

// helper
func keysOf(m map[string]any) []string {
keys := make([]string, 0, len(m))
Expand Down
16 changes: 2 additions & 14 deletions internal/components/sdd/read_assignments.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package sdd
import (
"encoding/json"
"os"
"strings"

"github.com/gentleman-programming/gentle-ai/internal/model"
"github.com/gentleman-programming/gentle-ai/internal/opencode"
Expand Down Expand Up @@ -77,19 +76,8 @@ func ReadCurrentModelAssignments(settingsPath string) (map[string]model.ModelAss
if !ok || modelStr == "" {
continue
}
// Try colon first (standard: "anthropic:claude-sonnet-4"), then slash
// ("zai-coding-plan/glm-5-turbo") for custom providers (issue #152).
idx := strings.Index(modelStr, ":")
if idx <= 0 {
idx = strings.Index(modelStr, "/")
}
if idx <= 0 {
// No separator or separator is the first character β€” skip malformed value.
continue
}
providerID := modelStr[:idx]
modelID := modelStr[idx+1:]
if modelID == "" {
providerID, modelID, ok := model.SplitProviderModel(modelStr)
if !ok {
continue
}
result[name] = model.ModelAssignment{
Expand Down
50 changes: 50 additions & 0 deletions internal/components/sdd/read_assignments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,53 @@ func TestReadCurrentModelAssignmentsMixedSeparators(t *testing.T) {
}
}
}

// TestReadCurrentModelAssignmentsOpenRouterFreeSuffix is a regression test
// for issue #260: OpenRouter free models have the form
// "openrouter/qwen/qwen3.6-plus:free" β€” they contain BOTH '/' and ':'.
// The parser must split on the FIRST separator (the leading '/'), producing
// providerID="openrouter" and modelID="qwen/qwen3.6-plus:free". Splitting
// on ':' first would mis-attribute "openrouter/qwen/qwen3.6-plus" to the
// provider, which breaks OpenCode model resolution.
func TestReadCurrentModelAssignmentsOpenRouterFreeSuffix(t *testing.T) {
dir := t.TempDir()
settingsPath := filepath.Join(dir, "opencode.json")

content := `{
"agent": {
"sdd-orchestrator": { "model": "openrouter/qwen/qwen3.6-plus:free" },
"sdd-apply": { "model": "openrouter/anthropic/claude-sonnet-4" }
}
}`
if err := os.WriteFile(settingsPath, []byte(content), 0o644); err != nil {
t.Fatalf("write settings: %v", err)
}

got, err := ReadCurrentModelAssignments(settingsPath)
if err != nil {
t.Fatalf("ReadCurrentModelAssignments() error = %v", err)
}

tests := []struct {
phase string
providerID string
modelID string
}{
{"sdd-orchestrator", "openrouter", "qwen/qwen3.6-plus:free"},
{"sdd-apply", "openrouter", "anthropic/claude-sonnet-4"},
}

for _, tt := range tests {
a, ok := got[tt.phase]
if !ok {
t.Errorf("phase %q missing from result", tt.phase)
continue
}
if a.ProviderID != tt.providerID {
t.Errorf("phase %q: ProviderID = %q, want %q", tt.phase, a.ProviderID, tt.providerID)
}
if a.ModelID != tt.modelID {
t.Errorf("phase %q: ModelID = %q, want %q", tt.phase, a.ModelID, tt.modelID)
}
}
}
29 changes: 29 additions & 0 deletions internal/model/split.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package model

// SplitProviderModel splits a model spec string into its provider and model
// components at the FIRST occurrence of '/' or ':'. Splitting on the first
// separator correctly handles compound model identifiers such as OpenRouter
// free models β€” e.g. "openrouter/qwen/qwen3.6-plus:free" is split into
// providerID="openrouter" and modelID="qwen/qwen3.6-plus:free".
//
// Returns (providerID, modelID, true) when both parts are non-empty.
// Returns ("", "", false) when the input is empty, has no separator, starts
// with a separator, or has an empty model part.
func SplitProviderModel(spec string) (providerID, modelID string, ok bool) {
sep := -1
for i, c := range spec {
if c == '/' || c == ':' {
sep = i
break
}
}
if sep <= 0 {
return "", "", false
}
providerID = spec[:sep]
modelID = spec[sep+1:]
if providerID == "" || modelID == "" {
return "", "", false
}
return providerID, modelID, true
}
101 changes: 101 additions & 0 deletions internal/model/split_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package model

import "testing"

func TestSplitProviderModel(t *testing.T) {
tests := []struct {
name string
spec string
wantProv string
wantModel string
wantOK bool
}{
{
name: "colon separator",
spec: "anthropic:claude-sonnet-4-20250514",
wantProv: "anthropic",
wantModel: "claude-sonnet-4-20250514",
wantOK: true,
},
{
name: "slash separator",
spec: "zai-coding-plan/glm-5-turbo",
wantProv: "zai-coding-plan",
wantModel: "glm-5-turbo",
wantOK: true,
},
// Regression #260: OpenRouter free models contain BOTH '/' and ':'.
// The provider is the first segment; the rest (slashes and colons
// included) is the model. Splitting on ':' first would mis-attribute
// "openrouter/qwen/qwen3.6-plus" to the provider.
{
name: "openrouter free suffix β€” slash before colon",
spec: "openrouter/qwen/qwen3.6-plus:free",
wantProv: "openrouter",
wantModel: "qwen/qwen3.6-plus:free",
wantOK: true,
},
{
name: "openrouter paid β€” multiple slashes, no colon",
spec: "openrouter/anthropic/claude-sonnet-4",
wantProv: "openrouter",
wantModel: "anthropic/claude-sonnet-4",
wantOK: true,
},
{
name: "colon appears before slash in model id",
spec: "provider:model/variant",
wantProv: "provider",
wantModel: "model/variant",
wantOK: true,
},
{
name: "empty spec",
spec: "",
wantOK: false,
},
{
name: "no separator",
spec: "just-a-string",
wantOK: false,
},
{
name: "leading slash",
spec: "/model-id",
wantOK: false,
},
{
name: "leading colon",
spec: ":model-id",
wantOK: false,
},
{
name: "trailing slash with empty model",
spec: "provider/",
wantOK: false,
},
{
name: "trailing colon with empty model",
spec: "provider:",
wantOK: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotProv, gotModel, gotOK := SplitProviderModel(tt.spec)
if gotOK != tt.wantOK {
t.Fatalf("ok = %v, want %v", gotOK, tt.wantOK)
}
if !tt.wantOK {
return
}
if gotProv != tt.wantProv {
t.Errorf("providerID = %q, want %q", gotProv, tt.wantProv)
}
if gotModel != tt.wantModel {
t.Errorf("modelID = %q, want %q", gotModel, tt.wantModel)
}
})
}
}
Loading