Skip to content

Commit 8b07887

Browse files
committed
perf: provider config and unify error display
1 parent 197c153 commit 8b07887

File tree

10 files changed

+246
-77
lines changed

10 files changed

+246
-77
lines changed

cmd/codebot/main.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77

8+
"github.com/voocel/codebot/internal/apperr"
89
"github.com/voocel/codebot/internal/bootstrap"
910
"github.com/voocel/codebot/internal/config"
1011
"github.com/voocel/codebot/internal/ui"
@@ -40,14 +41,14 @@ func main() {
4041
ApprovalMode: *modeFlag,
4142
})
4243
if err != nil {
43-
fmt.Fprintf(os.Stderr, "boot error: %v\n", err)
44+
fmt.Fprintln(os.Stderr, formatBootError(err))
4445
os.Exit(1)
4546
}
4647
defer rt.Close()
4748

4849
if printMode {
4950
if err := ui.RunPrint(rt.Session, flag.Args(), *jsonFlag); err != nil {
50-
fmt.Fprintf(os.Stderr, "error: %v\n", err)
51+
fmt.Fprintln(os.Stderr, formatCLIError(err))
5152
os.Exit(1)
5253
}
5354
return
@@ -58,7 +59,15 @@ func main() {
5859
modelName = rt.Session.ModelName()
5960
}
6061
if err := ui.RunTUI(rt.Session, rt.Cwd, rt.GitBranch, modelName, version, rt.ApprovalEngine, rt.TaskRuntime, rt.MCPManager, rt.MCPServers, rt.PluginCatalog, rt.SkillCatalog, rt.EnvHint, rt.SessionStore, rt.PlanSlug, rt.PlanTitle, rt.PlanPhase, rt.PlanPreMode, rt.PlanAllowedCommands); err != nil {
61-
fmt.Fprintf(os.Stderr, "error: %v\n", err)
62+
fmt.Fprintln(os.Stderr, formatCLIError(err))
6263
os.Exit(1)
6364
}
6465
}
66+
67+
func formatBootError(err error) string {
68+
return apperr.Format(err, "boot error")
69+
}
70+
71+
func formatCLIError(err error) string {
72+
return apperr.Format(err, "error")
73+
}

