From f17369c5521431d82f561c9092ed45aa9a57d831 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Fri, 12 Jun 2026 14:09:11 -0700 Subject: [PATCH 01/10] feat(config): trigger the config.toml migration at startup (#245) Co-authored-by: Claude Fable 5 --- pkg/cmd/root/root.go | 10 ++ pkg/config/migrate.go | 114 +++++++++++++++ pkg/config/migrate_test.go | 279 +++++++++++++++++++++++++++++++++++++ 3 files changed, 403 insertions(+) create mode 100644 pkg/config/migrate.go create mode 100644 pkg/config/migrate_test.go diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 069e7f28..fe70090b 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -136,6 +136,16 @@ func Execute() exitCode { cmdFactory := factory.New(version.Version, &cfg) stderr := cmdFactory.IOStreams.ErrOut + // One-time config.toml → state.toml + keychain migration (GROUT-363). + // Must run before the command executes: credential resolution reads + // state.toml once per invocation and caches it. A failed migration never + // blocks the command — state.toml stays absent, so it retries next run. + if cfg.ShouldMigrate() { + if err := cfg.Migrate(); err != nil && hasDebug { + fmt.Fprintf(stderr, "config migration failed (will retry on next run): %s\n", err) + } + } + // Set up the update notifier. updateMessageChan := make(chan *update.ReleaseInfo) go func() { diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go new file mode 100644 index 00000000..54949f25 --- /dev/null +++ b/pkg/config/migrate.go @@ -0,0 +1,114 @@ +package config + +import ( + "os" + "sort" + + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" + + "github.com/algolia/cli/pkg/keychain" +) + +// ShouldMigrate reports whether the one-time config.toml → state.toml + +// keychain migration still has to run: a legacy config.toml exists and +// state.toml does not. state.toml is only written when the migration (or any +// new-model write) succeeds, so its absence doubles as the "not migrated yet" +// marker — an aborted migration naturally retries on the next run. +func (c *Config) ShouldMigrate() bool { + if c.File == "" { + return false + } + if _, err := os.Stat(c.File); err != nil { + return false + } + return !c.StateFileExists() +} + +// Migrate moves the legacy config.toml profiles into the new model (state.toml +// + OS keychain). config.toml itself is never modified. +// +// Secrets go to the keychain first; state.toml is only written — atomically, +// via State.Save's temp + rename — once every profile's keys are stored. A +// keychain failure mid-run therefore leaves state.toml absent and the whole +// migration retries on the next command; entries already written are simply +// rewritten then. With nothing to migrate an empty state.toml still gets +// written, so ShouldMigrate stops firing on every command. +func (c *Config) Migrate() error { + state := &State{Applications: map[string]ApplicationState{}} + + for _, profile := range c.migratableProfiles() { + secrets := keychain.AppSecrets{ + APIKey: profile.APIKey, + CrawlerAPIKey: viper.GetString(profile.GetFieldName("crawler_api_key")), + } + if err := keychain.SaveAppSecrets(profile.ApplicationID, secrets); err != nil { + return err + } + + // api_key_uuid is unknown for legacy keys: left empty until an API + // lookup can backfill it. + state.UpsertApplication(profile.ApplicationID, ApplicationState{Alias: profile.Name}) + if profile.Default { + state.SetCurrentApplication(profile.ApplicationID) + } + } + + return state.Save(c.StateFile) +} + +// migratableProfiles applies the migration skip rules to the config.toml +// profiles before any keychain write happens: +// +// - admin_api_key never moves to the new model: one log line points to its +// replacements, whether the profile migrates or not. +// - A profile without application_id or with an empty api_key has nothing +// usable to migrate: skipped with a log line. +// - Profiles sharing the same application_id would overwrite each other's +// keychain entry: the default = true profile wins, the others are logged +// as conflicts and skipped. +// +// Profiles are processed in name order so conflict resolution and logs stay +// deterministic (ConfiguredProfiles iterates a map). +func (c *Config) migratableProfiles() []*Profile { + profiles := c.ConfiguredProfiles() + sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name }) + + selected := make([]*Profile, 0, len(profiles)) + owner := map[string]int{} // application ID → index in selected + + for _, profile := range profiles { + if profile.AdminAPIKey != "" { + log.Warnf( + "config migration: profile %q: admin_api_key is not migrated, use ALGOLIA_ADMIN_API_KEY or --api-key instead", + profile.Name, + ) + } + if profile.ApplicationID == "" { + log.Warnf("config migration: skipping profile %q: no application_id", profile.Name) + continue + } + if profile.APIKey == "" { + log.Warnf("config migration: skipping profile %q: empty api_key", profile.Name) + continue + } + if i, ok := owner[profile.ApplicationID]; ok { + kept, dropped := selected[i], profile + if profile.Default && !kept.Default { + selected[i] = profile + kept, dropped = profile, kept + } + log.Warnf( + "config migration: skipping profile %q: application %q already migrated from profile %q", + dropped.Name, + dropped.ApplicationID, + kept.Name, + ) + continue + } + owner[profile.ApplicationID] = len(selected) + selected = append(selected, profile) + } + + return selected +} diff --git a/pkg/config/migrate_test.go b/pkg/config/migrate_test.go new file mode 100644 index 00000000..1786dc19 --- /dev/null +++ b/pkg/config/migrate_test.go @@ -0,0 +1,279 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + logtest "github.com/sirupsen/logrus/hooks/test" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/pkg/keychain" +) + +func TestConfig_ShouldMigrate(t *testing.T) { + tests := []struct { + name string + configFile bool + stateFile bool + want bool + }{ + { + name: "config.toml only: migration pending", + configFile: true, + stateFile: false, + want: true, + }, + { + name: "both files: already migrated (or new model in use)", + configFile: true, + stateFile: true, + want: false, + }, + { + name: "state.toml only: nothing to migrate", + configFile: false, + stateFile: true, + want: false, + }, + { + name: "no files: fresh install", + configFile: false, + stateFile: false, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + cfg := &Config{ + File: filepath.Join(dir, "config.toml"), + StateFile: filepath.Join(dir, "state.toml"), + } + if tt.configFile { + require.NoError(t, os.WriteFile(cfg.File, []byte(""), 0o600)) + } + if tt.stateFile { + require.NoError(t, os.WriteFile(cfg.StateFile, []byte(""), 0o600)) + } + + assert.Equal(t, tt.want, cfg.ShouldMigrate()) + }) + } +} + +func TestConfig_ShouldMigrate_unresolvedPaths(t *testing.T) { + // InitConfig never ran: paths are empty, the trigger must stay off. + cfg := &Config{} + assert.False(t, cfg.ShouldMigrate()) +} + +// migrationConfig writes a config.toml with the given content, points the +// global viper at it (ConfiguredProfiles reads through viper) and returns a +// Config ready to migrate. +func migrationConfig(t *testing.T, content string) *Config { + t.Helper() + + dir := t.TempDir() + configFile := filepath.Join(dir, "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600)) + + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + t.Cleanup(viper.Reset) + + return &Config{ + File: configFile, + StateFile: filepath.Join(dir, "state.toml"), + } +} + +func TestConfig_Migrate(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, `[prod] +application_id = "APP1" +api_key = "key-1" +crawler_api_key = "crawler-1" +default = true + +[dev] +application_id = "APP2" +api_key = "key-2" +`) + + require.NoError(t, cfg.Migrate()) + + // Secrets land in the keychain, crawler key included when set. + prod, err := keychain.LoadAppSecrets("APP1") + require.NoError(t, err) + require.NotNil(t, prod) + assert.Equal(t, "key-1", prod.APIKey) + assert.Equal(t, "crawler-1", prod.CrawlerAPIKey) + + dev, err := keychain.LoadAppSecrets("APP2") + require.NoError(t, err) + require.NotNil(t, dev) + assert.Equal(t, "key-2", dev.APIKey) + assert.Empty(t, dev.CrawlerAPIKey) + + // state.toml: one entry per application, alias = profile name, current + // application = the default profile's one. + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + assert.Equal(t, "APP1", st.CurrentApplicationID) + assert.Equal(t, "prod", st.Applications["APP1"].Alias) + assert.Equal(t, "dev", st.Applications["APP2"].Alias) + assert.Empty(t, st.Applications["APP1"].APIKeyUUID) // unknown for legacy keys + + // state.toml now exists: the trigger turns off. + assert.False(t, cfg.ShouldMigrate()) +} + +func TestConfig_Migrate_NoDefaultProfileLeavesCurrentEmpty(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, `[dev] +application_id = "APP2" +api_key = "key-2" +`) + + require.NoError(t, cfg.Migrate()) + + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + assert.Empty(t, st.CurrentApplicationID) + assert.Equal(t, "dev", st.Applications["APP2"].Alias) +} + +func TestConfig_Migrate_EmptyConfigStillWritesState(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, "") + + require.NoError(t, cfg.Migrate()) + + // An empty state.toml must exist, otherwise the migration would re-run + // (and re-log) on every command. + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + assert.Empty(t, st.CurrentApplicationID) + assert.Empty(t, st.Applications) + assert.False(t, cfg.ShouldMigrate()) +} + +func TestConfig_Migrate_KeychainFailureLeavesStateAbsent(t *testing.T) { + keyring.MockInitWithError(keyring.ErrUnsupportedPlatform) + cfg := migrationConfig(t, `[prod] +application_id = "APP1" +api_key = "key-1" +`) + + require.Error(t, cfg.Migrate()) + + // state.toml untouched: ShouldMigrate keeps firing so the migration + // retries on the next run. + assert.NoFileExists(t, cfg.StateFile) + assert.True(t, cfg.ShouldMigrate()) +} + +func TestConfig_Migrate_SkipRules(t *testing.T) { + keyring.MockInit() + hook := logtest.NewGlobal() + t.Cleanup(hook.Reset) + + cfg := migrationConfig(t, `[nokey] +application_id = "APP3" +api_key = "" + +[noapp] +api_key = "key-x" + +[adminonly] +application_id = "APP4" +admin_api_key = "admin-key" +`) + + require.NoError(t, cfg.Migrate()) + + // Nothing migrated: no keychain entries, an empty state.toml that still + // turns the trigger off. + for _, appID := range []string{"APP3", "APP4"} { + secrets, err := keychain.LoadAppSecrets(appID) + require.NoError(t, err) + assert.Nil(t, secrets) + } + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + assert.Empty(t, st.Applications) + assert.False(t, cfg.ShouldMigrate()) + + // Each skip got its log line, plus the admin_api_key notice. + logs := make([]string, 0, len(hook.AllEntries())) + for _, entry := range hook.AllEntries() { + logs = append(logs, entry.Message) + } + assert.Contains(t, logs, + `config migration: skipping profile "nokey": empty api_key`) + assert.Contains(t, logs, + `config migration: skipping profile "noapp": no application_id`) + assert.Contains(t, logs, + `config migration: skipping profile "adminonly": empty api_key`) + assert.Contains( + t, + logs, + `config migration: profile "adminonly": admin_api_key is not migrated, use ALGOLIA_ADMIN_API_KEY or --api-key instead`, + ) +} + +func TestConfig_Migrate_DuplicateApplicationKeepsDefault(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, `[backup] +application_id = "APP1" +api_key = "backup-key" + +[prod] +application_id = "APP1" +api_key = "prod-key" +default = true +`) + + require.NoError(t, cfg.Migrate()) + + // One single entry for APP1: the default profile's alias and key. + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + require.Len(t, st.Applications, 1) + assert.Equal(t, "prod", st.Applications["APP1"].Alias) + assert.Equal(t, "APP1", st.CurrentApplicationID) + + secrets, err := keychain.LoadAppSecrets("APP1") + require.NoError(t, err) + require.NotNil(t, secrets) + assert.Equal(t, "prod-key", secrets.APIKey) +} + +func TestConfig_Migrate_AdminKeyAlongsideAPIKeyStillMigrates(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, `[prod] +application_id = "APP1" +api_key = "key-1" +admin_api_key = "admin-1" +default = true +`) + + require.NoError(t, cfg.Migrate()) + + // The search key migrates; the admin key has no slot in the new model. + secrets, err := keychain.LoadAppSecrets("APP1") + require.NoError(t, err) + require.NotNil(t, secrets) + assert.Equal(t, "key-1", secrets.APIKey) + + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + assert.Equal(t, "prod", st.Applications["APP1"].Alias) +} From 9a56df4bfc11269e524c12d60cdbbdfdf5c365c1 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Fri, 12 Jun 2026 14:14:11 -0700 Subject: [PATCH 02/10] chore(config): slim down the migration comments Co-Authored-By: Claude Fable 5 --- pkg/cmd/root/root.go | 6 ++---- pkg/config/migrate.go | 36 +++++++++--------------------------- pkg/config/migrate_test.go | 21 +++++---------------- 3 files changed, 16 insertions(+), 47 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index fe70090b..7d2626a1 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -136,10 +136,8 @@ func Execute() exitCode { cmdFactory := factory.New(version.Version, &cfg) stderr := cmdFactory.IOStreams.ErrOut - // One-time config.toml → state.toml + keychain migration (GROUT-363). - // Must run before the command executes: credential resolution reads - // state.toml once per invocation and caches it. A failed migration never - // blocks the command — state.toml stays absent, so it retries next run. + // One-time config.toml → state.toml + keychain migration (GROUT-363). Must + // run before credential resolution, which caches state.toml per command. if cfg.ShouldMigrate() { if err := cfg.Migrate(); err != nil && hasDebug { fmt.Fprintf(stderr, "config migration failed (will retry on next run): %s\n", err) diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go index 54949f25..c3b0c3de 100644 --- a/pkg/config/migrate.go +++ b/pkg/config/migrate.go @@ -11,10 +11,8 @@ import ( ) // ShouldMigrate reports whether the one-time config.toml → state.toml + -// keychain migration still has to run: a legacy config.toml exists and -// state.toml does not. state.toml is only written when the migration (or any -// new-model write) succeeds, so its absence doubles as the "not migrated yet" -// marker — an aborted migration naturally retries on the next run. +// keychain migration still has to run: config.toml exists and state.toml +// (only written on success, so doubling as the "migrated" marker) does not. func (c *Config) ShouldMigrate() bool { if c.File == "" { return false @@ -26,14 +24,9 @@ func (c *Config) ShouldMigrate() bool { } // Migrate moves the legacy config.toml profiles into the new model (state.toml -// + OS keychain). config.toml itself is never modified. -// -// Secrets go to the keychain first; state.toml is only written — atomically, -// via State.Save's temp + rename — once every profile's keys are stored. A -// keychain failure mid-run therefore leaves state.toml absent and the whole -// migration retries on the next command; entries already written are simply -// rewritten then. With nothing to migrate an empty state.toml still gets -// written, so ShouldMigrate stops firing on every command. +// + OS keychain); config.toml is never modified. Keychain first, state.toml +// last (atomic): a failure leaves state.toml absent, so the migration retries +// on the next run. func (c *Config) Migrate() error { state := &State{Applications: map[string]ApplicationState{}} @@ -46,8 +39,6 @@ func (c *Config) Migrate() error { return err } - // api_key_uuid is unknown for legacy keys: left empty until an API - // lookup can backfill it. state.UpsertApplication(profile.ApplicationID, ApplicationState{Alias: profile.Name}) if profile.Default { state.SetCurrentApplication(profile.ApplicationID) @@ -57,19 +48,10 @@ func (c *Config) Migrate() error { return state.Save(c.StateFile) } -// migratableProfiles applies the migration skip rules to the config.toml -// profiles before any keychain write happens: -// -// - admin_api_key never moves to the new model: one log line points to its -// replacements, whether the profile migrates or not. -// - A profile without application_id or with an empty api_key has nothing -// usable to migrate: skipped with a log line. -// - Profiles sharing the same application_id would overwrite each other's -// keychain entry: the default = true profile wins, the others are logged -// as conflicts and skipped. -// -// Profiles are processed in name order so conflict resolution and logs stay -// deterministic (ConfiguredProfiles iterates a map). +// migratableProfiles applies the skip rules: profiles without application_id +// or api_key are dropped, admin_api_key never migrates, and the default +// profile wins when several share an application_id. Name order keeps the +// conflict resolution deterministic (ConfiguredProfiles iterates a map). func (c *Config) migratableProfiles() []*Profile { profiles := c.ConfiguredProfiles() sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name }) diff --git a/pkg/config/migrate_test.go b/pkg/config/migrate_test.go index 1786dc19..a4dc9bef 100644 --- a/pkg/config/migrate_test.go +++ b/pkg/config/migrate_test.go @@ -72,9 +72,8 @@ func TestConfig_ShouldMigrate_unresolvedPaths(t *testing.T) { assert.False(t, cfg.ShouldMigrate()) } -// migrationConfig writes a config.toml with the given content, points the -// global viper at it (ConfiguredProfiles reads through viper) and returns a -// Config ready to migrate. +// migrationConfig writes a config.toml, points the global viper at it +// (ConfiguredProfiles reads through viper) and returns a Config ready to migrate. func migrationConfig(t *testing.T, content string) *Config { t.Helper() @@ -109,7 +108,6 @@ api_key = "key-2" require.NoError(t, cfg.Migrate()) - // Secrets land in the keychain, crawler key included when set. prod, err := keychain.LoadAppSecrets("APP1") require.NoError(t, err) require.NotNil(t, prod) @@ -122,8 +120,6 @@ api_key = "key-2" assert.Equal(t, "key-2", dev.APIKey) assert.Empty(t, dev.CrawlerAPIKey) - // state.toml: one entry per application, alias = profile name, current - // application = the default profile's one. st, err := LoadState(cfg.StateFile) require.NoError(t, err) assert.Equal(t, "APP1", st.CurrentApplicationID) @@ -131,7 +127,6 @@ api_key = "key-2" assert.Equal(t, "dev", st.Applications["APP2"].Alias) assert.Empty(t, st.Applications["APP1"].APIKeyUUID) // unknown for legacy keys - // state.toml now exists: the trigger turns off. assert.False(t, cfg.ShouldMigrate()) } @@ -156,8 +151,7 @@ func TestConfig_Migrate_EmptyConfigStillWritesState(t *testing.T) { require.NoError(t, cfg.Migrate()) - // An empty state.toml must exist, otherwise the migration would re-run - // (and re-log) on every command. + // An empty state.toml must exist, otherwise the migration re-runs forever. st, err := LoadState(cfg.StateFile) require.NoError(t, err) assert.Empty(t, st.CurrentApplicationID) @@ -174,8 +168,7 @@ api_key = "key-1" require.Error(t, cfg.Migrate()) - // state.toml untouched: ShouldMigrate keeps firing so the migration - // retries on the next run. + // state.toml untouched: the migration retries on the next run. assert.NoFileExists(t, cfg.StateFile) assert.True(t, cfg.ShouldMigrate()) } @@ -199,8 +192,7 @@ admin_api_key = "admin-key" require.NoError(t, cfg.Migrate()) - // Nothing migrated: no keychain entries, an empty state.toml that still - // turns the trigger off. + // Nothing migrated, but the trigger still turns off. for _, appID := range []string{"APP3", "APP4"} { secrets, err := keychain.LoadAppSecrets(appID) require.NoError(t, err) @@ -211,7 +203,6 @@ admin_api_key = "admin-key" assert.Empty(t, st.Applications) assert.False(t, cfg.ShouldMigrate()) - // Each skip got its log line, plus the admin_api_key notice. logs := make([]string, 0, len(hook.AllEntries())) for _, entry := range hook.AllEntries() { logs = append(logs, entry.Message) @@ -243,7 +234,6 @@ default = true require.NoError(t, cfg.Migrate()) - // One single entry for APP1: the default profile's alias and key. st, err := LoadState(cfg.StateFile) require.NoError(t, err) require.Len(t, st.Applications, 1) @@ -267,7 +257,6 @@ default = true require.NoError(t, cfg.Migrate()) - // The search key migrates; the admin key has no slot in the new model. secrets, err := keychain.LoadAppSecrets("APP1") require.NoError(t, err) require.NotNil(t, secrets) From fc5ae016e93ec638fa59ee7af95fa4b70ea94fc9 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Fri, 12 Jun 2026 15:11:03 -0700 Subject: [PATCH 03/10] fix(config): harden the migration against bad config.toml files Two startup-path regressions found while testing edge cases: - An undecodable profile (root-level scalar, unconvertible field type) hit ConfiguredProfiles' log.Fatalf and bricked every command, --help included, forever (state.toml never written). The migration now decodes profiles itself and skips undecodable entries with a log. - An unparseable config.toml was silently read as zero profiles, so an empty state.toml marked the migration as done forever. Migrate now aborts when config.toml cannot be parsed and retries on the next run. Co-Authored-By: Claude Fable 5 --- pkg/config/migrate.go | 19 +++++++++++++++- pkg/config/migrate_test.go | 44 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go index c3b0c3de..57cf2f28 100644 --- a/pkg/config/migrate.go +++ b/pkg/config/migrate.go @@ -28,6 +28,12 @@ func (c *Config) ShouldMigrate() bool { // last (atomic): a failure leaves state.toml absent, so the migration retries // on the next run. func (c *Config) Migrate() error { + // An unparseable config.toml must not mark the migration as done: abort + // before writing state.toml so it retries once the file is fixed. + if err := viper.ReadInConfig(); err != nil { + return err + } + state := &State{Applications: map[string]ApplicationState{}} for _, profile := range c.migratableProfiles() { @@ -53,7 +59,18 @@ func (c *Config) Migrate() error { // profile wins when several share an application_id. Name order keeps the // conflict resolution deterministic (ConfiguredProfiles iterates a map). func (c *Config) migratableProfiles() []*Profile { - profiles := c.ConfiguredProfiles() + // Decode the profiles here rather than through ConfiguredProfiles, whose + // log.Fatalf on an undecodable entry would brick every command at startup. + configs := viper.AllSettings() + profiles := make([]*Profile, 0, len(configs)) + for name := range configs { + profile := &Profile{Name: name} + if err := viper.UnmarshalKey(name, profile); err != nil { + log.Warnf("config migration: skipping profile %q: %s", name, err) + continue + } + profiles = append(profiles, profile) + } sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name }) selected := make([]*Profile, 0, len(profiles)) diff --git a/pkg/config/migrate_test.go b/pkg/config/migrate_test.go index a4dc9bef..4b2e795d 100644 --- a/pkg/config/migrate_test.go +++ b/pkg/config/migrate_test.go @@ -266,3 +266,47 @@ default = true require.NoError(t, err) assert.Equal(t, "prod", st.Applications["APP1"].Alias) } + +func TestConfig_Migrate_UndecodableProfileSkipped(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, `telemetry = "off" + +[prod] +application_id = "APP1" +api_key = "key-1" +default = true + +[bad] +application_id = "APP2" +api_key = ["a", "b"] +`) + + // Undecodable entries (root scalar, wrong types) are skipped, not fatal. + require.NoError(t, cfg.Migrate()) + + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + require.Len(t, st.Applications, 1) + assert.Equal(t, "APP1", st.CurrentApplicationID) + assert.Equal(t, "prod", st.Applications["APP1"].Alias) +} + +func TestConfig_Migrate_UnreadableConfigAborts(t *testing.T) { + keyring.MockInit() + dir := t.TempDir() + configFile := filepath.Join(dir, "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte("not [ valid ### toml\n"), 0o600)) + + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + _ = viper.ReadInConfig() // swallowed, like InitConfig does + t.Cleanup(viper.Reset) + + cfg := &Config{File: configFile, StateFile: filepath.Join(dir, "state.toml")} + + // No state.toml written: the migration retries once the file is fixed. + require.Error(t, cfg.Migrate()) + assert.NoFileExists(t, cfg.StateFile) + assert.True(t, cfg.ShouldMigrate()) +} From 93ae4d12889366b0e3ae97ac74afbafe3fe55580 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Fri, 12 Jun 2026 18:33:43 -0700 Subject: [PATCH 04/10] feat(config): migrate search_hosts and crawler_user_id to state.toml The remaining non-secret profile data moves with the migration so it survives the eventual config.toml removal. GetSearchHosts and GetCrawlerUserID gain a new-model branch: the resolved application's state.toml entry answers first, an empty value falls through to the legacy config.toml lookup while both models coexist. admin_api_key stays excluded as decided on the ticket. Co-Authored-By: Claude Fable 5 --- pkg/config/migrate.go | 6 +++- pkg/config/migrate_test.go | 10 +++++++ pkg/config/profile.go | 20 +++++++++++++ pkg/config/resolution_test.go | 55 +++++++++++++++++++++++++++++++++++ pkg/config/state.go | 6 ++-- 5 files changed, 94 insertions(+), 3 deletions(-) diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go index 57cf2f28..b5046d62 100644 --- a/pkg/config/migrate.go +++ b/pkg/config/migrate.go @@ -45,7 +45,11 @@ func (c *Config) Migrate() error { return err } - state.UpsertApplication(profile.ApplicationID, ApplicationState{Alias: profile.Name}) + state.UpsertApplication(profile.ApplicationID, ApplicationState{ + Alias: profile.Name, + SearchHosts: profile.SearchHosts, + CrawlerUserID: viper.GetString(profile.GetFieldName("crawler_user_id")), + }) if profile.Default { state.SetCurrentApplication(profile.ApplicationID) } diff --git a/pkg/config/migrate_test.go b/pkg/config/migrate_test.go index 4b2e795d..841ac930 100644 --- a/pkg/config/migrate_test.go +++ b/pkg/config/migrate_test.go @@ -99,6 +99,8 @@ func TestConfig_Migrate(t *testing.T) { application_id = "APP1" api_key = "key-1" crawler_api_key = "crawler-1" +crawler_user_id = "crawler-user" +search_hosts = ["h1.algolia.net", "h2.algolia.net"] default = true [dev] @@ -126,6 +128,14 @@ api_key = "key-2" assert.Equal(t, "prod", st.Applications["APP1"].Alias) assert.Equal(t, "dev", st.Applications["APP2"].Alias) assert.Empty(t, st.Applications["APP1"].APIKeyUUID) // unknown for legacy keys + assert.Equal( + t, + []string{"h1.algolia.net", "h2.algolia.net"}, + st.Applications["APP1"].SearchHosts, + ) + assert.Equal(t, "crawler-user", st.Applications["APP1"].CrawlerUserID) + assert.Empty(t, st.Applications["APP2"].SearchHosts) + assert.Empty(t, st.Applications["APP2"].CrawlerUserID) assert.False(t, cfg.ShouldMigrate()) } diff --git a/pkg/config/profile.go b/pkg/config/profile.go index 7cd1c3bc..fd9d8972 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -146,6 +146,16 @@ func (p *Profile) GetSearchHosts() []string { return p.SearchHosts } + // New model: hosts recorded for the resolved application. Empty falls + // through to the legacy config.toml lookup while both models coexist. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + if hosts := p.config.loadState().Applications[appID].SearchHosts; len(hosts) > 0 { + return hosts + } + } + } + if p.Name == "" { p.LoadDefault() } @@ -170,6 +180,16 @@ func (p *Profile) GetCrawlerUserID() (string, error) { return os.Getenv("ALGOLIA_CRAWLER_USER_ID"), nil } + // New model: the user ID recorded for the resolved application. Empty + // falls through to the legacy config.toml lookup. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + if userID := p.config.loadState().Applications[appID].CrawlerUserID; userID != "" { + return userID, nil + } + } + } + if p.Name == "" { p.LoadDefault() } diff --git a/pkg/config/resolution_test.go b/pkg/config/resolution_test.go index 00986fd8..03c5d3b8 100644 --- a/pkg/config/resolution_test.go +++ b/pkg/config/resolution_test.go @@ -243,3 +243,58 @@ func TestProfile_GetAPIKey_ActiveAppWithoutKeyErrors(t *testing.T) { _, err := cfg.Profile().GetAPIKey() require.Error(t, err) } + +func TestProfile_GetSearchHosts_FromState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\nsearch_hosts = [\"h1\", \"h2\"]\n", + ), 0o600)) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + assert.Equal(t, []string{"h1", "h2"}, cfg.Profile().GetSearchHosts()) +} + +func TestProfile_GetSearchHosts_StateEmptyFallsBackToConfigToml(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "state.toml") + require.NoError( + t, + os.WriteFile(statePath, []byte("current_application_id = \"APP1\"\n"), 0o600), + ) + + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile( + configFile, + []byte( + "[legacy]\napplication_id = \"APP1\"\nsearch_hosts = [\"legacy-host\"]\ndefault = true\n", + ), + 0o600, + )) + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + t.Cleanup(viper.Reset) + + cfg := &Config{StateFile: statePath} + cfg.CurrentProfile.config = cfg + + // No hosts in state.toml for APP1: the legacy lookup still answers while + // config.toml exists. + assert.Equal(t, []string{"legacy-host"}, cfg.Profile().GetSearchHosts()) +} + +func TestProfile_GetCrawlerUserID_FromState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\ncrawler_user_id = \"crawler-user\"\n", + ), 0o600)) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + userID, err := cfg.Profile().GetCrawlerUserID() + require.NoError(t, err) + assert.Equal(t, "crawler-user", userID) +} diff --git a/pkg/config/state.go b/pkg/config/state.go index 99d4809d..2cae49b7 100644 --- a/pkg/config/state.go +++ b/pkg/config/state.go @@ -11,8 +11,10 @@ import ( // ApplicationState holds the non-secret, per-application data persisted in // state.toml. Secrets (API keys) live in the OS keychain, not here. type ApplicationState struct { - APIKeyUUID string `toml:"api_key_uuid"` - Alias string `toml:"alias"` + APIKeyUUID string `toml:"api_key_uuid"` + Alias string `toml:"alias"` + SearchHosts []string `toml:"search_hosts,omitempty"` + CrawlerUserID string `toml:"crawler_user_id,omitempty"` } // State is the in-memory representation of state.toml, the new source of truth From f61c7ccc52b89fb43020b2cd4d967dcb1e428d85 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 08:23:07 -0700 Subject: [PATCH 05/10] refactor(config): omit empty api_key_uuid from state.toml Legacy migrated keys have no UUID, so without omitempty every migrated entry serialized a noisy api_key_uuid = "". The field is only set by new-model writes (app create/select); omitting the empty value matches search_hosts/crawler_user_id and reads back identically. Co-Authored-By: Claude Fable 5 --- pkg/config/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/state.go b/pkg/config/state.go index 2cae49b7..3b4c059f 100644 --- a/pkg/config/state.go +++ b/pkg/config/state.go @@ -11,7 +11,7 @@ import ( // ApplicationState holds the non-secret, per-application data persisted in // state.toml. Secrets (API keys) live in the OS keychain, not here. type ApplicationState struct { - APIKeyUUID string `toml:"api_key_uuid"` + APIKeyUUID string `toml:"api_key_uuid,omitempty"` Alias string `toml:"alias"` SearchHosts []string `toml:"search_hosts,omitempty"` CrawlerUserID string `toml:"crawler_user_id,omitempty"` From 8b7a57c1496a82f3a335a0eb15406b99f6d95cb9 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 08:34:26 -0700 Subject: [PATCH 06/10] feat(application): mark configured apps in select from state.toml The select picker decorated choices from config.toml profiles only, so apps configured under the new model showed as unconfigured. It now marks an app "(configured)" when state.toml holds an entry for it, falling back to legacy config.toml profiles while config.toml is still supported. Adds Config.ApplicationInState for the state-only lookup. Co-Authored-By: Claude Fable 5 --- pkg/cmd/application/selectapp/select.go | 24 +++++++++------- pkg/cmd/application/selectapp/select_test.go | 29 ++++++++++++++++++++ pkg/config/config.go | 9 ++++++ pkg/config/resolution_test.go | 11 ++++++++ test/config.go | 10 ++++++- 5 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 pkg/cmd/application/selectapp/select_test.go diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index fa143286..10d24b5d 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -135,6 +135,17 @@ func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { return chosen, nil } +// applicationConfigured reports whether an application is already known to the +// CLI. state.toml is the source of truth; the legacy config.toml profiles are +// a fallback while config.toml is still supported (remove once it's gone). +func applicationConfigured(cfg config.IConfig, appID string) bool { + if cfg.ApplicationInState(appID) { + return true + } + exists, _ := cfg.ApplicationIDExists(appID) + return exists +} + func pickApplication( opts *SelectOptions, apps []dashboard.Application, @@ -152,21 +163,14 @@ func pickApplication( return nil, fmt.Errorf("--app-name is required in non-interactive mode") } - configuredProfiles := opts.Config.ConfiguredProfiles() - configuredAppIDs := make(map[string]string) - for _, p := range configuredProfiles { - configuredAppIDs[p.ApplicationID] = p.Name - } - cs := opts.IO.ColorScheme() appOptions := make([]string, len(apps)) for i, app := range apps { label := fmt.Sprintf("%s (%s)", app.ID, app.Name) - if profileName, ok := configuredAppIDs[app.ID]; ok { - appOptions[i] = fmt.Sprintf("%s %s", label, cs.Greenf("profile: %s", profileName)) - } else { - appOptions[i] = label + if applicationConfigured(opts.Config, app.ID) { + label = fmt.Sprintf("%s %s", label, cs.Green("(configured)")) } + appOptions[i] = label } var selected int diff --git a/pkg/cmd/application/selectapp/select_test.go b/pkg/cmd/application/selectapp/select_test.go new file mode 100644 index 00000000..2a5cbf7c --- /dev/null +++ b/pkg/cmd/application/selectapp/select_test.go @@ -0,0 +1,29 @@ +package selectapp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/algolia/cli/test" +) + +func TestApplicationConfigured(t *testing.T) { + cfg := test.NewDefaultConfigStub() + cfg.SavedApps = map[string]test.SavedApplication{ + "STATE_APP": {Alias: "prod"}, + } + // "default" is the config.toml profile's application ID (legacy fallback). + + t.Run("in state.toml", func(t *testing.T) { + assert.True(t, applicationConfigured(cfg, "STATE_APP")) + }) + + t.Run("only in legacy config.toml", func(t *testing.T) { + assert.True(t, applicationConfigured(cfg, "default")) + }) + + t.Run("unknown application", func(t *testing.T) { + assert.False(t, applicationConfigured(cfg, "UNKNOWN")) + }) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index d09d4c69..097725f5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -32,6 +32,7 @@ type IConfig interface { // New model (state.toml + OS keychain). ActiveApplicationID() string + ApplicationInState(appID string) bool ApplicationIDByAlias(alias string) (string, bool) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error SetCrawlerAPIKey(appID, crawlerAPIKey string) error @@ -125,6 +126,14 @@ func (c *Config) loadState() *State { return c.state } +// ApplicationInState reports whether state.toml already holds an entry for the +// given application, i.e. the application has been configured under the new +// storage model. +func (c *Config) ApplicationInState(appID string) bool { + _, ok := c.loadState().Applications[appID] + return ok +} + // StateFileExists reports whether state.toml exists on disk, i.e. the new // storage model (state.toml + OS keychain) is already in use on this machine. func (c *Config) StateFileExists() bool { diff --git a/pkg/config/resolution_test.go b/pkg/config/resolution_test.go index 03c5d3b8..15fc7a81 100644 --- a/pkg/config/resolution_test.go +++ b/pkg/config/resolution_test.go @@ -298,3 +298,14 @@ func TestProfile_GetCrawlerUserID_FromState(t *testing.T) { require.NoError(t, err) assert.Equal(t, "crawler-user", userID) } + +func TestConfig_ApplicationInState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\n", + ), 0o600)) + + cfg := &Config{StateFile: path} + assert.True(t, cfg.ApplicationInState("APP1")) + assert.False(t, cfg.ApplicationInState("APP2")) +} diff --git a/test/config.go b/test/config.go index 0b0828ef..65a77ada 100644 --- a/test/config.go +++ b/test/config.go @@ -147,6 +147,11 @@ func (c *ConfigStub) ActiveApplicationID() string { return c.ActiveAppID } +func (c *ConfigStub) ApplicationInState(appID string) bool { + _, ok := c.SavedApps[appID] + return ok +} + func (c *ConfigStub) ApplicationIDByAlias(alias string) (string, bool) { for appID, app := range c.SavedApps { if app.Alias == alias { @@ -156,7 +161,10 @@ func (c *ConfigStub) ApplicationIDByAlias(alias string) (string, bool) { return "", false } -func (c *ConfigStub) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error { +func (c *ConfigStub) SaveApplication( + appID, alias, apiKeyUUID, apiKey string, + setCurrent bool, +) error { if c.SavedApps == nil { c.SavedApps = map[string]SavedApplication{} } From 383dd0db2948c8bb296da5308f00aecc268ee555 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 08:50:09 -0700 Subject: [PATCH 07/10] feat(application): state-aware "configured" marker + describe without auth Two follow-ups from auditing the command tree against state.toml: - application list marked apps configured from config.toml profiles only, so new-model apps showed as "(not configured)". It now uses the same state-first/config-fallback check as the select picker, centralized in apputil.ApplicationConfigured. - describe walks the command tree and needs no credentials, but lacked skipAuthCheck so it failed on a machine with nothing configured. It now skips the auth check. Co-Authored-By: Claude Fable 5 --- pkg/cmd/application/list/list.go | 11 ++--------- pkg/cmd/application/selectapp/select.go | 13 +------------ pkg/cmd/describe/describe.go | 5 +++++ pkg/cmd/shared/apputil/configured.go | 14 ++++++++++++++ .../apputil/configured_test.go} | 8 ++++---- 5 files changed, 26 insertions(+), 25 deletions(-) create mode 100644 pkg/cmd/shared/apputil/configured.go rename pkg/cmd/{application/selectapp/select_test.go => shared/apputil/configured_test.go} (73%) diff --git a/pkg/cmd/application/list/list.go b/pkg/cmd/application/list/list.go index 3473e877..34ae2e85 100644 --- a/pkg/cmd/application/list/list.go +++ b/pkg/cmd/application/list/list.go @@ -104,20 +104,13 @@ func runListCmd(opts *ListOptions) error { return nil } - configuredProfiles := opts.Config.ConfiguredProfiles() - configuredAppIDs := make(map[string]string) - for _, p := range configuredProfiles { - configuredAppIDs[p.ApplicationID] = p.Name - } - fmt.Fprintf(opts.IO.Out, "\nYour applications:\n\n") unconfigured := make([]dashboard.Application, 0) for _, app := range apps { - profileName, configured := configuredAppIDs[app.ID] label := fmt.Sprintf(" %s %s", app.ID, app.Name) - if configured { - fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Greenf("(configured: %s)", profileName)) + if apputil.ApplicationConfigured(opts.Config, app.ID) { + fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Green("(configured)")) } else { fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Gray("(not configured)")) unconfigured = append(unconfigured, app) diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index 10d24b5d..54a9fc80 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -135,17 +135,6 @@ func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { return chosen, nil } -// applicationConfigured reports whether an application is already known to the -// CLI. state.toml is the source of truth; the legacy config.toml profiles are -// a fallback while config.toml is still supported (remove once it's gone). -func applicationConfigured(cfg config.IConfig, appID string) bool { - if cfg.ApplicationInState(appID) { - return true - } - exists, _ := cfg.ApplicationIDExists(appID) - return exists -} - func pickApplication( opts *SelectOptions, apps []dashboard.Application, @@ -167,7 +156,7 @@ func pickApplication( appOptions := make([]string, len(apps)) for i, app := range apps { label := fmt.Sprintf("%s (%s)", app.ID, app.Name) - if applicationConfigured(opts.Config, app.ID) { + if apputil.ApplicationConfigured(opts.Config, app.ID) { label = fmt.Sprintf("%s %s", label, cs.Green("(configured)")) } appOptions[i] = label diff --git a/pkg/cmd/describe/describe.go b/pkg/cmd/describe/describe.go index 974f609d..fab8600a 100644 --- a/pkg/cmd/describe/describe.go +++ b/pkg/cmd/describe/describe.go @@ -38,6 +38,11 @@ func NewDescribeCmd(f *cmdutil.Factory) *cobra.Command { Aliases: []string{"schema"}, Args: cobra.ArbitraryArgs, Short: "Describe commands and flags as JSON.", + // Describe only walks the command tree; it needs no credentials and + // must work on a machine with nothing configured. + Annotations: map[string]string{ + "skipAuthCheck": "true", + }, Long: heredoc.Doc(` Describe the CLI's command tree in a machine-readable format. With no arguments, this command describes the root command. diff --git a/pkg/cmd/shared/apputil/configured.go b/pkg/cmd/shared/apputil/configured.go new file mode 100644 index 00000000..98e634e7 --- /dev/null +++ b/pkg/cmd/shared/apputil/configured.go @@ -0,0 +1,14 @@ +package apputil + +import "github.com/algolia/cli/pkg/config" + +// ApplicationConfigured reports whether an application is already known to the +// CLI. state.toml is the source of truth; the legacy config.toml profiles are +// a fallback while config.toml is still supported (remove once it's gone). +func ApplicationConfigured(cfg config.IConfig, appID string) bool { + if cfg.ApplicationInState(appID) { + return true + } + exists, _ := cfg.ApplicationIDExists(appID) + return exists +} diff --git a/pkg/cmd/application/selectapp/select_test.go b/pkg/cmd/shared/apputil/configured_test.go similarity index 73% rename from pkg/cmd/application/selectapp/select_test.go rename to pkg/cmd/shared/apputil/configured_test.go index 2a5cbf7c..1c544310 100644 --- a/pkg/cmd/application/selectapp/select_test.go +++ b/pkg/cmd/shared/apputil/configured_test.go @@ -1,4 +1,4 @@ -package selectapp +package apputil import ( "testing" @@ -16,14 +16,14 @@ func TestApplicationConfigured(t *testing.T) { // "default" is the config.toml profile's application ID (legacy fallback). t.Run("in state.toml", func(t *testing.T) { - assert.True(t, applicationConfigured(cfg, "STATE_APP")) + assert.True(t, ApplicationConfigured(cfg, "STATE_APP")) }) t.Run("only in legacy config.toml", func(t *testing.T) { - assert.True(t, applicationConfigured(cfg, "default")) + assert.True(t, ApplicationConfigured(cfg, "default")) }) t.Run("unknown application", func(t *testing.T) { - assert.False(t, applicationConfigured(cfg, "UNKNOWN")) + assert.False(t, ApplicationConfigured(cfg, "UNKNOWN")) }) } From 29a208be61a279d503ae719e6f7f25f5eabf77b0 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 09:05:10 -0700 Subject: [PATCH 08/10] feat(config): clearer error when the current app has no keychain key When state.toml resolves a current application but its key isn't in this machine's keychain (e.g. state.toml synced across machines without it), GetAPIKey/GetCrawlerAPIKey returned the generic "not configured yet". The error now names the application and points to the fix (`application select` / `auth crawler`, or the matching env var). Re-authenticating rewrites the keychain entry, so it restores a working state. Co-Authored-By: Claude Fable 5 --- pkg/config/profile.go | 15 +++++++++++++-- pkg/config/resolution_test.go | 4 ++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/config/profile.go b/pkg/config/profile.go index fd9d8972..ba606716 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "os" "path/filepath" "strings" @@ -94,7 +95,12 @@ func (p *Profile) GetAPIKey() (string, error) { if secrets := p.config.appSecretsFor(appID); secrets != nil && secrets.APIKey != "" { return secrets.APIKey, nil } - return "", ErrAPIKeyNotConfigured + // The application is set but its key isn't in this machine's + // keychain (e.g. state.toml synced across machines without it). + return "", fmt.Errorf( + "no API key stored in your keychain for the current application %q; run `algolia application select` to store one, or set ALGOLIA_API_KEY", + appID, + ) } } @@ -218,7 +224,12 @@ func (p *Profile) GetCrawlerAPIKey() (string, error) { secrets.CrawlerAPIKey != "" { return secrets.CrawlerAPIKey, nil } - return "", ErrCrawlerAPIKeyNotConfigured + // The application is set but its crawler key isn't in this + // machine's keychain. + return "", fmt.Errorf( + "no Crawler API key stored in your keychain for the current application %q; run `algolia auth crawler` to store one, or set ALGOLIA_CRAWLER_API_KEY", + appID, + ) } } diff --git a/pkg/config/resolution_test.go b/pkg/config/resolution_test.go index 15fc7a81..0dd32d51 100644 --- a/pkg/config/resolution_test.go +++ b/pkg/config/resolution_test.go @@ -242,6 +242,10 @@ func TestProfile_GetAPIKey_ActiveAppWithoutKeyErrors(t *testing.T) { // APP1 resolved from state but no keychain key → error, never "legacy-key". _, err := cfg.Profile().GetAPIKey() require.Error(t, err) + // The message names the application and points to a fix, rather than the + // generic "not configured yet". + assert.Contains(t, err.Error(), "APP1") + assert.Contains(t, err.Error(), "application select") } func TestProfile_GetSearchHosts_FromState(t *testing.T) { From 13dad65f0c8091ed752f2e84262eb83332293437 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 13:41:12 -0700 Subject: [PATCH 09/10] perf(config): short-circuit ShouldMigrate on the state.toml check Check state.toml first: an already-migrated machine (the steady state, hit on every command) now settles in a single stat instead of also stat-ing config.toml. The boolean result is unchanged. Co-Authored-By: Claude Fable 5 --- pkg/config/migrate.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go index b5046d62..022d8c9a 100644 --- a/pkg/config/migrate.go +++ b/pkg/config/migrate.go @@ -13,14 +13,16 @@ import ( // ShouldMigrate reports whether the one-time config.toml → state.toml + // keychain migration still has to run: config.toml exists and state.toml // (only written on success, so doubling as the "migrated" marker) does not. +// +// The state.toml check comes first so an already-migrated machine — the +// steady state, hit on every command — settles in a single stat instead of +// also stat-ing config.toml. func (c *Config) ShouldMigrate() bool { - if c.File == "" { + if c.File == "" || c.StateFileExists() { return false } - if _, err := os.Stat(c.File); err != nil { - return false - } - return !c.StateFileExists() + _, err := os.Stat(c.File) + return err == nil } // Migrate moves the legacy config.toml profiles into the new model (state.toml From 1db073e30fd35c142d9eb816726e9605907b70e2 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 13:56:11 -0700 Subject: [PATCH 10/10] perf(application): O(1) configured-app lookup when listing/selecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marking apps "(configured)" called ConfiguredProfiles() (a full viper re-parse) once per application — O(apps × profiles) with a heavy constant. Now the config.toml profile app IDs are collected once into a set (ProfileApplicationIDs); the per-app check is two O(1) map lookups (cached state.toml + the set). Both `application list` and `application select` build the set once before their loop. Co-Authored-By: Claude Fable 5 --- pkg/cmd/application/list/list.go | 3 ++- pkg/cmd/application/selectapp/select.go | 3 ++- pkg/cmd/shared/apputil/configured.go | 26 ++++++++++++++++------- pkg/cmd/shared/apputil/configured_test.go | 23 +++++++++++++++++--- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/application/list/list.go b/pkg/cmd/application/list/list.go index 34ae2e85..30cd6978 100644 --- a/pkg/cmd/application/list/list.go +++ b/pkg/cmd/application/list/list.go @@ -107,9 +107,10 @@ func runListCmd(opts *ListOptions) error { fmt.Fprintf(opts.IO.Out, "\nYour applications:\n\n") unconfigured := make([]dashboard.Application, 0) + profileApps := apputil.ProfileApplicationIDs(opts.Config.ConfiguredProfiles()) for _, app := range apps { label := fmt.Sprintf(" %s %s", app.ID, app.Name) - if apputil.ApplicationConfigured(opts.Config, app.ID) { + if apputil.ApplicationConfigured(opts.Config, profileApps, app.ID) { fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Green("(configured)")) } else { fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Gray("(not configured)")) diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index 54a9fc80..eac0389c 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -153,10 +153,11 @@ func pickApplication( } cs := opts.IO.ColorScheme() + profileApps := apputil.ProfileApplicationIDs(opts.Config.ConfiguredProfiles()) appOptions := make([]string, len(apps)) for i, app := range apps { label := fmt.Sprintf("%s (%s)", app.ID, app.Name) - if apputil.ApplicationConfigured(opts.Config, app.ID) { + if apputil.ApplicationConfigured(opts.Config, profileApps, app.ID) { label = fmt.Sprintf("%s %s", label, cs.Green("(configured)")) } appOptions[i] = label diff --git a/pkg/cmd/shared/apputil/configured.go b/pkg/cmd/shared/apputil/configured.go index 98e634e7..dc6b14c1 100644 --- a/pkg/cmd/shared/apputil/configured.go +++ b/pkg/cmd/shared/apputil/configured.go @@ -2,13 +2,23 @@ package apputil import "github.com/algolia/cli/pkg/config" -// ApplicationConfigured reports whether an application is already known to the -// CLI. state.toml is the source of truth; the legacy config.toml profiles are -// a fallback while config.toml is still supported (remove once it's gone). -func ApplicationConfigured(cfg config.IConfig, appID string) bool { - if cfg.ApplicationInState(appID) { - return true +// ProfileApplicationIDs returns the set of application IDs backed by a legacy +// config.toml profile. Built once by the caller so a per-application loop tests +// membership in O(1) instead of re-parsing config.toml on every iteration. +func ProfileApplicationIDs(profiles []*config.Profile) map[string]bool { + ids := make(map[string]bool, len(profiles)) + for _, p := range profiles { + if p.ApplicationID != "" { + ids[p.ApplicationID] = true + } } - exists, _ := cfg.ApplicationIDExists(appID) - return exists + return ids +} + +// ApplicationConfigured reports whether an application is already known to the +// CLI. state.toml is the source of truth (an O(1) cached lookup); profileApps +// is the legacy config.toml fallback while config.toml is still supported +// (remove once it's gone). +func ApplicationConfigured(cfg config.IConfig, profileApps map[string]bool, appID string) bool { + return cfg.ApplicationInState(appID) || profileApps[appID] } diff --git a/pkg/cmd/shared/apputil/configured_test.go b/pkg/cmd/shared/apputil/configured_test.go index 1c544310..e468d98e 100644 --- a/pkg/cmd/shared/apputil/configured_test.go +++ b/pkg/cmd/shared/apputil/configured_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/algolia/cli/pkg/config" "github.com/algolia/cli/test" ) @@ -14,16 +15,32 @@ func TestApplicationConfigured(t *testing.T) { "STATE_APP": {Alias: "prod"}, } // "default" is the config.toml profile's application ID (legacy fallback). + profileApps := ProfileApplicationIDs(cfg.ConfiguredProfiles()) t.Run("in state.toml", func(t *testing.T) { - assert.True(t, ApplicationConfigured(cfg, "STATE_APP")) + assert.True(t, ApplicationConfigured(cfg, profileApps, "STATE_APP")) }) t.Run("only in legacy config.toml", func(t *testing.T) { - assert.True(t, ApplicationConfigured(cfg, "default")) + assert.True(t, ApplicationConfigured(cfg, profileApps, "default")) }) t.Run("unknown application", func(t *testing.T) { - assert.False(t, ApplicationConfigured(cfg, "UNKNOWN")) + assert.False(t, ApplicationConfigured(cfg, profileApps, "UNKNOWN")) }) } + +func TestProfileApplicationIDs(t *testing.T) { + profiles := []*config.Profile{ + {Name: "prod", ApplicationID: "APP1"}, + {Name: "dev", ApplicationID: "APP2"}, + {Name: "broken", ApplicationID: ""}, // skipped: no app ID + } + + ids := ProfileApplicationIDs(profiles) + + assert.True(t, ids["APP1"]) + assert.True(t, ids["APP2"]) + assert.False(t, ids[""]) // empty IDs never become a member + assert.Len(t, ids, 2) +}