mirror of
https://github.com/Heretek-AI/open-code-review.git
synced 2026-11-07 10:56:07 -10:00
fix(config): fix bad config for providers or keys that cotains dots (self-defined) (#1661)
* main fix * chore: write tests (AI) * fix(config): validate dotted provider names across CLI and IDEs --------- Co-authored-by: Kite <[email protected]>
This commit is contained in:
1 parent
11ac6d4c57
commit
8eb83340a4
19 files changed
+433
-51
No files matched your search
@@ -641,6 +641,11 @@ func setConfigValue(cfg *Config, key, value string) error {
|
||||
|
||||
switch key {
|
||||
case "provider":
|
||||
if value != "" {
|
||||
if err := validateProviderName(value); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if cfg.Provider != value {
|
||||
cfg.Model = ""
|
||||
}
|
||||
@@ -995,33 +1000,35 @@ func ensureModelInList(models []string, model string) []string {
|
||||
}
|
||||
|
||||
func setProviderValue(cfg *Config, key, value string) error {
|
||||
parts := strings.SplitN(key, ".", 3)
|
||||
if len(parts) != 3 || parts[1] == "" || parts[2] == "" {
|
||||
dot := strings.LastIndex(key, ".")
|
||||
if dot <= len("providers.") || dot == len(key)-1 {
|
||||
return fmt.Errorf("invalid provider key %q: expected providers.<name>.<field>", key)
|
||||
}
|
||||
if _, isPreset := llm.LookupProvider(parts[1]); !isPreset {
|
||||
return setCustomProviderField(cfg, parts[1], parts[2], key, value)
|
||||
name, field := key[len("providers."):dot], key[dot+1:]
|
||||
if _, isPreset := llm.LookupProvider(name); !isPreset {
|
||||
return setCustomProviderField(cfg, name, field, key, value)
|
||||
}
|
||||
entry := cfg.Providers[name]
|
||||
if err := applyProviderField(name, &entry, field, key, value); err != nil {
|
||||
return err
|
||||
}
|
||||
if cfg.Providers == nil {
|
||||
cfg.Providers = make(map[string]ProviderEntry)
|
||||
}
|
||||
entry := cfg.Providers[parts[1]]
|
||||
if err := applyProviderField(parts[1], &entry, parts[2], key, value); err != nil {
|
||||
return err
|
||||
}
|
||||
cfg.Providers[parts[1]] = entry
|
||||
cfg.Providers[name] = entry
|
||||
return nil
|
||||
}
|
||||
|
||||
func setCustomProviderValue(cfg *Config, key, value string) error {
|
||||
parts := strings.SplitN(key, ".", 3)
|
||||
if len(parts) != 3 || parts[1] == "" || parts[2] == "" {
|
||||
dot := strings.LastIndex(key, ".")
|
||||
if dot <= len("custom_providers.") || dot == len(key)-1 {
|
||||
return fmt.Errorf("invalid custom provider key %q: expected custom_providers.<name>.<field>", key)
|
||||
}
|
||||
if preset, isPreset := llm.LookupProvider(parts[1]); isPreset {
|
||||
return fmt.Errorf("custom provider name %q conflicts with a preset provider; use providers.%s.%s to configure the preset or choose a different custom provider name", parts[1], preset.Name, parts[2])
|
||||
name, field := key[len("custom_providers."):dot], key[dot+1:]
|
||||
if preset, isPreset := llm.LookupProvider(name); isPreset {
|
||||
return fmt.Errorf("custom provider name %q conflicts with a preset provider; use providers.%s.%s to configure the preset or choose a different custom provider name", name, preset.Name, field)
|
||||
}
|
||||
return setCustomProviderField(cfg, parts[1], parts[2], key, value)
|
||||
return setCustomProviderField(cfg, name, field, key, value)
|
||||
}
|
||||
|
||||
func isAuxiliaryProviderField(field string) bool {
|
||||
@@ -1033,18 +1040,33 @@ func isAuxiliaryProviderField(field string) bool {
|
||||
}
|
||||
}
|
||||
|
||||
func validateProviderName(name string) error {
|
||||
for _, segment := range strings.Split(name, ".") {
|
||||
if strings.TrimSpace(segment) == "" {
|
||||
return fmt.Errorf("invalid provider name %q: dot-separated segments must not be empty or whitespace", name)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func setCustomProviderField(cfg *Config, name, field, key, value string) error {
|
||||
if err := validateProviderName(name); err != nil {
|
||||
return fmt.Errorf("%s: %w", key, err)
|
||||
}
|
||||
if _, exists := cfg.CustomProviders[name]; strings.Contains(name, ".") && !exists {
|
||||
return fmt.Errorf("provider %q is not configured for key %q; create or select it first:\n ocr config set provider %s", name, key, name)
|
||||
}
|
||||
if _, exists := cfg.CustomProviders[name]; isAuxiliaryProviderField(field) && !exists {
|
||||
providerKey := strings.TrimSuffix(key, "."+field)
|
||||
return fmt.Errorf("provider %q is not configured; set a core field first (protocol is required for every custom provider):\n ocr config set %s.protocol <protocol>", name, providerKey)
|
||||
}
|
||||
if cfg.CustomProviders == nil {
|
||||
cfg.CustomProviders = make(map[string]ProviderEntry)
|
||||
}
|
||||
entry := cfg.CustomProviders[name]
|
||||
if err := applyProviderField(name, &entry, field, key, value); err != nil {
|
||||
return err
|
||||
}
|
||||
if cfg.CustomProviders == nil {
|
||||
cfg.CustomProviders = make(map[string]ProviderEntry)
|
||||
}
|
||||
cfg.CustomProviders[name] = entry
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -220,6 +220,118 @@ func TestSetConfigValueProviderEntryNonPresetWritesCustomProvider(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetConfigValueDottedProviderName(t *testing.T) {
|
||||
fields := []struct {
|
||||
field string
|
||||
value string
|
||||
}{
|
||||
{"url", "https://example.invalid/v1"},
|
||||
{"protocol", llm.ProtocolAnthropicBedrock},
|
||||
{"model", "some-model"},
|
||||
{"api_key", "test-key"},
|
||||
{"api_key_cmd", "secret-tool lookup service x"},
|
||||
{"models", `["some-model","other-model"]`},
|
||||
{"auth_header", "authorization"},
|
||||
{"timeout_sec", "120"},
|
||||
{"extra_body", `{"temperature":0.2}`},
|
||||
{"extra_headers", "X-Gateway=test"},
|
||||
{"retry_codes", "400,403"},
|
||||
{"aws_region", "us-west-2"},
|
||||
{"aws_profile", "test-profile"},
|
||||
}
|
||||
want := ProviderEntry{
|
||||
URL: "https://example.invalid/v1",
|
||||
Protocol: llm.ProtocolAnthropicBedrock,
|
||||
Model: "some-model",
|
||||
APIKey: "test-key",
|
||||
APIKeyCmd: "secret-tool lookup service x",
|
||||
Models: []string{"some-model", "other-model"},
|
||||
AuthHeader: "authorization",
|
||||
TimeoutSec: 120,
|
||||
ExtraBody: map[string]any{"temperature": 0.2},
|
||||
ExtraHeaders: map[string]string{"X-Gateway": "test"},
|
||||
RetryCodes: []int{400, 403},
|
||||
AWSRegion: "us-west-2",
|
||||
AWSProfile: "test-profile",
|
||||
}
|
||||
for _, prefix := range []string{"providers", "custom_providers"} {
|
||||
for _, name := range []string{"mimo-v2.6-flash", "openai.gateway.v2", "foo.api_key"} {
|
||||
t.Run(prefix+"."+name, func(t *testing.T) {
|
||||
cfg := &Config{}
|
||||
if err := setConfigValue(cfg, "provider", name); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, tt := range fields {
|
||||
key := prefix + "." + name + "." + tt.field
|
||||
if err := setConfigValue(cfg, key, tt.value); err != nil {
|
||||
t.Fatalf("setConfigValue(%q): %v", key, err)
|
||||
}
|
||||
}
|
||||
if !reflect.DeepEqual(cfg.CustomProviders, map[string]ProviderEntry{name: want}) {
|
||||
t.Errorf("CustomProviders = %#v, want only %q with %#v", cfg.CustomProviders, name, want)
|
||||
}
|
||||
if len(cfg.Providers) != 0 || cfg.Provider != name {
|
||||
t.Errorf("Providers = %#v, Provider = %q", cfg.Providers, cfg.Provider)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetConfigValueRejectsInvalidProviderNames(t *testing.T) {
|
||||
for _, name := range []string{" ", ".a", "a.", "a..b", "a. .b", "anthropic."} {
|
||||
for _, key := range []string{"provider", "providers." + name + ".url", "custom_providers." + name + ".url"} {
|
||||
t.Run(key+"/"+name, func(t *testing.T) {
|
||||
cfg := &Config{Provider: "openai", Model: "saved-model", CustomProviders: map[string]ProviderEntry{name: {}}}
|
||||
before, err := json.Marshal(cfg)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
value := "https://example.invalid/v1"
|
||||
if key == "provider" {
|
||||
value = name
|
||||
}
|
||||
if err := setConfigValue(cfg, key, value); err == nil || !strings.Contains(err.Error(), "invalid provider name") {
|
||||
t.Fatalf("setConfigValue(%q, %q) = %v, want invalid provider name", key, value, err)
|
||||
}
|
||||
after, err := json.Marshal(cfg)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(after) != string(before) {
|
||||
t.Fatalf("configuration changed after rejected write: %s", after)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetConfigValueDottedProviderRequiresExistingEntry(t *testing.T) {
|
||||
for _, prefix := range []string{"providers", "custom_providers"} {
|
||||
for _, name := range []string{"mimo-v2.6-flash", "foo.api_key", "openai.extra_body"} {
|
||||
key := prefix + "." + name + ".model"
|
||||
t.Run(key, func(t *testing.T) {
|
||||
cfg := &Config{Provider: "openai", Model: "saved-model"}
|
||||
want := *cfg
|
||||
err := setConfigValue(cfg, key, "some-model")
|
||||
if err == nil || !strings.Contains(err.Error(), key) || !strings.Contains(err.Error(), "ocr config set provider "+name) {
|
||||
t.Fatalf("setConfigValue(%q) = %v, want explicit creation guidance", key, err)
|
||||
}
|
||||
if !reflect.DeepEqual(*cfg, want) {
|
||||
t.Fatalf("configuration changed after rejected write: %#v", cfg)
|
||||
}
|
||||
cfg.CustomProviders = map[string]ProviderEntry{name: {}}
|
||||
if err := setConfigValue(cfg, key, "some-model"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if cfg.Provider != "openai" || cfg.CustomProviders[name].Model != "some-model" {
|
||||
t.Fatalf("unexpected configuration: %#v", cfg)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetConfigValueProviderEntryModelsJSON(t *testing.T) {
|
||||
cfg := &Config{}
|
||||
|
||||
@@ -364,10 +476,15 @@ func TestSetConfigValueProviderEntryInvalidKey(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestSetConfigValueProviderEntryInvalidPath(t *testing.T) {
|
||||
cfg := &Config{}
|
||||
|
||||
if err := setConfigValue(cfg, "providers.anthropic", "value"); err == nil {
|
||||
t.Fatal("expected error for incomplete provider path")
|
||||
for _, prefix := range []string{"providers", "custom_providers"} {
|
||||
for _, suffix := range []string{"", "anthropic", ".url", "anthropic.", "mimo-v2.6-flash."} {
|
||||
key := prefix + "." + suffix
|
||||
t.Run(key, func(t *testing.T) {
|
||||
if err := setConfigValue(&Config{}, key, "value"); err == nil || !strings.Contains(err.Error(), "invalid") {
|
||||
t.Fatalf("setConfigValue(%q) = %v, want invalid path error", key, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,12 +4,79 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestRunConfigSetProviderNames(t *testing.T) {
|
||||
if os.Getenv("OCR_TEST_CONFIG_SET") == "1" {
|
||||
rootCmd.SetArgs(os.Args[3:])
|
||||
os.Exit(run())
|
||||
}
|
||||
setTestHome(t, t.TempDir())
|
||||
configPath, err := defaultConfigPath()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := saveConfig(configPath, &Config{Provider: "openai", Model: "saved-model"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tests := []struct {
|
||||
key string
|
||||
value string
|
||||
code int
|
||||
}{
|
||||
{"providers.openai.extra_body.model", "some-model", 1},
|
||||
{"custom_providers.openai.extra_body.model", "some-model", 1},
|
||||
{"providers.openai..url", "https://example.invalid/v1", 1},
|
||||
{"providers..a.url", "https://example.invalid/v1", 1},
|
||||
{"providers. .url", "https://example.invalid/v1", 1},
|
||||
{"custom_providers.anthropic..url", "https://example.invalid/v1", 1},
|
||||
{"provider", ".gateway", 1},
|
||||
{"provider", "gateway.", 1},
|
||||
{"provider", "gateway..v2", 1},
|
||||
{"provider", " ", 1},
|
||||
{"provider", "mimo-v2.6-flash", 0},
|
||||
{"providers.mimo-v2.6-flash.url", "https://example.invalid/v1", 0},
|
||||
{"custom_providers.mimo-v2.6-flash.protocol", "openai", 0},
|
||||
{"provider", "foo.api_key", 0},
|
||||
{"custom_providers.foo.api_key.model", "some-model", 0},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.key+"/"+tt.value, func(t *testing.T) {
|
||||
before, err := os.ReadFile(configPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cmd := exec.Command(os.Args[0], "-test.run=^TestRunConfigSetProviderNames$", "--", "config", "set", tt.key, tt.value)
|
||||
cmd.Env = append(os.Environ(), "OCR_TEST_CONFIG_SET=1", "OCR_NO_UPDATE=1", "OCR_ENABLE_TELEMETRY=0")
|
||||
output, err := cmd.CombinedOutput()
|
||||
if cmd.ProcessState == nil || cmd.ProcessState.ExitCode() != tt.code {
|
||||
t.Fatalf("exit error = %v, want code %d; output:\n%s", err, tt.code, output)
|
||||
}
|
||||
after, err := os.ReadFile(configPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if tt.code != 0 && !bytes.Equal(before, after) {
|
||||
t.Fatalf("configuration changed after rejected write: %s", after)
|
||||
}
|
||||
})
|
||||
}
|
||||
cfg, err := loadOrCreateConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if cfg.CustomProviders["mimo-v2.6-flash"].URL != "https://example.invalid/v1" ||
|
||||
cfg.CustomProviders["mimo-v2.6-flash"].Protocol != "openai" ||
|
||||
cfg.CustomProviders["foo.api_key"].Model != "some-model" {
|
||||
t.Fatalf("dotted provider fields were not persisted: %#v", cfg.CustomProviders)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunFlushesTelemetryOnError(t *testing.T) {
|
||||
if os.Getenv("OCR_TEST_RUN_ERROR") == "1" {
|
||||
rootCmd.SetArgs([]string{"--invalid-flag"})
|
||||
|
||||
@@ -158,6 +158,9 @@ func applyCustomProviderConfig(configPath string, cfg *Config, result providerTU
|
||||
if result.provider == "" {
|
||||
return fmt.Errorf("provider name is required")
|
||||
}
|
||||
if err := validateProviderName(result.provider); err != nil {
|
||||
return err
|
||||
}
|
||||
model := result.resolvedModel()
|
||||
if model == "" {
|
||||
return fmt.Errorf("model is required")
|
||||
|
||||
@@ -289,6 +289,21 @@ func TestApplyCustomProviderConfig_MissingProvider(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyCustomProviderConfigInvalidName(t *testing.T) {
|
||||
for _, name := range []string{" ", ".gateway", "gateway.", "gateway..v2"} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
cfg := &Config{Provider: "openai", Model: "saved-model"}
|
||||
err := applyCustomProviderConfig(filepath.Join(t.TempDir(), "config.json"), cfg, providerTUIResult{provider: name, model: "some-model"})
|
||||
if err == nil {
|
||||
t.Fatal("expected invalid provider name error")
|
||||
}
|
||||
if cfg.Provider != "openai" || cfg.Model != "saved-model" || cfg.CustomProviders != nil {
|
||||
t.Fatalf("configuration changed after rejected write: %#v", cfg)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyCustomProviderConfig_MissingModel(t *testing.T) {
|
||||
err := applyCustomProviderConfig("", &Config{}, providerTUIResult{provider: "p", model: ""})
|
||||
if err == nil {
|
||||
|
||||
@@ -1246,6 +1246,11 @@ func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) {
|
||||
m.cpStep = cpStepName
|
||||
return m, m.cpNameInput.Focus()
|
||||
}
|
||||
if err := validateProviderName(r.provider); err != nil {
|
||||
m.formError = err.Error()
|
||||
m.cpStep = cpStepName
|
||||
return m, m.cpNameInput.Focus()
|
||||
}
|
||||
if m.customProviderNameTaken(r.provider) {
|
||||
m.formError = fmt.Sprintf(`Provider "%s" already exists`, r.provider)
|
||||
m.cpStep = cpStepName
|
||||
|
||||
@@ -184,6 +184,22 @@ func TestApplyCreateCustomProvider(t *testing.T) {
|
||||
return m
|
||||
}
|
||||
|
||||
for _, name := range []string{".gateway", "gateway.", "gateway..v2"} {
|
||||
t.Run("invalid name "+name, func(t *testing.T) {
|
||||
cfg := &Config{Provider: "openai", Model: "saved-model"}
|
||||
m := setup(t, cfg, filepath.Join(t.TempDir(), "config.json"))
|
||||
m.cpNameInput.SetValue(name)
|
||||
out, _ := m.applyCreateCustomProvider()
|
||||
got := out.(providerTUIModel)
|
||||
if got.formError == "" || got.savedInSession || got.cpStep != cpStepName {
|
||||
t.Fatalf("invalid name was not rejected: formError=%q, saved=%v, step=%v", got.formError, got.savedInSession, got.cpStep)
|
||||
}
|
||||
if cfg.Provider != "openai" || cfg.Model != "saved-model" || cfg.CustomProviders != nil {
|
||||
t.Fatalf("configuration changed after rejected write: %#v", cfg)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("nil config guard", func(t *testing.T) {
|
||||
m := setup(t, &Config{}, filepath.Join(t.TempDir(), "c.json"))
|
||||
m.existingCfg = nil
|
||||
|
||||
@@ -3,10 +3,18 @@
|
||||
|
||||
import { PROVIDER_PRESETS as generatedPresets } from '../providers.generated';
|
||||
import { isPresetProvider, lookupPreset, mergeModelLists, PROVIDER_PRESETS, usesAmbientAuth } from '../providers';
|
||||
import { buildOfficialSaveEntries, detectInitialTab, isConfigReady } from '../configUtils';
|
||||
import { buildOfficialSaveEntries, detectInitialTab, isConfigReady, isValidProviderName } from '../configUtils';
|
||||
import { OcrConfig } from '../types';
|
||||
|
||||
describe('generated provider presets', () => {
|
||||
it.each(['my-gateway', 'mimo-v2.6-flash', 'foo.api_key'])('accepts provider name %s', (name) => {
|
||||
expect(isValidProviderName(name)).toBe(true);
|
||||
});
|
||||
|
||||
it.each(['', ' ', '.a', 'a.', 'a..b', 'a. .b'])('rejects provider name %s', (name) => {
|
||||
expect(isValidProviderName(name)).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps providers sorted by name with the existing initial provider', () => {
|
||||
const names = PROVIDER_PRESETS.map((preset) => preset.name);
|
||||
expect(names).toEqual([...names].sort());
|
||||
|
||||
@@ -18,6 +18,10 @@ export interface ConfigEntry {
|
||||
value: string;
|
||||
}
|
||||
|
||||
export function isValidProviderName(name: string): boolean {
|
||||
return name.split('.').every((segment) => segment.trim() !== '');
|
||||
}
|
||||
|
||||
export function detectInitialTab(config: OcrConfig | null): ProviderTab {
|
||||
if (!config) return 'official';
|
||||
if (config.provider) {
|
||||
@@ -120,15 +124,15 @@ export function buildCustomCreateSaveEntries(params: {
|
||||
authHeader: string;
|
||||
}): ConfigEntry[] {
|
||||
const entries: ConfigEntry[] = [
|
||||
{ key: 'provider', value: params.name.trim() },
|
||||
{ key: `custom_providers.${params.name}.protocol`, value: params.protocol },
|
||||
{ key: `custom_providers.${params.name}.url`, value: params.url.trim() },
|
||||
{ key: `custom_providers.${params.name}.model`, value: params.model.trim() },
|
||||
{ key: `custom_providers.${params.name}.api_key`, value: params.apiKey.trim() },
|
||||
{ key: 'provider', value: params.name.trim() },
|
||||
];
|
||||
const models = params.models.trim();
|
||||
if (models) {
|
||||
entries.splice(3, 0, { key: `custom_providers.${params.name}.models`, value: models });
|
||||
entries.splice(4, 0, { key: `custom_providers.${params.name}.models`, value: models });
|
||||
}
|
||||
if (params.authHeader.trim()) {
|
||||
entries.push({ key: `custom_providers.${params.name}.auth_header`, value: params.authHeader.trim() });
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
import { useEffect, useMemo, useState } from 'preact/hooks';
|
||||
import type { ComponentChildren } from 'preact';
|
||||
import { ConfigEntry, ConfigPanelFocus, ProviderTab, buildCustomCreateSaveEntries, buildCustomUpdateSaveEntries, buildOfficialSaveEntries, describeActiveProvider, detectInitialTab, isConfigReady, listCustomProviderNames } from '../../shared/configUtils';
|
||||
import { ConfigEntry, ConfigPanelFocus, ProviderTab, buildCustomCreateSaveEntries, buildCustomUpdateSaveEntries, buildOfficialSaveEntries, describeActiveProvider, detectInitialTab, isConfigReady, isValidProviderName, listCustomProviderNames } from '../../shared/configUtils';
|
||||
import { mergeModelLists, PROVIDER_PRESETS, usesAmbientAuth } from '../../shared/providers';
|
||||
import { EnvCheckResult, LogLine, OcrConfig } from '../../shared/types';
|
||||
import { CliStatus, ConnTest } from '../configStore';
|
||||
@@ -450,8 +450,8 @@ function CustomForm({
|
||||
const [apiKeyTouched, setApiKeyTouched] = useState(false);
|
||||
const [authHeader, setAuthHeader] = useState(entry?.authHeader ?? '');
|
||||
|
||||
const canSaveCreate = name.trim() !== '' && url.trim() !== '' && model.trim() !== '' && apiKey.trim() !== '';
|
||||
const canSaveEdit = !isCreate && name.trim() !== '' && url.trim() !== '' && model.trim() !== ''
|
||||
const canSaveCreate = isValidProviderName(name) && url.trim() !== '' && model.trim() !== '' && apiKey.trim() !== '';
|
||||
const canSaveEdit = !isCreate && isValidProviderName(name) && url.trim() !== '' && model.trim() !== ''
|
||||
&& Boolean(entry?.apiKey || apiKey.trim());
|
||||
const canSave = isCreate ? canSaveCreate : canSaveEdit;
|
||||
|
||||
|
||||
+14
-12
@@ -113,16 +113,22 @@ private fun RawConfig.setProviderEntryField(
|
||||
}
|
||||
}
|
||||
|
||||
private fun RawConfig.setCustomProviderField(name: String, field: String, value: String) =
|
||||
private fun isValidProviderName(name: String): Boolean = name.split('.').all { it.isNotBlank() }
|
||||
|
||||
private fun RawConfig.setCustomProviderField(name: String, field: String, value: String) {
|
||||
if (!isValidProviderName(name)) return
|
||||
if ('.' in name && (this["custom_providers"] as? JsonObject)?.containsKey(name) != true) return
|
||||
setProviderEntryField("custom_providers", name, field, value)
|
||||
}
|
||||
|
||||
/** `providers.<name>.<field>`: if name is a built-in provider, write to providers; otherwise write to custom_providers. */
|
||||
private fun RawConfig.setProviderValue(key: String, value: String) {
|
||||
val parts = key.split('.')
|
||||
if (parts.size != 3) return
|
||||
val name = parts[1]
|
||||
val field = parts[2]
|
||||
if (isPresetProvider(name)) {
|
||||
val prefix = if (key.startsWith("providers.")) "providers." else "custom_providers."
|
||||
val dot = key.lastIndexOf('.')
|
||||
if (dot <= prefix.length || dot == key.length - 1) return
|
||||
val name = key.substring(prefix.length, dot)
|
||||
val field = key.substring(dot + 1)
|
||||
if (prefix == "providers." && isPresetProvider(name)) {
|
||||
// Consistent with isPresetProvider / ConfigUtils: preset keys are stored canonically lowercased, so "OpenAI"
|
||||
// is not judged preset yet written under a capitalized key that later lookups would miss.
|
||||
setProviderEntryField("providers", name.trim().lowercase(Locale.ROOT), field, value)
|
||||
@@ -145,18 +151,14 @@ private fun RawConfig.setLlmField(field: String, element: JsonElement) {
|
||||
}
|
||||
|
||||
private fun RawConfig.setConfigValue(key: String, value: String) {
|
||||
if (key.startsWith("providers.")) {
|
||||
if (key.startsWith("providers.") || key.startsWith("custom_providers.")) {
|
||||
setProviderValue(key, value)
|
||||
return
|
||||
}
|
||||
if (key.startsWith("custom_providers.")) {
|
||||
val parts = key.split('.')
|
||||
if (parts.size == 3) setCustomProviderField(parts[1], parts[2], value)
|
||||
return
|
||||
}
|
||||
|
||||
when (key) {
|
||||
"provider" -> {
|
||||
if (value.isNotEmpty() && !isValidProviderName(value)) return
|
||||
// Preset providers are stored canonically lowercased, matching the providers bucket keys; custom providers keep the user's original name.
|
||||
val providerName = if (isPresetProvider(value)) value.trim().lowercase(Locale.ROOT) else value
|
||||
// Clear the top-level model when the provider changes: the old model usually does not belong to the new provider
|
||||
|
||||
+52
@@ -14,6 +14,58 @@ import kotlin.test.assertTrue
|
||||
|
||||
class ConfigDraftTest {
|
||||
|
||||
@Test
|
||||
fun `explicitly created dotted providers retain all draft fields`() {
|
||||
for (prefix in listOf("providers", "custom_providers")) {
|
||||
for (name in listOf("mimo-v2.6-flash", "foo.api_key", "openai.extra_body")) {
|
||||
val d = draft(
|
||||
"{}",
|
||||
"provider" to name,
|
||||
"$prefix.$name.protocol" to "openai",
|
||||
"$prefix.$name.url" to "https://example.invalid/v1",
|
||||
"$prefix.$name.model" to "some-model",
|
||||
"$prefix.$name.api_key" to "test-key",
|
||||
"$prefix.$name.models" to "some-model,other-model",
|
||||
"$prefix.$name.auth_header" to "authorization",
|
||||
)
|
||||
assertEquals(name, d.str("provider"))
|
||||
val expected = parseRawConfig(
|
||||
"""{"protocol":"openai","url":"https://example.invalid/v1","model":"some-model","api_key":"test-key","models":["some-model","other-model"],"auth_header":"authorization"}""",
|
||||
)
|
||||
assertEquals(JsonObject(expected), d.obj("custom_providers", name))
|
||||
assertNull(d.obj("providers"))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `dotted field writes require an existing provider without changing the active provider`() {
|
||||
for (prefix in listOf("providers", "custom_providers")) {
|
||||
for (name in listOf("mimo-v2.6-flash", "foo.api_key", "openai.extra_body")) {
|
||||
val base = parseRawConfig("""{"provider":"openai","model":"saved-model"}""")
|
||||
val entries = listOf(ConfigEntry("$prefix.$name.model", "some-model"))
|
||||
assertEquals(base, applyConfigEntries(base, entries))
|
||||
val existing = parseRawConfig("""{"provider":"openai","custom_providers":{"$name":{}}}""")
|
||||
val d = applyConfigEntries(existing, entries)
|
||||
assertEquals("openai", d.str("provider"))
|
||||
assertEquals("some-model", d.str("custom_providers", name, "model"))
|
||||
assertEquals(JsonObject(emptyMap()), existing.obj("custom_providers", name))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `invalid provider names leave the draft unchanged`() {
|
||||
for (name in listOf(" ", ".a", "a.", "a..b", "a. .b", "anthropic.")) {
|
||||
val base = parseRawConfig("""{"provider":"openai","model":"saved-model","custom_providers":{"$name":{}}}""")
|
||||
assertEquals(base, applyConfigEntries(base, listOf(ConfigEntry("provider", name))))
|
||||
for (prefix in listOf("providers", "custom_providers")) {
|
||||
val entries = listOf(ConfigEntry("$prefix.$name.url", "https://example.invalid/v1"))
|
||||
assertEquals(base, applyConfigEntries(base, entries))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun draft(base: String, vararg entries: Pair<String, String>): RawConfig =
|
||||
applyConfigEntries(parseRawConfig(base), entries.map { ConfigEntry(it.first, it.second) })
|
||||
|
||||
|
||||
@@ -2,8 +2,48 @@
|
||||
// Copyright 2026 alibaba/open-code-review Contributors
|
||||
|
||||
import { applyConfigEntries } from '../configDraft';
|
||||
import { buildCustomCreateSaveEntries } from '@shared/configUtils';
|
||||
|
||||
describe('applyConfigEntries', () => {
|
||||
it.each(['mimo-v2.6-flash', 'foo.api_key', 'openai.extra_body'])('creates %s before applying form fields', (name) => {
|
||||
const entries = buildCustomCreateSaveEntries({
|
||||
name, protocol: 'openai', url: 'https://example.invalid/v1', model: 'some-model',
|
||||
models: 'some-model,other-model', apiKey: 'test-key', authHeader: 'authorization',
|
||||
});
|
||||
expect(entries[0]).toEqual({ key: 'provider', value: name });
|
||||
const base = { provider: 'openai', model: 'saved-model' };
|
||||
const draft = applyConfigEntries(base, entries);
|
||||
expect(draft.provider).toBe(name);
|
||||
expect(draft.custom_providers).toEqual({
|
||||
[name]: {
|
||||
protocol: 'openai', url: 'https://example.invalid/v1', model: 'some-model',
|
||||
models: ['some-model', 'other-model'], api_key: 'test-key', auth_header: 'authorization',
|
||||
},
|
||||
});
|
||||
expect(base).toEqual({ provider: 'openai', model: 'saved-model' });
|
||||
});
|
||||
|
||||
it.each(['providers', 'custom_providers'])('requires existing dotted names for %s', (prefix) => {
|
||||
const base = { provider: 'openai', model: 'saved-model' };
|
||||
for (const name of ['mimo-v2.6-flash', 'foo.api_key', 'openai.extra_body']) {
|
||||
const entry = { key: `${prefix}.${name}.model`, value: 'some-model' };
|
||||
expect(applyConfigEntries(base, [entry])).toEqual(base);
|
||||
const existing = { ...base, custom_providers: { [name]: {} } };
|
||||
const draft = applyConfigEntries(existing, [entry]);
|
||||
expect(draft.provider).toBe('openai');
|
||||
expect(draft.custom_providers).toEqual({ [name]: { model: 'some-model' } });
|
||||
expect(existing.custom_providers[name]).toEqual({});
|
||||
}
|
||||
});
|
||||
|
||||
it.each([' ', '.a', 'a.', 'a..b', 'a. .b', 'anthropic.'])('rejects invalid name %s', (name) => {
|
||||
const base = { provider: 'openai', model: 'saved-model', custom_providers: { [name]: {} } };
|
||||
expect(applyConfigEntries(base, [{ key: 'provider', value: name }])).toEqual(base);
|
||||
for (const prefix of ['providers', 'custom_providers']) {
|
||||
expect(applyConfigEntries(base, [{ key: `${prefix}.${name}.url`, value: 'https://example.invalid/v1' }])).toEqual(base);
|
||||
}
|
||||
});
|
||||
|
||||
it('merges built-in provider entries', () => {
|
||||
const draft = applyConfigEntries({}, [
|
||||
{ key: 'provider', value: 'anthropic' },
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
// Copyright 2026 alibaba/open-code-review Contributors
|
||||
|
||||
import { ConfigEntry } from '@shared/configUtils';
|
||||
import { ConfigEntry, isValidProviderName } from '@shared/configUtils';
|
||||
import { isPresetProvider } from '@shared/providers';
|
||||
|
||||
type RawProviderEntry = Record<string, unknown>;
|
||||
@@ -57,6 +57,8 @@ function applyProviderField(entry: RawProviderEntry, field: string, value: strin
|
||||
}
|
||||
|
||||
function setCustomProviderField(cfg: RawConfig, name: string, field: string, value: string): void {
|
||||
if (!isValidProviderName(name)) return;
|
||||
if (name.includes('.') && !Object.prototype.hasOwnProperty.call(cfg.custom_providers ?? {}, name)) return;
|
||||
if (!cfg.custom_providers) cfg.custom_providers = {};
|
||||
const entry = { ...(cfg.custom_providers[name] ?? {}) };
|
||||
applyProviderField(entry, field, value);
|
||||
@@ -64,11 +66,12 @@ function setCustomProviderField(cfg: RawConfig, name: string, field: string, val
|
||||
}
|
||||
|
||||
function setProviderValue(cfg: RawConfig, key: string, value: string): void {
|
||||
const parts = key.split('.');
|
||||
if (parts.length !== 3) return;
|
||||
const name = parts[1];
|
||||
const field = parts[2];
|
||||
if (isPresetProvider(name)) {
|
||||
const prefix = key.startsWith('providers.') ? 'providers.' : 'custom_providers.';
|
||||
const dot = key.lastIndexOf('.');
|
||||
if (dot <= prefix.length || dot === key.length - 1) return;
|
||||
const name = key.slice(prefix.length, dot);
|
||||
const field = key.slice(dot + 1);
|
||||
if (prefix === 'providers.' && isPresetProvider(name)) {
|
||||
if (!cfg.providers) cfg.providers = {};
|
||||
const entry = { ...(cfg.providers[name] ?? {}) };
|
||||
applyProviderField(entry, field, value);
|
||||
@@ -79,18 +82,14 @@ function setProviderValue(cfg: RawConfig, key: string, value: string): void {
|
||||
}
|
||||
|
||||
function setConfigValue(cfg: RawConfig, key: string, value: string): void {
|
||||
if (key.startsWith('providers.')) {
|
||||
if (key.startsWith('providers.') || key.startsWith('custom_providers.')) {
|
||||
setProviderValue(cfg, key, value);
|
||||
return;
|
||||
}
|
||||
if (key.startsWith('custom_providers.')) {
|
||||
const parts = key.split('.');
|
||||
if (parts.length === 3) setCustomProviderField(cfg, parts[1], parts[2], value);
|
||||
return;
|
||||
}
|
||||
|
||||
switch (key) {
|
||||
case 'provider':
|
||||
if (value && !isValidProviderName(value)) return;
|
||||
if (cfg.provider !== value) cfg.model = '';
|
||||
cfg.provider = value;
|
||||
if (isPresetProvider(value)) {
|
||||
|
||||
@@ -163,6 +163,14 @@ rather than accepted and ignored.
|
||||
|
||||
### Custom providers
|
||||
|
||||
Names such as `mimo-v2.6-flash` and `foo.api_key` are supported. Before
|
||||
setting fields on a new name containing dots, create and select it with
|
||||
`ocr config set provider <name>`; this changes the active provider. Existing
|
||||
dotted entries can be edited without selecting them, through either
|
||||
`providers.<name>.<field>` or `custom_providers.<name>.<field>`. Names must
|
||||
not start or end with a dot, contain consecutive dots, or have empty or
|
||||
whitespace-only segments. Dot-free names still support implicit creation.
|
||||
|
||||
Any provider name not in the table above is treated as custom and must
|
||||
supply at least `url` and `protocol` (`protocol` is `anthropic`,
|
||||
`openai`, `openai-responses`, or `anthropic-bedrock`):
|
||||
|
||||
@@ -160,6 +160,13 @@ Model: claude-sonnet-5
|
||||
|
||||
### カスタム provider
|
||||
|
||||
`mimo-v2.6-flash` や `foo.api_key` のような名前を使用できます。ドットを含む新しい
|
||||
名前のフィールドを設定する前に、`ocr config set provider <name>` で作成して選択してください。
|
||||
この操作は現在の provider を変更します。既存のエントリは選択せずに
|
||||
`providers.<name>.<field>` または `custom_providers.<name>.<field>` で編集できます。
|
||||
名前の先頭・末尾のドット、連続したドット、空または空白のみの区切り部分は使用できません。
|
||||
ドットを含まない名前は引き続きフィールドの設定時に自動作成されます。
|
||||
|
||||
上記の表にない provider 名はすべてカスタムとみなされ、少なくとも `url` と
|
||||
`protocol` を指定する必要があります(`protocol` は `anthropic`、`openai`、
|
||||
`openai-responses`、または `anthropic-bedrock`)。
|
||||
|
||||
@@ -139,6 +139,8 @@ Bedrock은 `llm.protocol`이나 `OCR_LLM_PROTOCOL`로는 사용할 수 **없습
|
||||
|
||||
### 커스텀 프로바이더 {#custom-providers}
|
||||
|
||||
`mimo-v2.6-flash`, `foo.api_key`처럼 점이 포함된 이름을 사용할 수 있습니다. 새로운 이름에 점이 있다면 필드를 설정하기 전에 `ocr config set provider <name>`으로 생성하고 선택하세요. 이 명령은 현재 프로바이더를 변경합니다. 기존 항목은 선택하지 않고도 `providers.<name>.<field>` 또는 `custom_providers.<name>.<field>`로 수정할 수 있습니다. 이름의 처음이나 끝에 점이 있거나, 점이 연속되거나, 구분된 부분이 비어 있거나 공백으로만 구성된 이름은 허용되지 않습니다. 점이 없는 이름은 필드 설정 시 계속 자동으로 생성됩니다.
|
||||
|
||||
위 표에 없는 프로바이더 이름은 커스텀으로 취급하며 최소한 `url`과 `protocol`을 지정해야 합니다(`protocol`은 `anthropic`, `openai`, `openai-responses`, `anthropic-bedrock` 중 하나):
|
||||
|
||||
```bash
|
||||
|
||||
@@ -169,6 +169,15 @@ Model: claude-sonnet-5
|
||||
|
||||
### Пользовательские провайдеры
|
||||
|
||||
Поддерживаются имена вида `mimo-v2.6-flash` и `foo.api_key`. Прежде чем
|
||||
задавать поля нового имени с точками, создайте и выберите его командой
|
||||
`ocr config set provider <name>`; она меняет активного провайдера.
|
||||
Существующие записи можно редактировать без выбора через
|
||||
`providers.<name>.<field>` или `custom_providers.<name>.<field>`.
|
||||
Имя не должно начинаться или заканчиваться точкой, содержать несколько
|
||||
точек подряд, пустые части или части только из пробелов. Имена без точек
|
||||
по-прежнему создаются неявно при настройке полей.
|
||||
|
||||
Любое имя провайдера, которого нет в таблице выше, считается
|
||||
пользовательским. Для него необходимо задать как минимум `url` и `protocol`
|
||||
(`protocol` может принимать значения `anthropic`, `openai`,
|
||||
|
||||
@@ -153,6 +153,12 @@ URL 加一个 token,没有地方放区域或 profile,而 bedrock 这两个
|
||||
|
||||
### 自定义 provider
|
||||
|
||||
支持 `mimo-v2.6-flash` 和 `foo.api_key` 这样的名称。为新的含点名称设置字段前,
|
||||
请先运行 `ocr config set provider <name>` 创建并选中它;这会更改当前 provider。
|
||||
已有的含点条目无需选中即可通过 `providers.<name>.<field>` 或
|
||||
`custom_providers.<name>.<field>` 编辑。名称不能以点开头或结尾、包含连续的点,
|
||||
也不能包含空白或仅由空白字符组成的分段。不含点的名称仍支持隐式创建。
|
||||
|
||||
任何不在上表中的 provider 名都视为自定义,至少要提供 `url` 和 `protocol`
|
||||
(`protocol` 取 `anthropic`、`openai`、`openai-responses` 或
|
||||
`anthropic-bedrock`):
|
||||
|
||||
Reference in new issue
Block a user