internal/agent/session_runtime.go

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,12 @@ func (s *Session) resolveModelOverride(pattern string) (string, string, agentcor
378378

379379
if strings.Contains(pattern, "/") {
380380
if prov, model, ok := strings.Cut(pattern, "/"); ok {
381+
provType, err := s.providerType(prov)
382+
if err != nil {
383+
return "", "", nil, err
384+
}
381385
apiKey, baseURL := s.resolveCredentials(prov)
382-
chatModel, err := s.createModel(s.providerType(prov), model, apiKey, baseURL)
386+
chatModel, err := s.createModel(provType, model, apiKey, baseURL)
383387
if err == nil {
384388
return prov, model, chatModel, nil
385389
}
@@ -401,12 +405,20 @@ func (s *Session) resolveModelOverride(pattern string) (string, string, agentcor
401405
switch len(matches) {
402406
case 1:
403407
m := matches[0]
408+
provType, err := s.providerType(m.provider)
409+
if err != nil {
410+
return "", "", nil, err
411+
}
404412
apiKey, baseURL := s.resolveCredentials(m.provider)
405-
chatModel, err := s.createModel(s.providerType(m.provider), m.model, apiKey, baseURL)
413+
chatModel, err := s.createModel(provType, m.model, apiKey, baseURL)
406414
return m.provider, m.model, chatModel, err
407415
case 0:
416+
provType, err := s.providerType(curProv)
417+
if err != nil {
418+
return "", "", nil, err
419+
}
408420
apiKey, baseURL := s.resolveCredentials(curProv)
409-
chatModel, err := s.createModel(s.providerType(curProv), pattern, apiKey, baseURL)
421+
chatModel, err := s.createModel(provType, pattern, apiKey, baseURL)
410422
if err != nil {
411423
return "", "", nil, err
412424
}
@@ -437,7 +449,10 @@ func (s *Session) reclampThinkingTemporary() {
437449
}
438450

439451
func (s *Session) SetModel(prov, model string) error {
440-
provType := s.providerType(prov)
452+
provType, err := s.providerType(prov)
453+
if err != nil {
454+
return err
455+
}
441456
apiKey, baseURL := s.resolveCredentials(prov)
442457
s.mu.Lock()
443458
store := s.store
@@ -500,17 +515,14 @@ func (s *Session) SetModel(prov, model string) error {
500515
}
501516

502517
// providerType returns the protocol type for a provider key.
503-
func (s *Session) providerType(prov string) string {
518+
func (s *Session) providerType(prov string) (string, error) {
504519
s.mu.Lock()
505520
pc, ok := s.providers[prov]
506521
s.mu.Unlock()
507522
if ok {
508523
return pc.ProviderType(prov)
509524
}
510-
if t, ok := config.KnownProviderTypes[prov]; ok {
511-
return t
512-
}
513-
return "openai"
525+
return config.ResolveProviderType(prov, "")
514526
}
515527

516528
func (s *Session) ResolveAndSetModel(pattern string) (string, error) {
@@ -583,13 +595,20 @@ func (s *Session) updateContextFromRegistry(providerKey, modelID string) {
583595
return
584596
}
585597
// Try provider-qualified lookup using the protocol type.
586-
provType := s.providerType(providerKey)
587-
entry, _, err := s.registry.Resolve(provType + "/" + modelID)
588-
if err != nil {
589-
// Fallback to bare model ID for custom providers.
590-
entry, _, err = s.registry.Resolve(modelID)
598+
provType, err := s.providerType(providerKey)
599+
if err == nil {
600+
entry, _, err := s.registry.Resolve(provType + "/" + modelID)
601+
if err == nil && entry.ContextWindow > 0 {
602+
s.agent.SetContextWindow(entry.ContextWindow)
603+
s.mu.Lock()
604+
s.settings.ContextWindow = entry.ContextWindow
605+
s.mu.Unlock()
606+
return
607+
}
591608
}
609+
entry, _, err := s.registry.Resolve(modelID)
592610
if err != nil || entry.ContextWindow <= 0 {
611+
// Fallback to bare model ID for custom providers.
593612
return
594613
}
595614
s.agent.SetContextWindow(entry.ContextWindow)
@@ -756,7 +775,10 @@ func (s *Session) SwitchSession(id string) error {
756775

757776
var restoredModel agentcore.ChatModel
758777
if snapshot.Model != "" || snapshot.Provider != "" {
759-
targetType := s.providerType(targetProvider)
778+
targetType, err := s.providerType(targetProvider)
779+
if err != nil {
780+
return err
781+
}
760782
restoredModel, err = s.createModel(targetType, targetModel, targetKey, targetBase)
761783
if err != nil {
762784
return fmt.Errorf("restore model %s/%s: %w", targetProvider, targetModel, err)

internal/apperr/errors.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package apperr
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
)
7+
8+
// UserError exposes a concise user-facing message while preserving the full
9+
// wrapped error for logging and debugging.
10+
type UserError interface {
11+
error
12+
DisplayMessage() string
13+
}
14+
15+
type Error struct {
16+
Display string
17+
Err error
18+
}
19+
20+
func (e *Error) Error() string {
21+
if e == nil {
22+
return ""
23+
}
24+
if e.Err == nil {
25+
return e.Display
26+
}
27+
return e.Display + ": " + e.Err.Error()
28+
}
29+
30+
func (e *Error) Unwrap() error {
31+
if e == nil {
32+
return nil
33+
}
34+
return e.Err
35+
}
36+
37+
func (e *Error) DisplayMessage() string {
38+
if e == nil {
39+
return ""
40+
}
41+
return e.Display
42+
}
43+
44+
func New(display string) error {
45+
return &Error{Display: display}
46+
}
47+
48+
func Newf(format string, args ...any) error {
49+
return &Error{Display: fmt.Sprintf(format, args...)}
50+
}
51+
52+
func Wrap(display string, err error) error {
53+
return &Error{Display: display, Err: err}
54+
}
55+
56+
func Format(err error, fallbackPrefix string) string {
57+
if err == nil {
58+
return ""
59+
}
60+
var display UserError
61+
if errors.As(err, &display) {
62+
return display.DisplayMessage()
63+
}
64+
if fallbackPrefix == "" {
65+
return err.Error()
66+
}
67+
return fallbackPrefix + ": " + err.Error()
68+
}

internal/bootstrap/assemble_session.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,9 @@ func resolveActiveModel(input *resolvedInput) (config.Resolved, string, agentcor
7878
}
7979

8080
activeAPIKey, activeBaseURL := input.settings.ProviderCredentials(activeProvider)
81-
provType := "openai"
82-
if pc, ok := input.settings.Providers[activeProvider]; ok {
83-
provType = pc.ProviderType(activeProvider)
84-
} else if t, ok := config.KnownProviderTypes[activeProvider]; ok {
85-
provType = t
81+
provType, err := config.ResolveConfiguredProviderType(input.settings.Providers, activeProvider)
82+
if err != nil {
83+
return config.Resolved{}, "", nil, err
8684
}
8785
chatModel, err := input.modelFactory(provType, activeModel, activeAPIKey, activeBaseURL)
8886
if err != nil {

internal/bootstrap/boot_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,32 @@ func TestResolveInputEnvProviderDoesNotCreateSettingsFile(t *testing.T) {
103103
t.Fatalf("project settings file should not be created, stat err = %v", err)
104104
}
105105
}
106+
107+
func TestBootCustomProviderRequiresExplicitType(t *testing.T) {
108+
t.Setenv("HOME", t.TempDir())
109+
110+
cwd := t.TempDir()
111+
if err := os.MkdirAll(filepath.Join(cwd, ".codebot"), 0o755); err != nil {
112+
t.Fatal(err)
113+
}
114+
data := `{
115+
"provider": "myproxy",
116+
"model": "gpt-4o-mini",
117+
"providers": {
118+
"myproxy": {
119+
"api_key": "test-key"
120+
}
121+
}
122+
}`
123+
if err := os.WriteFile(filepath.Join(cwd, ".codebot", "settings.json"), []byte(data), 0o600); err != nil {
124+
t.Fatal(err)
125+
}
126+
127+
_, err := Boot(Options{Cwd: cwd, NonTTYMode: true})
128+
if err == nil {
129+
t.Fatal("expected boot error for custom provider without type")
130+
}
131+
if err.Error() != `configuration error: providers.myproxy.type is required for custom providers` {
132+
t.Fatalf("unexpected error: %v", err)
133+
}
134+
}

internal/bootstrap/input.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212

1313
"github.com/voocel/codebot/internal/agent"
14+
"github.com/voocel/codebot/internal/apperr"
1415
"github.com/voocel/codebot/internal/approval"
1516
"github.com/voocel/codebot/internal/config"
1617
"github.com/voocel/codebot/internal/provider"
@@ -97,7 +98,7 @@ func ensureProviderSetup(cwd string, settings config.Resolved, nonTTY bool) (con
9798
if hasConfiguredProviderCredentials(settings, settings.Provider) {
9899
return settings, "", nil
99100
}
100-
return settings, "", fmt.Errorf("configuration error: settings.provider=%q is missing or not configured in settings.json", settings.Provider)
101+
return settings, "", apperr.Newf("configuration error: settings.provider=%q is missing or not configured in settings.json", settings.Provider)
101102
}
102103

103104
apiKey, _ := settings.ProviderCredentials(settings.Provider)

internal/bootstrap/subagents.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@ func buildSubAgentTool(deps subAgentDeps) *agentcore.SubAgentTool {
3333
if deps.SmallModel != "" && deps.CreateModel != nil {
3434
prov := deps.Provider
3535
apiKey, baseURL := resolveFromProviders(deps.Providers, prov)
36-
provType := resolveProviderType(deps.Providers, prov)
37-
if m, err := deps.CreateModel(provType, deps.SmallModel, apiKey, baseURL); err == nil {
38-
exploreModel = m
36+
provType, err := resolveProviderType(deps.Providers, prov)
37+
if err == nil {
38+
if m, err := deps.CreateModel(provType, deps.SmallModel, apiKey, baseURL); err == nil {
39+
exploreModel = m
40+
}
3941
}
4042
}
4143

@@ -100,7 +102,10 @@ func buildSubAgentTool(deps subAgentDeps) *agentcore.SubAgentTool {
100102
}
101103
}
102104
apiKey, baseURL := resolveFromProviders(providers, prov)
103-
provType := resolveProviderType(providers, prov)
105+
provType, err := resolveProviderType(providers, prov)
106+
if err != nil {
107+
return nil, err
108+
}
104109
return factory(provType, name, apiKey, baseURL)
105110
})
106111
}
@@ -137,14 +142,8 @@ func resolveFromProviders(providers map[string]config.ProviderConfig, prov strin
137142
}
138143

139144
// resolveProviderType returns the protocol type for a provider key.
140-
func resolveProviderType(providers map[string]config.ProviderConfig, prov string) string {
141-
if pc, ok := providers[prov]; ok {
142-
return pc.ProviderType(prov)
143-
}
144-
if t, ok := config.KnownProviderTypes[prov]; ok {
145-
return t
146-
}
147-
return "openai"
145+
func resolveProviderType(providers map[string]config.ProviderConfig, prov string) (string, error) {
146+
return config.ResolveConfiguredProviderType(providers, prov)
148147
}
149148

150149
// readOnlyTools constructs a read-only tool set for explore/plan sub-agents.

internal/config/settings.go

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import (
77
"path/filepath"
88
"regexp"
99
"strings"
10+
11+
"github.com/voocel/codebot/internal/apperr"
12+
"github.com/voocel/codebot/internal/provider"
1013
)
1114

1215
// ConfigDir is the project-level config directory name.
@@ -22,24 +25,42 @@ var KnownProviderTypes = map[string]string{
2225

2326
// ProviderConfig holds credentials and model configuration for a single provider.
2427
type ProviderConfig struct {
25-
Type string `json:"type,omitempty"` // protocol: openai/anthropic/gemini; inferred from name if empty
28+
Type string `json:"type,omitempty"` // LiteLLM provider type: openai/anthropic/gemini/openrouter; required for custom providers
2629
APIKey string `json:"api_key,omitempty"`
2730
BaseURL string `json:"base_url,omitempty"`
2831
Models []string `json:"models,omitempty"` // available model list for this provider
2932
SmallModel string `json:"small_model,omitempty"` // lightweight model for sub-agents
3033
}
3134

32-
// ProviderType returns the protocol type for this provider.
33-
// Uses explicit Type field first, then infers from the provider name,
34-
// defaulting to "openai" for unknown providers.
35-
func (pc ProviderConfig) ProviderType(name string) string {
36-
if pc.Type != "" {
37-
return pc.Type
35+
// ProviderType resolves the protocol type for this provider.
36+
// Built-in providers infer their type from the provider name.
37+
// Custom providers must declare providers.<name>.type explicitly.
38+
func (pc ProviderConfig) ProviderType(name string) (string, error) {
39+
return ResolveProviderType(name, pc.Type)
40+
}
41+
42+
// ResolveProviderType resolves a provider's protocol type.
43+
// The type value maps directly to the underlying LiteLLM provider name.
44+
func ResolveProviderType(name, explicitType string) (string, error) {
45+
provType := strings.ToLower(strings.TrimSpace(explicitType))
46+
if provType != "" {
47+
if provider.IsSupportedType(provType) {
48+
return provType, nil
49+
}
50+
return "", apperr.Newf("configuration error: providers.%s.type=%q is unsupported", name, explicitType)
3851
}
3952
if t, ok := KnownProviderTypes[name]; ok {
40-
return t
53+
return t, nil
54+
}
55+
return "", apperr.Newf("configuration error: providers.%s.type is required for custom providers", name)
56+
}
57+
58+
// ResolveConfiguredProviderType resolves the protocol type for a configured provider.
59+
func ResolveConfiguredProviderType(providers map[string]ProviderConfig, name string) (string, error) {
60+
if pc, ok := providers[name]; ok {
61+
return pc.ProviderType(name)
4162
}
42-
return "openai"
63+
return ResolveProviderType(name, "")
4364
}
4465

4566
// HookEntry describes a single hook.
@@ -481,7 +502,7 @@ func loadSettingsFileStrict(path string) (Settings, error) {
481502
return s, err
482503
}
483504
if err := json.Unmarshal(data, &s); err != nil {
484-
return s, fmt.Errorf("malformed %s: %w", path, err)
505+
return s, apperr.Wrap("configuration error: malformed settings.json", fmt.Errorf("%s: %w", path, err))
485506
}
486507
return s, nil
487508
}

0 commit comments

Comments
 (0)