From ed19e0c4c7357e0f41cd9b6ef7ccc7b4f1d50d19 Mon Sep 17 00:00:00 2001 From: Ernesto Ongaro Date: Fri, 28 Aug 2026 14:02:28 +0100 Subject: [PATCH] fix(config): stop reporting broken configs and unknown profiles as "no API token" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve() discarded Load()'s error, so a malformed or unreadable config.json, and a -p naming a profile that doesn't exist, all surfaced as "no API token configured. Set OMNI_API_TOKEN, use --token, or run `omni config init`" — advice that sends the caller to re-authenticate when the real fix is a typo or a corrupt file. - An unknown -p (or an unknown defaultProfile) now errors immediately, naming the config path and listing the available profiles. - A config file that exists but fails to load is reported as such whenever the config is actually needed; when --token and --base-url are both given the file is still ignored. A missing file remains the silent, normal case. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Ei8UYaG1bW5PJhk93EaqzA --- internal/config/config.go | 42 +++++++++++++++++++++++++--------- internal/config/config_test.go | 42 ++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 8699521..0b9aafe 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,11 +5,13 @@ package config import ( "context" "encoding/json" + "errors" "fmt" "net/url" "os" "path/filepath" "runtime" + "sort" "strings" "time" @@ -82,8 +84,13 @@ type ResolvedConfig struct { func Resolve(profileName, tokenFlag, baseURLFlag string) (*ResolvedConfig, error) { rc := &ResolvedConfig{} - // Start from config file - cfg, _ := Load() + // Start from config file. A missing file is normal (env/flags may carry + // everything); any other failure is remembered and reported below if the + // config turns out to be needed, instead of masquerading as "no token". + cfg, loadErr := Load() + if loadErr != nil && errors.Is(loadErr, os.ErrNotExist) { + loadErr = nil + } var profile *Profile if cfg != nil { name := profileName @@ -91,15 +98,26 @@ func Resolve(profileName, tokenFlag, baseURLFlag string) (*ResolvedConfig, error name = cfg.DefaultProfile } if name != "" { - if p, ok := cfg.Profiles[name]; ok { - profile = &p - rc.BaseURL = p.APIEndpoint - switch p.AuthMethod { - case "oauth": - rc.Token = p.AccessToken - default: // "api-key" - rc.Token = p.APIKey + p, ok := cfg.Profiles[name] + if !ok { + names := make([]string, 0, len(cfg.Profiles)) + for n := range cfg.Profiles { + names = append(names, n) + } + sort.Strings(names) + which := "profile" + if profileName == "" { + which = "default profile" } + return nil, fmt.Errorf("%s %q not found in %s (available: %s)", which, name, ConfigPath(), strings.Join(names, ", ")) + } + profile = &p + rc.BaseURL = p.APIEndpoint + switch p.AuthMethod { + case "oauth": + rc.Token = p.AccessToken + default: // "api-key" + rc.Token = p.APIKey } } } @@ -151,6 +169,9 @@ func Resolve(profileName, tokenFlag, baseURLFlag string) (*ResolvedConfig, error } // Validate + if loadErr != nil && (rc.Token == "" || rc.BaseURL == "") { + return nil, fmt.Errorf("loading config %s: %w", ConfigPath(), loadErr) + } if rc.Token == "" { return nil, fmt.Errorf("no API token configured. Set OMNI_API_TOKEN, use --token, or run `omni config init`") } @@ -256,4 +277,3 @@ func ConfigPath() string { } return filepath.Join(configDir(), "config.json") } - diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 89165e1..12abd2a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -715,3 +715,45 @@ func TestLoad_MissingFile(t *testing.T) { t.Fatal("expected error loading nonexistent file, got nil") } } + +func TestResolve_UnknownProfileNamesAvailable(t *testing.T) { + dir := t.TempDir() + t.Setenv("OMNI_CONFIG_DIR", dir) + t.Setenv("OMNI_API_TOKEN", "") + cfg := &Config{Version: 1, DefaultProfile: "a", Profiles: map[string]Profile{ + "a": {APIEndpoint: "https://a.omniapp.co", AuthMethod: "api-key", APIKey: "k"}, + "b": {APIEndpoint: "https://b.omniapp.co", AuthMethod: "api-key", APIKey: "k"}, + }} + if err := Save(cfg); err != nil { + t.Fatal(err) + } + _, err := Resolve("nope", "", "") + if err == nil { + t.Fatal("expected error for unknown profile") + } + for _, want := range []string{`profile "nope" not found`, "available: a, b"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q missing %q", err, want) + } + } + if strings.Contains(err.Error(), "no API token") { + t.Errorf("unknown profile must not be reported as a missing token: %q", err) + } +} + +func TestResolve_MalformedConfigIsReported(t *testing.T) { + dir := t.TempDir() + t.Setenv("OMNI_CONFIG_DIR", dir) + t.Setenv("OMNI_API_TOKEN", "") + if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"version":1,"profiles":{`), 0o600); err != nil { + t.Fatal(err) + } + _, err := Resolve("", "", "") + if err == nil || !strings.Contains(err.Error(), "loading config") || strings.Contains(err.Error(), "no API token") { + t.Errorf("want a parse error naming the config file, got %v", err) + } + // With everything supplied on the command line the broken file is irrelevant. + if _, err := Resolve("", "tok", "https://x.omniapp.co"); err != nil { + t.Errorf("flags should bypass the broken config, got %v", err) + } +}