Skip to content
Closed
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
95 changes: 95 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,32 @@ func TestProviderLookupAndResolveSelectedProvider(t *testing.T) {
}
}

func TestProviderLookupAndResolveSelectedProviderUsesOverride(t *testing.T) {
t.Setenv("CUSTOM_OPENAI_KEY", "override-key")

manager := NewManager(NewLoader(t.TempDir(), testDefaultConfig()))
if _, err := manager.Load(context.Background()); err != nil {
t.Fatalf("Load() error = %v", err)
}
if err := manager.Update(context.Background(), func(cfg *Config) error {
cfg.APIKeyEnvOverride = " CUSTOM_OPENAI_KEY "
return nil
}); err != nil {
t.Fatalf("Update() error = %v", err)
}

resolved, err := manager.ResolvedSelectedProvider()
if err != nil {
t.Fatalf("ResolvedSelectedProvider() error = %v", err)
}
if resolved.APIKeyEnv != "CUSTOM_OPENAI_KEY" {
t.Fatalf("expected resolved env %q, got %q", "CUSTOM_OPENAI_KEY", resolved.APIKeyEnv)
}
if resolved.APIKey != "override-key" {
t.Fatalf("expected resolved key %q, got %q", "override-key", resolved.APIKey)
}
}

func TestLoaderLoadAndSaveRoundTrip(t *testing.T) {
tempDir := t.TempDir()
loader := NewLoader(tempDir, testDefaultConfig())
Expand All @@ -644,6 +670,7 @@ func TestLoaderLoadAndSaveRoundTrip(t *testing.T) {
cfg.CurrentModel = "gpt-5.4"
cfg.Providers[0].Models = []string{"gpt-4.1", "gpt-5.4"}
cfg.Providers[0].BaseURL = "https://ignored.example/v1"
cfg.APIKeyEnvOverride = "CUSTOM_OPENAI_KEY"
cfg.Tools.WebFetch.MaxResponseBytes = 1024
cfg.Tools.WebFetch.SupportedContentTypes = []string{"text/html", "application/json"}
if err := loader.Save(context.Background(), cfg); err != nil {
Expand All @@ -664,11 +691,17 @@ func TestLoaderLoadAndSaveRoundTrip(t *testing.T) {
if strings.Contains(text, "models:") || strings.Contains(text, "base_url:") || strings.Contains(text, "api_key_env:") {
t.Fatalf("expected persisted config to keep only selection state and common runtime settings, got:\n%s", text)
}
if !strings.Contains(text, "api_key_env_override: CUSTOM_OPENAI_KEY") {
t.Fatalf("expected persisted config to include api_key_env_override, got:\n%s", text)
}

reloaded, err := loader.Load(context.Background())
if err != nil {
t.Fatalf("Load() reload error = %v", err)
}
if reloaded.APIKeyEnvOverride != "CUSTOM_OPENAI_KEY" {
t.Fatalf("expected api key env override to round-trip, got %q", reloaded.APIKeyEnvOverride)
}
if reloaded.CurrentModel != "gpt-5.4" {
t.Fatalf("expected current model %q, got %q", "gpt-5.4", reloaded.CurrentModel)
}
Expand All @@ -693,6 +726,68 @@ func TestLoaderLoadAndSaveRoundTrip(t *testing.T) {
}
}

func TestLoaderSaveOmitsEmptyAPIKeyEnvOverride(t *testing.T) {
tempDir := t.TempDir()
loader := NewLoader(tempDir, testDefaultConfig())

cfg, err := loader.Load(context.Background())
if err != nil {
t.Fatalf("Load() error = %v", err)
}
cfg.APIKeyEnvOverride = "CUSTOM_OPENAI_KEY"
if err := loader.Save(context.Background(), cfg); err != nil {
t.Fatalf("Save() with override error = %v", err)
}

cfg.APIKeyEnvOverride = " "
if err := loader.Save(context.Background(), cfg); err != nil {
t.Fatalf("Save() clearing override error = %v", err)
}

data, err := os.ReadFile(loader.ConfigPath())
if err != nil {
t.Fatalf("read config file: %v", err)
}
if strings.Contains(string(data), "api_key_env_override:") {
t.Fatalf("expected empty override to be omitted, got:\n%s", string(data))
}
}

func TestConfigValidateRejectsInvalidAPIKeyEnvOverride(t *testing.T) {
tests := []struct {
name string
override string
expectErr string
}{
{
name: "contains spaces",
override: "CUSTOM KEY",
expectErr: "must not contain whitespace",
},
{
name: "contains equals",
override: "CUSTOM=KEY",
expectErr: "must not contain =",
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

cfg := testDefaultConfig()
cfg.APIKeyEnvOverride = tt.override
cfg.ApplyDefaultsFrom(*testDefaultConfig())

err := cfg.Validate()
if err == nil || !strings.Contains(err.Error(), tt.expectErr) {
t.Fatalf("expected error containing %q, got %v", tt.expectErr, err)
}
})
}
}

func TestLoaderUsesUpdatedBuiltinProviderWhenUserHasNoOverride(t *testing.T) {
tempDir := t.TempDir()

Expand Down
45 changes: 24 additions & 21 deletions internal/config/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ type Loader struct {
}

type persistedConfig struct {
SelectedProvider string `yaml:"selected_provider"`
CurrentModel string `yaml:"current_model"`
Workdir string `yaml:"workdir"`
Shell string `yaml:"shell"`
MaxLoops int `yaml:"max_loops,omitempty"`
ToolTimeoutSec int `yaml:"tool_timeout_sec,omitempty"`
Tools ToolsConfig `yaml:"tools,omitempty"`
SelectedProvider string `yaml:"selected_provider"`
CurrentModel string `yaml:"current_model"`
APIKeyEnvOverride string `yaml:"api_key_env_override,omitempty"`
Workdir string `yaml:"workdir"`
Shell string `yaml:"shell"`
MaxLoops int `yaml:"max_loops,omitempty"`
ToolTimeoutSec int `yaml:"tool_timeout_sec,omitempty"`
Tools ToolsConfig `yaml:"tools,omitempty"`
}

func NewLoader(baseDir string, defaults *Config) *Loader {
Expand Down Expand Up @@ -124,13 +125,14 @@ func (l *Loader) Save(ctx context.Context, cfg *Config) error {
}

file := persistedConfig{
SelectedProvider: snapshot.SelectedProvider,
CurrentModel: snapshot.CurrentModel,
Workdir: snapshot.Workdir,
Shell: snapshot.Shell,
MaxLoops: snapshot.MaxLoops,
ToolTimeoutSec: snapshot.ToolTimeoutSec,
Tools: snapshot.Tools,
SelectedProvider: snapshot.SelectedProvider,
CurrentModel: snapshot.CurrentModel,
APIKeyEnvOverride: snapshot.APIKeyEnvOverride,
Workdir: snapshot.Workdir,
Shell: snapshot.Shell,
MaxLoops: snapshot.MaxLoops,
ToolTimeoutSec: snapshot.ToolTimeoutSec,
Tools: snapshot.Tools,
}

data, err := yaml.Marshal(&file)
Expand Down Expand Up @@ -192,13 +194,14 @@ func parseCurrentConfig(data []byte, _ Config) (*Config, error) {
}

cfg := &Config{
SelectedProvider: strings.TrimSpace(file.SelectedProvider),
CurrentModel: strings.TrimSpace(file.CurrentModel),
Workdir: strings.TrimSpace(file.Workdir),
Shell: strings.TrimSpace(file.Shell),
MaxLoops: file.MaxLoops,
ToolTimeoutSec: file.ToolTimeoutSec,
Tools: file.Tools,
SelectedProvider: strings.TrimSpace(file.SelectedProvider),
CurrentModel: strings.TrimSpace(file.CurrentModel),
APIKeyEnvOverride: strings.TrimSpace(file.APIKeyEnvOverride),
Workdir: strings.TrimSpace(file.Workdir),
Shell: strings.TrimSpace(file.Shell),
MaxLoops: file.MaxLoops,
ToolTimeoutSec: file.ToolTimeoutSec,
Tools: file.Tools,
}

return cfg, nil
Expand Down
3 changes: 2 additions & 1 deletion internal/config/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ func (m *Manager) SelectedProvider() (ProviderConfig, error) {
}

func (m *Manager) ResolvedSelectedProvider() (ResolvedProviderConfig, error) {
provider, err := m.SelectedProvider()
cfg := m.Get()
provider, err := cfg.EffectiveSelectedProviderConfig()
if err != nil {
return ResolvedProviderConfig{}, err
}
Expand Down
76 changes: 68 additions & 8 deletions internal/config/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ var defaultWebFetchSupportedContentTypes = []string{
}

type Config struct {
Providers []ProviderConfig `yaml:"-"`
SelectedProvider string `yaml:"selected_provider"`
CurrentModel string `yaml:"current_model"`
Workdir string `yaml:"workdir"`
Shell string `yaml:"shell"`
MaxLoops int `yaml:"max_loops,omitempty"`
ToolTimeoutSec int `yaml:"tool_timeout_sec,omitempty"`
Tools ToolsConfig `yaml:"tools,omitempty"`
Providers []ProviderConfig `yaml:"-"`
SelectedProvider string `yaml:"selected_provider"`
CurrentModel string `yaml:"current_model"`
APIKeyEnvOverride string `yaml:"api_key_env_override,omitempty"`
Workdir string `yaml:"workdir"`
Shell string `yaml:"shell"`
MaxLoops int `yaml:"max_loops,omitempty"`
ToolTimeoutSec int `yaml:"tool_timeout_sec,omitempty"`
Tools ToolsConfig `yaml:"tools,omitempty"`
}

type ProviderConfig struct {
Expand Down Expand Up @@ -125,6 +126,7 @@ func (c *Config) ApplyDefaultsFrom(defaults Config) {
}
c.Tools.ApplyDefaults(defaults.Tools)

c.APIKeyEnvOverride = normalizeAPIKeyEnvOverride(c.APIKeyEnvOverride)
c.Workdir = normalizeWorkdir(c.Workdir)
}

Expand Down Expand Up @@ -171,6 +173,9 @@ func (c *Config) Validate() error {
if !containsModelID(selected.SupportedModels(), c.CurrentModel) {
return fmt.Errorf("config: current_model %q is not supported by provider %q", c.CurrentModel, selected.Name)
}
if err := validateAPIKeyEnvOverride(c.APIKeyEnvOverride); err != nil {
return fmt.Errorf("config: %w", err)
}
if err := c.Tools.Validate(); err != nil {
return fmt.Errorf("config: tools: %w", err)
}
Expand Down Expand Up @@ -200,6 +205,14 @@ func (c *Config) ProviderByName(name string) (ProviderConfig, error) {
return ProviderConfig{}, fmt.Errorf("config: provider %q not found", name)
}

func (c *Config) EffectiveSelectedProviderConfig() (ProviderConfig, error) {
selected, err := c.SelectedProviderConfig()
if err != nil {
return ProviderConfig{}, err
}
return selected.WithAPIKeyEnvOverride(c.APIKeyEnvOverride)
}

func (p ProviderConfig) Validate() error {
if strings.TrimSpace(p.Name) == "" {
return errors.New("provider name is empty")
Expand Down Expand Up @@ -240,6 +253,33 @@ func (p ProviderConfig) SupportedModels() []string {
return []string{model}
}

func (p ProviderConfig) EffectiveAPIKeyEnv(override string) (string, error) {
if err := validateAPIKeyEnvOverride(override); err != nil {
return "", err
}

if override = normalizeAPIKeyEnvOverride(override); override != "" {
return override, nil
}

envName := strings.TrimSpace(p.APIKeyEnv)
if envName == "" {
return "", fmt.Errorf("provider %q api_key_env is empty", p.Name)
}
return envName, nil
}

func (p ProviderConfig) WithAPIKeyEnvOverride(override string) (ProviderConfig, error) {
envName, err := p.EffectiveAPIKeyEnv(override)
if err != nil {
return ProviderConfig{}, err
}

next := p
next.APIKeyEnv = envName
return next, nil
}

func (p ProviderConfig) ResolveAPIKey() (string, error) {
envName := strings.TrimSpace(p.APIKeyEnv)
if envName == "" {
Expand Down Expand Up @@ -302,6 +342,26 @@ func mergeProviderDefaults(provider ProviderConfig, defaults []ProviderConfig) P
return provider
}

func normalizeAPIKeyEnvOverride(value string) string {
return strings.TrimSpace(value)
}

func validateAPIKeyEnvOverride(value string) error {
value = normalizeAPIKeyEnvOverride(value)
if value == "" {
return nil
}
if strings.Contains(value, "=") {
return errors.New("api_key_env_override must not contain =")
}
for _, r := range value {
if r == ' ' || r == '\t' || r == '\n' || r == '\r' {
return errors.New("api_key_env_override must not contain whitespace")
}
}
return nil
}

func matchDefaultProvider(provider ProviderConfig, defaults []ProviderConfig) (ProviderConfig, bool) {
name := strings.ToLower(strings.TrimSpace(provider.Name))
if name == "" {
Expand Down
55 changes: 55 additions & 0 deletions internal/provider/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package provider_test
import (
"context"
"errors"
"strings"
"testing"

"neo-code/internal/config"
Expand Down Expand Up @@ -220,6 +221,60 @@ func TestServiceSelectProviderAndSetCurrentModel(t *testing.T) {
}
}

func TestServiceSetAPIKeyEnvOverridePersistsAcrossReloadAndSwitches(t *testing.T) {
defaults := builtin.DefaultConfig()
defaults.Providers = append(defaults.Providers, config.ProviderConfig{
Name: "custom-main",
Driver: "custom",
BaseURL: "https://example.com",
Model: "custom-model",
Models: []string{"custom-model", "custom-alt"},
APIKeyEnv: "CUSTOM_PROVIDER_KEY",
})
manager := newTestManagerWithDefaults(t, defaults)
registry := newTestRegistry(t)
if err := registry.Register(stubDriver("custom")); err != nil {
t.Fatalf("register stub driver: %v", err)
}

service := provider.NewService(manager, registry)
if err := service.SetAPIKeyEnvOverride(context.Background(), " GLOBAL_OVERRIDE_KEY "); err != nil {
t.Fatalf("SetAPIKeyEnvOverride() error = %v", err)
}

cfg := manager.Get()
if cfg.APIKeyEnvOverride != "GLOBAL_OVERRIDE_KEY" {
t.Fatalf("expected override to be normalized, got %q", cfg.APIKeyEnvOverride)
}

if _, err := service.SelectProvider(context.Background(), "custom-main"); err != nil {
t.Fatalf("SelectProvider() error = %v", err)
}
if _, err := service.SetCurrentModel(context.Background(), "custom-alt"); err != nil {
t.Fatalf("SetCurrentModel() error = %v", err)
}

reloaded, err := manager.Reload(context.Background())
if err != nil {
t.Fatalf("Reload() error = %v", err)
}
if reloaded.APIKeyEnvOverride != "GLOBAL_OVERRIDE_KEY" {
t.Fatalf("expected override to persist after reload, got %q", reloaded.APIKeyEnvOverride)
}
if reloaded.SelectedProvider != "custom-main" || reloaded.CurrentModel != "custom-alt" {
t.Fatalf("expected provider/model selection to persist, got provider=%q model=%q", reloaded.SelectedProvider, reloaded.CurrentModel)
}
}

func TestServiceSetAPIKeyEnvOverrideRejectsInvalidInput(t *testing.T) {
service := provider.NewService(newTestManager(t), newTestRegistry(t))

err := service.SetAPIKeyEnvOverride(context.Background(), "CUSTOM KEY")
if err == nil || (!strings.Contains(err.Error(), "whitespace") && !strings.Contains(err.Error(), "api_key_env_override")) {
t.Fatalf("expected override validation error, got %v", err)
}
}

func TestServiceModelOperationsUseProviderConfigEvenWithoutDriver(t *testing.T) {
t.Parallel()

Expand Down
Loading